Skip to content

Commit 3487263

Browse files
mydeamydea
committed
feat(node): Ensure fastify spans have better data
This ensures we have correct op, name & origin for all fastify middleware spans.
1 parent 2f88a5f commit 3487263

File tree

5 files changed

+141
-93
lines changed

5 files changed

+141
-93
lines changed

‎dev-packages/e2e-tests/test-applications/node-fastify/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
},
2424
"devDependencies": {
2525
"@sentry-internal/event-proxy-server": "link:../../../event-proxy-server",
26-
"@playwright/test": "^1.38.1"
26+
"@playwright/test": "^1.44.0"
2727
},
2828
"volta": {
2929
"extends": "../../package.json"

‎dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.ts‎ renamed to ‎dev-packages/e2e-tests/test-applications/node-fastify/playwright.config.mjs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type { PlaywrightTestConfig } from '@playwright/test';
21
import { devices } from '@playwright/test';
32

43
const fastifyPort = 3030;
@@ -7,7 +6,7 @@ const eventProxyPort = 3031;
76
/**
87
* See https://playwright.dev/docs/test-configuration.
98
*/
10-
const config: PlaywrightTestConfig = {
9+
const config = {
1110
testDir: './tests',
1211
/* Maximum time one test can run for. */
1312
timeout: 150_000,

‎dev-packages/e2e-tests/test-applications/node-fastify/tests/transactions.test.ts‎

Lines changed: 72 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -55,70 +55,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
5555

5656
expect(transactionEvent).toEqual(
5757
expect.objectContaining({
58-
spans: [
59-
{
60-
data: {
61-
'plugin.name': 'fastify -> sentry-fastify-error-handler',
62-
'fastify.type': 'middleware',
63-
'hook.name': 'onRequest',
64-
'otel.kind': 'INTERNAL',
65-
'sentry.origin': 'manual',
66-
},
67-
description: 'middleware - fastify -> sentry-fastify-error-handler',
68-
parent_span_id: expect.any(String),
69-
span_id: expect.any(String),
70-
start_timestamp: expect.any(Number),
71-
status: 'ok',
72-
timestamp: expect.any(Number),
73-
trace_id: expect.any(String),
74-
origin: 'manual',
75-
},
76-
{
77-
data: {
78-
'plugin.name': 'fastify -> sentry-fastify-error-handler',
79-
'fastify.type': 'request_handler',
80-
'http.route': '/test-transaction',
81-
'otel.kind': 'INTERNAL',
82-
'sentry.origin': 'auto.http.otel.fastify',
83-
},
84-
description: 'request handler - fastify -> sentry-fastify-error-handler',
85-
parent_span_id: expect.any(String),
86-
span_id: expect.any(String),
87-
start_timestamp: expect.any(Number),
88-
status: 'ok',
89-
timestamp: expect.any(Number),
90-
trace_id: expect.any(String),
91-
origin: 'auto.http.otel.fastify',
92-
},
93-
{
94-
data: {
95-
'otel.kind': 'INTERNAL',
96-
'sentry.origin': 'manual',
97-
},
98-
description: 'test-span',
99-
parent_span_id: expect.any(String),
100-
span_id: expect.any(String),
101-
start_timestamp: expect.any(Number),
102-
status: 'ok',
103-
timestamp: expect.any(Number),
104-
trace_id: expect.any(String),
105-
origin: 'manual',
106-
},
107-
{
108-
data: {
109-
'otel.kind': 'INTERNAL',
110-
'sentry.origin': 'manual',
111-
},
112-
description: 'child-span',
113-
parent_span_id: expect.any(String),
114-
span_id: expect.any(String),
115-
start_timestamp: expect.any(Number),
116-
status: 'ok',
117-
timestamp: expect.any(Number),
118-
trace_id: expect.any(String),
119-
origin: 'manual',
120-
},
121-
],
12258
transaction: 'GET /test-transaction',
12359
type: 'transaction',
12460
transaction_info: {
@@ -127,6 +63,78 @@ test('Sends an API route transaction', async ({ baseURL }) => {
12763
}),
12864
);
12965

66+
const spans = transactionEvent.spans || [];
67+
68+
expect(spans).toContainEqual({
69+
data: {
70+
'plugin.name': 'fastify -> sentry-fastify-error-handler',
71+
'fastify.type': 'middleware',
72+
'hook.name': 'onRequest',
73+
'otel.kind': 'INTERNAL',
74+
'sentry.origin': 'auto.http.otel.fastify',
75+
'sentry.op': 'middleware.fastify',
76+
},
77+
description: 'sentry-fastify-error-handler',
78+
op: 'middleware.fastify',
79+
parent_span_id: expect.any(String),
80+
span_id: expect.any(String),
81+
start_timestamp: expect.any(Number),
82+
status: 'ok',
83+
timestamp: expect.any(Number),
84+
trace_id: expect.any(String),
85+
origin: 'auto.http.otel.fastify',
86+
});
87+
88+
expect(spans).toContainEqual({
89+
data: {
90+
'plugin.name': 'fastify -> sentry-fastify-error-handler',
91+
'fastify.type': 'request_handler',
92+
'http.route': '/test-transaction',
93+
'otel.kind': 'INTERNAL',
94+
'sentry.op': 'request_handler.fastify',
95+
'sentry.origin': 'auto.http.otel.fastify',
96+
},
97+
description: 'sentry-fastify-error-handler',
98+
op: 'request_handler.fastify',
99+
parent_span_id: expect.any(String),
100+
span_id: expect.any(String),
101+
start_timestamp: expect.any(Number),
102+
status: 'ok',
103+
timestamp: expect.any(Number),
104+
trace_id: expect.any(String),
105+
origin: 'auto.http.otel.fastify',
106+
});
107+
108+
expect(spans).toContainEqual({
109+
data: {
110+
'otel.kind': 'INTERNAL',
111+
'sentry.origin': 'manual',
112+
},
113+
description: 'test-span',
114+
parent_span_id: expect.any(String),
115+
span_id: expect.any(String),
116+
start_timestamp: expect.any(Number),
117+
status: 'ok',
118+
timestamp: expect.any(Number),
119+
trace_id: expect.any(String),
120+
origin: 'manual',
121+
});
122+
123+
expect(spans).toContainEqual({
124+
data: {
125+
'otel.kind': 'INTERNAL',
126+
'sentry.origin': 'manual',
127+
},
128+
description: 'child-span',
129+
parent_span_id: expect.any(String),
130+
span_id: expect.any(String),
131+
start_timestamp: expect.any(Number),
132+
status: 'ok',
133+
timestamp: expect.any(Number),
134+
trace_id: expect.any(String),
135+
origin: 'manual',
136+
});
137+
130138
await expect
131139
.poll(
132140
async () => {

‎dev-packages/e2e-tests/test-applications/node-fastify/tsconfig.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@
66
"strict": true,
77
"outDir": "dist"
88
},
9-
"include": ["*.ts"]
9+
"include": ["./src/*.ts"]
1010
}

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

Lines changed: 66 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,39 @@
11
import { isWrapped } from '@opentelemetry/core';
22
import { FastifyInstrumentation } from '@opentelemetry/instrumentation-fastify';
3-
import { captureException, defineIntegration, getIsolationScope, isEnabled } from '@sentry/core';
3+
import {
4+
SEMANTIC_ATTRIBUTE_SENTRY_OP,
5+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
6+
captureException,
7+
defineIntegration,
8+
getClient,
9+
getIsolationScope,
10+
isEnabled,
11+
spanToJSON,
12+
} from '@sentry/core';
413
import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry';
5-
import type { IntegrationFn } from '@sentry/types';
14+
import type { IntegrationFn, Span } from '@sentry/types';
615
import { consoleSandbox } from '@sentry/utils';
716

8-
import { addOriginToSpan } from '../../utils/addOriginToSpan';
17+
// We inline the types we care about here
18+
interface Fastify {
19+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
20+
register: (plugin: any) => void;
21+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
22+
addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void;
23+
}
24+
25+
/**
26+
* Minimal request type containing properties around route information.
27+
* Works for Fastify 3, 4 and presumably 5.
28+
*/
29+
interface FastifyRequestRouteInfo {
30+
// since [email protected]
31+
routeOptions?: {
32+
url?: string;
33+
method?: string;
34+
};
35+
routerPath?: string;
36+
}
937

1038
const _fastifyIntegration = (() => {
1139
return {
@@ -14,7 +42,7 @@ const _fastifyIntegration = (() => {
1442
addOpenTelemetryInstrumentation(
1543
new FastifyInstrumentation({
1644
requestHook(span) {
17-
addOriginToSpan(span, 'auto.http.otel.fastify');
45+
addFastifySpanAttributes(span);
1846
},
1947
}),
2048
);
@@ -29,27 +57,6 @@ const _fastifyIntegration = (() => {
2957
*/
3058
export const fastifyIntegration = defineIntegration(_fastifyIntegration);
3159

32-
// We inline the types we care about here
33-
interface Fastify {
34-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
35-
register: (plugin: any) => void;
36-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
37-
addHook: (hook: string, handler: (request: any, reply: any, error: Error) => void) => void;
38-
}
39-
40-
/**
41-
* Minimal request type containing properties around route information.
42-
* Works for Fastify 3, 4 and presumably 5.
43-
*/
44-
interface FastifyRequestRouteInfo {
45-
// since [email protected]
46-
routeOptions?: {
47-
url?: string;
48-
method?: string;
49-
};
50-
routerPath?: string;
51-
}
52-
5360
/**
5461
* Setup an error handler for Fastify.
5562
*/
@@ -84,6 +91,16 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
8491

8592
fastify.register(plugin);
8693

94+
// Sadly, middleware spans do not go through `requestHook`, so we handle those here
95+
// We register this hook in this method, because if we register it in the integration `setup`,
96+
// it would always run even for users that are not even using fastify
97+
const client = getClient();
98+
if (client) {
99+
client.on('spanStart', span => {
100+
addFastifySpanAttributes(span);
101+
});
102+
}
103+
87104
if (!isWrapped(fastify.addHook) && isEnabled()) {
88105
consoleSandbox(() => {
89106
// eslint-disable-next-line no-console
@@ -93,3 +110,27 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
93110
});
94111
}
95112
}
113+
114+
function addFastifySpanAttributes(span: Span): void {
115+
const attributes = spanToJSON(span).data || {};
116+
117+
// this is one of: middleware, request_handler
118+
const type = attributes['fastify.type'];
119+
120+
// If this is already set, or we have no fastify span, no need to process again...
121+
if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
122+
return;
123+
}
124+
125+
span.setAttributes({
126+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.fastify',
127+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.fastify`,
128+
});
129+
130+
// Also update the name, we don't need to "middleware - " prefix
131+
const name = attributes['fastify.name'] || attributes['plugin.name'] || attributes['hook.name'];
132+
if (name) {
133+
// Also remove `fastify -> ` prefix
134+
span.updateName(name.replace(/^fastify -> /, ''));
135+
}
136+
}

0 commit comments

Comments
 (0)