Skip to content

Commit cea476c

Browse files
Lms24Lms24
committed
node tests and jsdoc
1 parent d29258e commit cea476c

File tree

12 files changed

+265
-12
lines changed

12 files changed

+265
-12
lines changed

‎dev-packages/node-integration-tests/suites/express/tracing/updateName/server.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
// disable attaching headers to /test/* endpoints
8+
tracePropagationTargets: [/^(?!.*test).*$/],
9+
tracesSampleRate: 1.0,
10+
transport: loggingTransport,
11+
});
12+
13+
// express must be required after Sentry is initialized
14+
const express = require('express');
15+
const cors = require('cors');
16+
const bodyParser = require('body-parser');
17+
const { startExpressServerAndSendPortToRunner } = require('@sentry-internal/node-integration-tests');
18+
19+
const app = express();
20+
21+
app.use(cors());
22+
app.use(bodyParser.json());
23+
app.use(bodyParser.text());
24+
app.use(bodyParser.raw());
25+
26+
app.get('/test/:id/span-updateName', (_req, res) => {
27+
const span = Sentry.getActiveSpan();
28+
const rootSpan = Sentry.getRootSpan(span);
29+
rootSpan.updateName('new-name');
30+
res.send({ response: 'response 1' });
31+
});
32+
33+
app.get('/test/:id/span-updateName-source', (_req, res) => {
34+
const span = Sentry.getActiveSpan();
35+
const rootSpan = Sentry.getRootSpan(span);
36+
rootSpan.updateName('new-name');
37+
rootSpan.setAttribute(Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
38+
res.send({ response: 'response 2' });
39+
});
40+
41+
app.get('/test/:id/updateSpanName', (_req, res) => {
42+
const span = Sentry.getActiveSpan();
43+
const rootSpan = Sentry.getRootSpan(span);
44+
Sentry.updateSpanName(rootSpan, 'new-name');
45+
res.send({ response: 'response 3' });
46+
});
47+
48+
Sentry.setupExpressErrorHandler(app);
49+
50+
startExpressServerAndSendPortToRunner(app);

‎dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
2+
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
3+
4+
describe('express tracing', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
9+
describe('CJS', () => {
10+
// This test documents the unfortunate behaviour of using `span.updateName` on the server-side.
11+
// For http.server root spans (which is the root span on the server 99% of the time), Otel's http instrumentation
12+
// calls `span.updateName` and overwrites whatever the name was set to before (by us or by users).
13+
test("calling just `span.updateName` doesn't update the final name in express (missing source)", done => {
14+
createRunner(__dirname, 'server.js')
15+
.expect({
16+
transaction: {
17+
transaction: 'GET /test/:id/span-updateName',
18+
transaction_info: {
19+
source: 'route',
20+
},
21+
},
22+
})
23+
.start(done)
24+
.makeRequest('get', '/test/123/span-updateName');
25+
});
26+
27+
// Also calling `updateName` AND setting a source doesn't change anything - Otel has no concept of source, this is sentry-internal.
28+
// Therefore, only the source is updated but the name is still overwritten by Otel.
29+
test("calling `span.updateName` and setting attribute source doesn't update the final name in express but it updates the source", done => {
30+
createRunner(__dirname, 'server.js')
31+
.expect({
32+
transaction: {
33+
transaction: 'GET /test/:id/span-updateName-source',
34+
transaction_info: {
35+
source: 'custom',
36+
},
37+
},
38+
})
39+
.start(done)
40+
.makeRequest('get', '/test/123/span-updateName-source');
41+
});
42+
43+
// This test documents the correct way to update the span name (and implicitly the source) in Node:
44+
test('calling `Sentry.updateSpanName` updates the final name and source in express', done => {
45+
createRunner(__dirname, 'server.js')
46+
.expect({
47+
transaction: txnEvent => {
48+
expect(txnEvent).toMatchObject({
49+
transaction: 'new-name',
50+
transaction_info: {
51+
source: 'custom',
52+
},
53+
contexts: {
54+
trace: {
55+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
56+
},
57+
},
58+
});
59+
// ensure we delete the internal attribute once we're done with it
60+
expect(txnEvent.contexts?.trace?.data?.['_sentry_span_name_set_by_user']).toBeUndefined();
61+
},
62+
})
63+
.start(done)
64+
.makeRequest('get', '/test/123/updateSpanName');
65+
});
66+
});
67+
});

‎dev-packages/node-integration-tests/suites/public-api/startSpan/basic-usage/test.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,24 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
12
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
23

34
afterAll(() => {
45
cleanupChildProcesses();
56
});
67

