Skip to content

Commit 56aac53

Browse files
authored
fix(node): Ensure late init works with all integrations (#16016)
In cases where we tweak the options we pass to the instrumentation, our code did not handle this very well. This could lead to certain options missing when init is called again later, as we updated the config with incomplete/wrong options. To fix this properly, I added a new variant to `generateInstrumentOnce` which accepts a class and an options transformer, which ensures that the options are always correctly transformed, no matter if called the first or second time. (The code to make this overloaded `generateInstrumentOnce` function work with TS was pretty tricky, but I think it is good now - type inferral etc. works nicely now!) Fixes #16004
1 parent 30591d4 commit 56aac53

File tree

5 files changed

+102
-34
lines changed

5 files changed

+102
-34
lines changed

packages/aws-serverless/src/integration/awslambda.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,19 @@ interface AwsLambdaOptions {
1515
disableAwsContextPropagation?: boolean;
1616
}
1717

18-
export const instrumentAwsLambda = generateInstrumentOnce<AwsLambdaOptions>(
18+
export const instrumentAwsLambda = generateInstrumentOnce(
1919
'AwsLambda',
20-
(_options: AwsLambdaOptions = {}) => {
21-
const options = {
20+
AwsLambdaInstrumentation,
21+
(options: AwsLambdaOptions) => {
22+
return {
2223
disableAwsContextPropagation: true,
23-
..._options,
24-
};
25-
26-
return new AwsLambdaInstrumentation({
2724
...options,
2825
eventContextExtractor,
2926
requestHook(span) {
3027
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.otel.aws-lambda');
3128
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'function.aws.lambda');
3229
},
33-
});
30+
};
3431
},
3532
);
3633

packages/node/src/integrations/node-fetch/index.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, getClient } from '
55
import { generateInstrumentOnce } from '../../otel/instrument';
66
import type { NodeClient } from '../../sdk/client';
77
import type { NodeClientOptions } from '../../types';
8-
import type { SentryNodeFetchInstrumentationOptions } from './SentryNodeFetchInstrumentation';
98
import { SentryNodeFetchInstrumentation } from './SentryNodeFetchInstrumentation';
109

1110
const INTEGRATION_NAME = 'NodeFetch';
@@ -33,14 +32,19 @@ interface NodeFetchOptions {
3332
ignoreOutgoingRequests?: (url: string) => boolean;
3433
}
3534

36-
const instrumentOtelNodeFetch = generateInstrumentOnce<UndiciInstrumentationConfig>(INTEGRATION_NAME, config => {
37-
return new UndiciInstrumentation(config);
38-
});
35+
const instrumentOtelNodeFetch = generateInstrumentOnce(
36+
INTEGRATION_NAME,
37+
UndiciInstrumentation,
38+
(options: NodeFetchOptions) => {
39+
return getConfigWithDefaults(options);
40+
},
41+
);
3942

40-
const instrumentSentryNodeFetch = generateInstrumentOnce<SentryNodeFetchInstrumentationOptions>(
43+
const instrumentSentryNodeFetch = generateInstrumentOnce(
4144
`${INTEGRATION_NAME}.sentry`,
42-
config => {
43-
return new SentryNodeFetchInstrumentation(config);
45+
SentryNodeFetchInstrumentation,
46+
(options: NodeFetchOptions) => {
47+
return options;
4448
},
4549
);
4650

@@ -52,8 +56,7 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
5256

5357
// This is the "regular" OTEL instrumentation that emits spans
5458
if (instrumentSpans) {
55-
const instrumentationConfig = getConfigWithDefaults(options);
56-
instrumentOtelNodeFetch(instrumentationConfig);
59+
instrumentOtelNodeFetch(options);
5760
}
5861

5962
// This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces

packages/node/src/integrations/tracing/graphql.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,13 @@ interface GraphqlOptions {
3737

3838
const INTEGRATION_NAME = 'Graphql';
3939

40-
export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
40+
export const instrumentGraphql = generateInstrumentOnce(
4141
INTEGRATION_NAME,
42-
(_options: GraphqlOptions = {}) => {
42+
GraphQLInstrumentation,
43+
(_options: GraphqlOptions) => {
4344
const options = getOptionsWithDefaults(_options);
4445

45-
return new GraphQLInstrumentation({
46+
return {
4647
...options,
4748
responseHook(span) {
4849
addOriginToSpan(span, 'auto.graphql.otel.graphql');
@@ -73,7 +74,7 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
7374
}
7475
}
7576
},
76-
});
77+
};
7778
},
7879
);
7980

packages/node/src/otel/instrument.ts

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,47 @@ import { type Instrumentation, registerInstrumentations } from '@opentelemetry/i
33
/** Exported only for tests. */
44
export const INSTRUMENTED: Record<string, Instrumentation> = {};
55

6-
/**
7-
* Instrument an OpenTelemetry instrumentation once.
8-
* This will skip running instrumentation again if it was already instrumented.
9-
*/
6+
export function generateInstrumentOnce<
7+
Options,
8+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9+
InstrumentationClass extends new (...args: any[]) => Instrumentation,
10+
>(
11+
name: string,
12+
instrumentationClass: InstrumentationClass,
13+
optionsCallback: (options: Options) => ConstructorParameters<InstrumentationClass>[0],
14+
): ((options: Options) => InstanceType<InstrumentationClass>) & { id: string };
1015
export function generateInstrumentOnce<
1116
Options = unknown,
1217
InstrumentationInstance extends Instrumentation = Instrumentation,
1318
>(
1419
name: string,
1520
creator: (options?: Options) => InstrumentationInstance,
21+
): ((options?: Options) => InstrumentationInstance) & { id: string };
22+
/**
23+
* Instrument an OpenTelemetry instrumentation once.
24+
* This will skip running instrumentation again if it was already instrumented.
25+
*/
26+
export function generateInstrumentOnce<Options>(
27+
name: string,
28+
creatorOrClass: (new (...args: any[]) => Instrumentation) | ((options?: Options) => Instrumentation),
29+
optionsCallback?: (options: Options) => unknown,
30+
): ((options: Options) => Instrumentation) & { id: string } {
31+
if (optionsCallback) {
32+
return _generateInstrumentOnceWithOptions(
33+
name,
34+
creatorOrClass as new (...args: unknown[]) => Instrumentation,
35+
optionsCallback,
36+
);
37+
}
38+
39+
return _generateInstrumentOnce(name, creatorOrClass as (options?: Options) => Instrumentation);
40+
}
41+
42+
// The plain version without handling of options
43+
// Should not be used with custom options that are mutated in the creator!
44+
function _generateInstrumentOnce<Options = unknown, InstrumentationInstance extends Instrumentation = Instrumentation>(
45+
name: string,
46+
creator: (options?: Options) => InstrumentationInstance,
1647
): ((options?: Options) => InstrumentationInstance) & { id: string } {
1748
return Object.assign(
1849
(options?: Options) => {
@@ -38,6 +69,40 @@ export function generateInstrumentOnce<
3869
);
3970
}
4071

72+
// This version handles options properly
73+
function _generateInstrumentOnceWithOptions<
74+
Options,
75+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
76+
InstrumentationClass extends new (...args: any[]) => Instrumentation,
77+
>(
78+
name: string,
79+
instrumentationClass: InstrumentationClass,
80+
optionsCallback: (options: Options) => ConstructorParameters<InstrumentationClass>[0],
81+
): ((options: Options) => InstanceType<InstrumentationClass>) & { id: string } {
82+
return Object.assign(
83+
(_options: Options) => {
84+
const options = optionsCallback(_options);
85+
86+
const instrumented = INSTRUMENTED[name] as InstanceType<InstrumentationClass> | undefined;
87+
if (instrumented) {
88+
// Ensure we update options
89+
instrumented.setConfig(options);
90+
return instrumented;
91+
}
92+
93+
const instrumentation = new instrumentationClass(options) as InstanceType<InstrumentationClass>;
94+
INSTRUMENTED[name] = instrumentation;
95+
96+
registerInstrumentations({
97+
instrumentations: [instrumentation],
98+
});
99+
100+
return instrumentation;
101+
},
102+
{ id: name },
103+
);
104+
}
105+
41106
/**
42107
* Ensure a given callback is called when the instrumentation is actually wrapping something.
43108
* This can be used to ensure some logic is only called when the instrumentation is actually active.

packages/remix/src/server/integrations/opentelemetry.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,24 @@ import type { RemixOptions } from '../../utils/remixOptions';
77

88
const INTEGRATION_NAME = 'Remix';
99

10-
const instrumentRemix = generateInstrumentOnce<RemixOptions>(
11-
INTEGRATION_NAME,
12-
(_options?: RemixOptions) =>
13-
new RemixInstrumentation({
14-
actionFormDataAttributes: _options?.sendDefaultPii ? _options?.captureActionFormDataKeys : undefined,
15-
}),
16-
);
10+
interface RemixInstrumentationOptions {
11+
actionFormDataAttributes?: Record<string, string | boolean>;
12+
}
13+
14+
const instrumentRemix = generateInstrumentOnce(INTEGRATION_NAME, (options?: RemixInstrumentationOptions) => {
15+
return new RemixInstrumentation(options);
16+
});
1717

1818
const _remixIntegration = (() => {
1919
return {
2020
name: 'Remix',
2121
setupOnce() {
2222
const client = getClient();
23-
const options = client?.getOptions();
23+
const options = client?.getOptions() as RemixOptions | undefined;
2424

25-
instrumentRemix(options);
25+
instrumentRemix({
26+
actionFormDataAttributes: options?.sendDefaultPii ? options?.captureActionFormDataKeys : undefined,
27+
});
2628
},
2729

2830
setup(client: Client) {

0 commit comments

Comments
 (0)