Skip to content

Commit 05af8d0

Browse files
authored
fix(node): Fix preloading of instrumentation (#17403)
Extracted out of #17371 I noticed that we were not fully consistent in instrumentation IDs for integrations that have multiple instrumentation. The intent is that users can provide the _integration name_ (e.g. `Http`) and it will preload all http instrumentation. To achieve this, I adjusted the preload filter code to look for exact matches as well as `startsWith(`${name}.id`)`. I also adjusted the test to be more declarative and mock/reset stuff properly (this lead to issues in the linked PR, and should generally be a bit cleaner). I also updated all instrumentation IDs to follow this pattern. We should be mindful of following this with new instrumentation we add.
1 parent 2ec09c9 commit 05af8d0

File tree

7 files changed

+38
-13
lines changed

7 files changed

+38
-13
lines changed

.cursor/BUGBOT.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,4 @@ Do not flag the issues below if they appear in tests.
4040
- If there's no direct span that's wrapping the captured exception, apply a proper `type` value, following the same naming
4141
convention as the `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN` value.
4242
- When calling `startSpan`, check if error cases are handled. If flag that it might make sense to try/catch and call `captureException`.
43+
- When calling `generateInstrumentationOnce`, the passed in name MUST match the name of the integration that uses it. If there are more than one instrumentations, they need to follow the pattern `${INSTRUMENTATION_NAME}.some-suffix`.

packages/nestjs/src/integrations/nest.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import { SentryNestInstrumentation } from './sentry-nest-instrumentation';
66

77
const INTEGRATION_NAME = 'Nest';
88

9-
const instrumentNestCore = generateInstrumentOnce('Nest-Core', () => {
9+
const instrumentNestCore = generateInstrumentOnce(`${INTEGRATION_NAME}.Core`, () => {
1010
return new NestInstrumentationCore();
1111
});
1212

13-
const instrumentNestCommon = generateInstrumentOnce('Nest-Common', () => {
13+
const instrumentNestCommon = generateInstrumentOnce(`${INTEGRATION_NAME}.Common`, () => {
1414
return new SentryNestInstrumentation();
1515
});
1616

17-
const instrumentNestEvent = generateInstrumentOnce('Nest-Event', () => {
17+
const instrumentNestEvent = generateInstrumentOnce(`${INTEGRATION_NAME}.Event`, () => {
1818
return new SentryNestEventInstrumentation();
1919
});
2020

packages/node/src/integrations/tracing/fastify/index.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,11 @@ interface FastifyHandlerOptions {
9090
}
9191

9292
const INTEGRATION_NAME = 'Fastify';
93-
const INTEGRATION_NAME_V5 = 'Fastify-V5';
94-
const INTEGRATION_NAME_V3 = 'Fastify-V3';
9593

96-
export const instrumentFastifyV3 = generateInstrumentOnce(INTEGRATION_NAME_V3, () => new FastifyInstrumentationV3());
94+
export const instrumentFastifyV3 = generateInstrumentOnce(
95+
`${INTEGRATION_NAME}.v3`,
96+
() => new FastifyInstrumentationV3(),
97+
);
9798

9899
function getFastifyIntegration(): ReturnType<typeof _fastifyIntegration> | undefined {
99100
const client = getClient();
@@ -135,7 +136,7 @@ function handleFastifyError(
135136
}
136137
}
137138

138-
export const instrumentFastify = generateInstrumentOnce(INTEGRATION_NAME_V5, () => {
139+
export const instrumentFastify = generateInstrumentOnce(`${INTEGRATION_NAME}.v5`, () => {
139140
const fastifyOtelInstrumentationInstance = new FastifyOtelInstrumentation();
140141
const plugin = fastifyOtelInstrumentationInstance.plugin();
141142

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,13 @@ const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, red
7575
span.updateName(truncate(spanDescription, 1024));
7676
};
7777

78-
const instrumentIORedis = generateInstrumentOnce('IORedis', () => {
78+
const instrumentIORedis = generateInstrumentOnce(`${INTEGRATION_NAME}.IORedis`, () => {
7979
return new IORedisInstrumentation({
8080
responseHook: cacheResponseHook,
8181
});
8282
});
8383

84-
const instrumentRedisModule = generateInstrumentOnce('Redis', () => {
84+
const instrumentRedisModule = generateInstrumentOnce(`${INTEGRATION_NAME}.Redis`, () => {
8585
return new RedisInstrumentation({
8686
responseHook: cacheResponseHook,
8787
});

packages/node/src/sdk/initOtel.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ function getPreloadMethods(integrationNames?: string[]): ((() => void) & { id: s
101101
return instruments;
102102
}
103103

104-
return instruments.filter(instrumentation => integrationNames.includes(instrumentation.id));
104+
// We match exact matches of instrumentation, but also match prefixes, e.g. "Fastify.v5" will match "Fastify"
105+
return instruments.filter(instrumentation => {
106+
const id = instrumentation.id;
107+
return integrationNames.some(integrationName => id === integrationName || id.startsWith(`${integrationName}.`));
108+
});
105109
}
106110

107111
/** Just exported for tests. */

packages/node/test/sdk/preload.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,27 @@
11
import { debug } from '@sentry/core';
2-
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
3+
import { resetGlobals } from '../helpers/mockSdkInit';
34

45
describe('preload', () => {
6+
beforeEach(() => {
7+
// Mock this to prevent conflicts with other tests
8+
vi.mock('../../src/integrations/tracing', async (importOriginal: () => Promise<Record<string, unknown>>) => {
9+
const actual = await importOriginal();
10+
return {
11+
...actual,
12+
getOpenTelemetryInstrumentationToPreload: () => [
13+
Object.assign(vi.fn(), { id: 'Http.sentry' }),
14+
Object.assign(vi.fn(), { id: 'Http' }),
15+
Object.assign(vi.fn(), { id: 'Express' }),
16+
Object.assign(vi.fn(), { id: 'Graphql' }),
17+
],
18+
};
19+
});
20+
});
21+
522
afterEach(() => {
6-
vi.resetAllMocks();
723
debug.disable();
24+
resetGlobals();
825

926
delete process.env.SENTRY_DEBUG;
1027
delete process.env.SENTRY_PRELOAD_INTEGRATIONS;
@@ -29,6 +46,7 @@ describe('preload', () => {
2946

3047
await import('../../src/preload');
3148

49+
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http.sentry instrumentation');
3250
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation');
3351
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation');
3452
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation');
@@ -44,6 +62,7 @@ describe('preload', () => {
4462

4563
await import('../../src/preload');
4664

65+
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http.sentry instrumentation');
4766
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation');
4867
expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation');
4968
expect(logSpy).not.toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation');

packages/react-router/src/server/integration/reactRouterServer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ReactRouterInstrumentation } from '../instrumentation/reactRouter';
55

66
const INTEGRATION_NAME = 'ReactRouterServer';
77

8-
const instrumentReactRouter = generateInstrumentOnce('React-Router-Server', () => {
8+
const instrumentReactRouter = generateInstrumentOnce(INTEGRATION_NAME, () => {
99
return new ReactRouterInstrumentation();
1010
});
1111

0 commit comments

Comments
 (0)