7-
test('should send a manually started root span', done => {
8+
test('sends a manually started root span with source custom', done => {
89
createRunner(__dirname, 'scenario.ts')
9-
.expect({ transaction: { transaction: 'test_span' } })
10+
.expect({
11+
transaction: {
12+
transaction: 'test_span',
13+
transaction_info: { source: 'custom' },
14+
contexts: {
15+
trace: {
16+
span_id: expect.any(String),
17+
trace_id: expect.any(String),
18+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
19+
},
20+
},
21+
},
22+
})
1023
.start(done);
1124
});

‎dev-packages/node-integration-tests/suites/public-api/startSpan/updateName-method/scenario.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
});
10+
11+
Sentry.startSpan(
12+
{ name: 'test_span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } },
13+
(span: Sentry.Span) => {
14+
span.updateName('new name');
15+
},
16+
);

‎dev-packages/node-integration-tests/suites/public-api/startSpan/updateName-method/test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
2+
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
test('updates the span name when calling `span.updateName`', done => {
9+
createRunner(__dirname, 'scenario.ts')
10+
.expect({
11+
transaction: {
12+
transaction: 'new name',
13+
transaction_info: { source: 'url' },
14+
contexts: {
15+
trace: {
16+
span_id: expect.any(String),
17+
trace_id: expect.any(String),
18+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' },
19+
},
20+
},
21+
},
22+
})
23+
.start(done);
24+
});

‎dev-packages/node-integration-tests/suites/public-api/startSpan/updateSpanName-function/scenario.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://[email protected]/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
});
10+
11+
Sentry.startSpan(
12+
{ name: 'test_span', attributes: { [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } },
13+
(span: Sentry.Span) => {
14+
Sentry.updateSpanName(span, 'new name');
15+
},
16+
);

‎dev-packages/node-integration-tests/suites/public-api/startSpan/updateSpanName-function/test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/node';
2+
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
3+
4+
afterAll(() => {
5+
cleanupChildProcesses();
6+
});
7+
8+
test('updates the span name and source when calling `updateSpanName`', done => {
9+
createRunner(__dirname, 'scenario.ts')
10+
.expect({
11+
transaction: {
12+
transaction: 'new name',
13+
transaction_info: { source: 'custom' },
14+
contexts: {
15+
trace: {
16+
span_id: expect.any(String),
17+
trace_id: expect.any(String),
18+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' },
19+
},
20+
},
21+
},
22+
})
23+
.start(done);
24+
});

‎dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,4 +169,10 @@ describe('httpIntegration', () => {
169169
runner.makeRequest('get', '/testRequest');
170170
});
171171
});
172+
173+
describe('does not override the span name set by the user', () => {
174+
test('via `span.updateName`', done => {
175+
createRunner(__dirname, 'server-updateName.js').start(done);
176+
});
177+
});
172178
});

