Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

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)

AltStyle によって変換されたページ (->オリジナル) /