Skip to content

Commit

Permalink
fix(node): Ensure graphql options are correct when preloading (#13769)
Browse files Browse the repository at this point in the history
Noticed this by chance, due to the way `generateInstrumentOnce` works,
we need to pass in the fully formed config.
  • Loading branch information
mydea authored Oct 8, 2024
1 parent 48b3a78 commit 4ad2c35
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
21 changes: 14 additions & 7 deletions packages/node/src/integrations/tracing/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,7 @@ const INTEGRATION_NAME = 'Graphql';
export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
INTEGRATION_NAME,
(_options: GraphqlOptions = {}) => {
const options = {
ignoreResolveSpans: true,
ignoreTrivialResolveSpans: true,
useOperationNameForRootSpan: true,
..._options,
};
const options = getOptionsWithDefaults(_options);

return new GraphQLInstrumentation({
...options,
Expand Down Expand Up @@ -89,7 +84,10 @@ const _graphqlIntegration = ((options: GraphqlOptions = {}) => {
return {
name: INTEGRATION_NAME,
setupOnce() {
instrumentGraphql(options);
// We set defaults here, too, because otherwise we'd update the instrumentation config
// to the config without defaults, as `generateInstrumentOnce` automatically calls `setConfig(options)`
// when being called the second time
instrumentGraphql(getOptionsWithDefaults(options));
},
};
}) satisfies IntegrationFn;
Expand All @@ -100,3 +98,12 @@ const _graphqlIntegration = ((options: GraphqlOptions = {}) => {
* Capture tracing data for GraphQL.
*/
export const graphqlIntegration = defineIntegration(_graphqlIntegration);

function getOptionsWithDefaults(options?: GraphqlOptions): GraphqlOptions {
return {
ignoreResolveSpans: true,
ignoreTrivialResolveSpans: true,
useOperationNameForRootSpan: true,
...options,
};
}
3 changes: 2 additions & 1 deletion packages/node/src/otel/instrument.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { Instrumentation } from '@opentelemetry/instrumentation';
import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry';

const INSTRUMENTED: Record<string, Instrumentation> = {};
/** Exported only for tests. */
export const INSTRUMENTED: Record<string, Instrumentation> = {};

/**
* Instrument an OpenTelemetry instrumentation once.
Expand Down
46 changes: 46 additions & 0 deletions packages/node/test/integrations/tracing/graphql.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql';
import { graphqlIntegration, instrumentGraphql } from '../../../src/integrations/tracing/graphql';
import { INSTRUMENTED } from '../../../src/otel/instrument';

jest.mock('@opentelemetry/instrumentation-graphql');

describe('GraphQL', () => {
beforeEach(() => {
jest.clearAllMocks();
delete INSTRUMENTED.Graphql;

(GraphQLInstrumentation as unknown as jest.SpyInstance).mockImplementation(() => {
return {
setTracerProvider: () => undefined,
setMeterProvider: () => undefined,
getConfig: () => ({}),
setConfig: () => ({}),
enable: () => undefined,
};
});
});

it('defaults are correct for instrumentGraphql', () => {
instrumentGraphql({ ignoreTrivialResolveSpans: false });

expect(GraphQLInstrumentation).toHaveBeenCalledTimes(1);
expect(GraphQLInstrumentation).toHaveBeenCalledWith({
ignoreResolveSpans: true,
ignoreTrivialResolveSpans: false,
useOperationNameForRootSpan: true,
responseHook: expect.any(Function),
});
});

it('defaults are correct for _graphqlIntegration', () => {
graphqlIntegration({ ignoreTrivialResolveSpans: false }).setupOnce!();

expect(GraphQLInstrumentation).toHaveBeenCalledTimes(1);
expect(GraphQLInstrumentation).toHaveBeenCalledWith({
ignoreResolveSpans: true,
ignoreTrivialResolveSpans: false,
useOperationNameForRootSpan: true,
responseHook: expect.any(Function),
});
});
});

0 comments on commit 4ad2c35

Please sign in to comment.