‎packages/core/src/envelope.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ export function createEventEnvelope(
9595
// have temporarily added, etc. Even if we don't happen to be using it at some point in the future, let's not get rid
9696
// of this `delete`, lest we miss putting it back in the next time the property is in use.)
9797
delete event.sdkProcessingMetadata;
98+
try {
99+
// @ts-expect-error - for bundle size we try/catch the access to this property
100+
delete event.contexts.trace.data._sentry_span_name_set_by_user;
101+
// @ts-expect-error - for bundle size we try/catch the access to this property
102+
event.spans.forEach(span => delete span.data._sentry_span_name_set_by_user);
103+
} catch {
104+
// Do nothing
105+
}
98106

99107
const eventItem: EventItem = [{ type: eventType }, event];
100108
return createEnvelope<EventEnvelope>(envelopeHeaders, [eventItem]);

‎packages/core/src/types-hoist/span.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,22 @@ export interface Span {
234234

235235
/**
236236
* Update the name of the span.
237+
*
238+
* **Important:** You most likely want to use `Sentry.updateSpanName(span, name)` instead!
239+
*
240+
* This method will update the current span name but cannot guarantee that the new name will be
241+
* the final name of the span. Instrumentation might still overwrite the name with an automatically
242+
* computed name, for example in `http.server` or `db` spans.
243+
*
244+
* You can ensure that your name is kept and not overwritten by
245+
* - either calling `Sentry.updateSpanName(span, name)`
246+
* - or by calling `span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom')`
247+
* in addition to `span.updateName`.
248+
*
249+
* If you want to update a span name in a browser-only app, `span.updateName` and `Sentry.updateSpanName`
250+
* are identical: In both cases, the name will not be overwritten by the Sentry SDK.
251+
*
252+
* @param name the new name of the span
237253
*/
238254
updateName(name: string): this;
239255

‎packages/core/src/utils/spanUtils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,4 +334,5 @@ export function showSpanDropWarning(): void {
334334
export function updateSpanName(span: Span, name: string): void {
335335
span.updateName(name);
336336
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
337+
span.setAttribute('_sentry_span_name_set_by_user', name);
337338
}

‎packages/opentelemetry/src/utils/parseSpanDescription.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ interface SpanDescription {
3737
/**
3838
* Infer the op & description for a set of name, attributes and kind of a span.
3939
*/
40-
export function inferSpanData(originalName: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
40+
export function inferSpanData(spanName: string, attributes: SpanAttributes, kind: SpanKind): SpanDescription {
4141
// if http.method exists, this is an http request span
4242
// eslint-disable-next-line deprecation/deprecation
4343
const httpMethod = attributes[ATTR_HTTP_REQUEST_METHOD] || attributes[SEMATTRS_HTTP_METHOD];
4444
if (httpMethod) {
45-
return descriptionForHttpMethod({ attributes, name: originalName, kind }, httpMethod);
45+
return descriptionForHttpMethod({ attributes, name: getOriginalName(spanName, attributes), kind }, httpMethod);
4646
}
4747

4848
// eslint-disable-next-line deprecation/deprecation
@@ -54,7 +54,7 @@ export function inferSpanData(originalName: string, attributes: SpanAttributes,
5454
// If db.type exists then this is a database call span
5555
// If the Redis DB is used as a cache, the span description should not be changed
5656
if (dbSystem && !opIsCache) {
57-
return descriptionForDbSystem({ attributes, name: originalName });
57+
return descriptionForDbSystem({ attributes, name: spanName });
5858
}
5959

6060
const customSourceOrRoute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' ? 'custom' : 'route';
@@ -65,7 +65,7 @@ export function inferSpanData(originalName: string, attributes: SpanAttributes,
6565
if (rpcService) {
6666
return {
6767
op: 'rpc',
68-
description: originalName,
68+
description: getOriginalName(spanName, attributes),
6969
source: customSourceOrRoute,
7070
};
7171
}
@@ -76,7 +76,7 @@ export function inferSpanData(originalName: string, attributes: SpanAttributes,
7676
if (messagingSystem) {
7777
return {
7878
op: 'message',
79-
description: originalName,
79+
description: getOriginalName(spanName, attributes),
8080
source: customSourceOrRoute,
8181
};
8282
}
@@ -85,10 +85,14 @@ export function inferSpanData(originalName: string, attributes: SpanAttributes,
8585
// eslint-disable-next-line deprecation/deprecation
8686
const faasTrigger = attributes[SEMATTRS_FAAS_TRIGGER];
8787
if (faasTrigger) {
88-
return { op: faasTrigger.toString(), description: originalName, source: customSourceOrRoute };
88+
return {
89+
op: faasTrigger.toString(),
90+
description: getOriginalName(spanName, attributes),
91+
source: customSourceOrRoute,
92+
};
8993
}
9094

91-
return { op: undefined, description: originalName, source: 'custom' };
95+
return { op: undefined, description: spanName, source: 'custom' };
9296
}
9397

9498
/**
@@ -111,7 +115,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
111115
function descriptionForDbSystem({ attributes, name }: { attributes: Attributes; name: string }): SpanDescription {
112116
// if we already set the source to custom, we don't overwrite the span description but just adjust the op
113117
if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom') {
114-
return { op: 'db', description: name, source: 'custom' };
118+
return { op: 'db', description: getOriginalName(name, attributes), source: 'custom' };
115119
}
116120

117121
// Use DB statement (Ex "SELECT * FROM table") if possible as description.
@@ -147,7 +151,7 @@ export function descriptionForHttpMethod(
147151
const { urlPath, url, query, fragment, hasRoute } = getSanitizedUrl(attributes, kind);
148152

149153
if (!urlPath) {
150-
return { op: opParts.join('.'), description: name, source: 'custom' };
154+
return { op: opParts.join('.'), description: getOriginalName(name, attributes), source: 'custom' };
151155
}
152156

153157
const graphqlOperationsAttribute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION];
@@ -193,7 +197,7 @@ export function descriptionForHttpMethod(
193197

194198
return {
195199
op: opParts.join('.'),
196-
description: useInferredDescription ? description : name,
200+
description: useInferredDescription ? description : getOriginalName(name, attributes),
197201
source: useInferredDescription ? source : 'custom',
198202
data,
199203
};
@@ -259,3 +263,11 @@ export function getSanitizedUrl(
259263

260264
return { urlPath: undefined, url, query, fragment, hasRoute: false };
261265
}
266+
267+
function getOriginalName(name: string, attributes: Attributes): string {
268+
return attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'custom' &&
269+
attributes['_sentry_span_name_set_by_user'] &&
270+
typeof attributes['_sentry_span_name_set_by_user'] === 'string'
271+
? attributes['_sentry_span_name_set_by_user']
272+
: name;
273+
}

0 commit comments

Comments
 (0)