Skip to content

Commit 3e1c45d

Browse files
mydeamydea
authored
ref: Remove deprecated properties from startSpan options (#11054)
So the public API for this is now fully aligned! In nextjs, I moved places where we passed `request` as metadata to put this on the isolation scope (in some places we already did that anyhow).
1 parent a20bf74 commit 3e1c45d

File tree

23 files changed

+49
-202
lines changed

23 files changed

+49
-202
lines changed

‎dev-packages/e2e-tests/test-applications/nextjs-14/tests/generation-functions.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ test('Should send a transaction event for a generateMetadata() function invokati
77
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
88
return (
99
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions)' &&
10-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle
10+
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
1111
);
1212
});
1313

@@ -26,8 +26,8 @@ test('Should send a transaction and an error event for a faulty generateMetadata
2626

2727
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
2828
return (
29-
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions)' &&
30-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle
29+
transactionEvent.transaction === 'Page.generateMetadata (/generation-functions)' &&
30+
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
3131
);
3232
});
3333

@@ -47,7 +47,7 @@ test('Should send a transaction event for a generateViewport() function invokati
4747
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
4848
return (
4949
transactionEvent?.transaction === 'Page.generateViewport (/generation-functions)' &&
50-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['viewportThemeColor'] === testTitle
50+
(transactionEvent.extra?.route_data as any)?.searchParams?.viewportThemeColor === testTitle
5151
);
5252
});
5353

@@ -64,7 +64,7 @@ test('Should send a transaction and an error event for a faulty generateViewport
6464
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
6565
return (
6666
transactionEvent?.transaction === 'Page.generateViewport (/generation-functions)' &&
67-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['viewportThemeColor'] === testTitle
67+
(transactionEvent.extra?.route_data as any)?.searchParams?.viewportThemeColor === testTitle
6868
);
6969
});
7070

@@ -86,7 +86,7 @@ test('Should send a transaction event with correct status for a generateMetadata
8686
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
8787
return (
8888
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-redirect)' &&
89-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle
89+
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
9090
);
9191
});
9292

@@ -103,7 +103,7 @@ test('Should send a transaction event with correct status for a generateMetadata
103103
const transactionPromise = waitForTransaction('nextjs-14', async transactionEvent => {
104104
return (
105105
transactionEvent?.transaction === 'Page.generateMetadata (/generation-functions/with-notfound)' &&
106-
transactionEvent.contexts?.trace?.data?.['searchParams']?.['metadataTitle'] === testTitle
106+
(transactionEvent.extra?.route_data as any)?.searchParams?.metadataTitle === testTitle
107107
);
108108
});
109109

‎dev-packages/e2e-tests/tracing-shim-esm/index.cjs

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎dev-packages/e2e-tests/tracing-shim-esm/index.mjs

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎dev-packages/e2e-tests/tracing-shim-esm/package.json

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎dev-packages/e2e-tests/tracing-shim/index.js

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎dev-packages/e2e-tests/tracing-shim/package.json

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎packages/core/test/lib/tracing/trace.test.ts

Lines changed: 5 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
getMainCarrier,
1111
setAsyncContextStrategy,
1212
setCurrentClient,
13-
spanIsSampled,
1413
spanToJSON,
1514
withScope,
1615
} from '../../../src';
@@ -101,33 +100,6 @@ describe('startSpan', () => {
101100
expect(spanToJSON(_span!).status).toEqual(isError ? 'internal_error' : undefined);
102101
});
103102

104-
it('allows traceparent information to be overriden', async () => {
105-
let _span: Span | undefined = undefined;
106-
client.on('spanEnd', span => {
107-
_span = span;
108-
});
109-
try {
110-
await startSpan(
111-
{
112-
name: 'GET users/[id]',
113-
parentSampled: true,
114-
traceId: '12345678901234567890123456789012',
115-
parentSpanId: '1234567890123456',
116-
},
117-
() => {
118-
return callback();
119-
},
120-
);
121-
} catch (e) {
122-
//
123-
}
124-
expect(_span).toBeDefined();
125-
126-
expect(spanIsSampled(_span!)).toEqual(true);
127-
expect(spanToJSON(_span!).trace_id).toEqual('12345678901234567890123456789012');
128-
expect(spanToJSON(_span!).parent_span_id).toEqual('1234567890123456');
129-
});
130-
131103
it('allows for transaction to be mutated', async () => {
132104
let _span: Span | undefined = undefined;
133105
client.on('spanEnd', span => {
@@ -153,7 +125,7 @@ describe('startSpan', () => {
153125
}
154126
});
155127
try {
156-
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
128+
await startSpan({ name: 'GET users/[id]' }, () => {
157129
return startSpan({ name: 'SELECT * from users' }, () => {
158130
return callback();
159131
});
@@ -179,7 +151,7 @@ describe('startSpan', () => {
179151
}
180152
});
181153
try {
182-
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
154+
await startSpan({ name: 'GET users/[id]' }, () => {
183155
return startSpan({ name: 'SELECT * from users' }, childSpan => {
184156
if (childSpan) {
185157
childSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'db.query');
@@ -453,12 +425,9 @@ describe('startSpan', () => {
453425
setCurrentClient(client);
454426
client.init();
455427

456-
startSpan(
457-
{ name: 'outer', attributes: { test1: 'aa', test2: 'aa' }, data: { test1: 'bb', test3: 'bb' } },
458-
outerSpan => {
459-
expect(outerSpan).toBeDefined();
460-
},
461-
);
428+
startSpan({ name: 'outer', attributes: { test1: 'aa', test2: 'aa', test3: 'bb' } }, outerSpan => {
429+
expect(outerSpan).toBeDefined();
430+
});
462431

463432
expect(tracesSampler).toBeCalledTimes(1);
464433
expect(tracesSampler).toHaveBeenLastCalledWith({

‎packages/ember/addon/instance-initializers/sentry-performance.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ function _instrumentEmberRunloop(config: EmberSentryConfig): void {
223223
},
224224
name: 'runloop',
225225
op: `ui.ember.runloop.${queue}`,
226-
startTimestamp: currentQueueStart,
226+
startTime: currentQueueStart,
227227
})?.end(now);
228228
}
229229
currentQueueStart = undefined;
@@ -295,7 +295,7 @@ function processComponentRenderAfter(
295295
startInactiveSpan({
296296
name: payload.containerKey || payload.object,
297297
op,
298-
startTimestamp: begin.now,
298+
startTime: begin.now,
299299
attributes: {
300300
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',
301301
},
@@ -374,17 +374,17 @@ function _instrumentInitialLoad(config: EmberSentryConfig): void {
374374
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
375375
const measure = measures[0]!;
376376

377-
const startTimestamp = (measure.startTime + browserPerformanceTimeOrigin) / 1000;
378-
const endTimestamp = startTimestamp + measure.duration / 1000;
377+
const startTime = (measure.startTime + browserPerformanceTimeOrigin) / 1000;
378+
const endTime = startTime + measure.duration / 1000;
379379

380380
startInactiveSpan({
381381
op: 'ui.ember.init',
382382
name: 'init',
383383
attributes: {
384384
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',
385385
},
386-
startTimestamp,
387-
})?.end(endTimestamp);
386+
startTime,
387+
})?.end(endTime);
388388
performance.clearMarks(startName);
389389
performance.clearMarks(endName);
390390

‎packages/nextjs/src/common/utils/edgeWrapperUtils.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
addTracingExtensions,
66
captureException,
77
continueTrace,
8+
getIsolationScope,
89
handleCallbackErrors,
910
setHttpStatus,
1011
startSpan,
@@ -39,6 +40,9 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
3940
baggage,
4041
},
4142
() => {
43+
getIsolationScope().setSDKProcessingMetadata({
44+
request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined,
45+
});
4246
return startSpan(
4347
{
4448
name: options.spanDescription,
@@ -47,9 +51,6 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
4751
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
4852
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
4953
},
50-
metadata: {
51-
request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined,
52-
},
5354
},
5455
async span => {
5556
const handlerResult = await handleCallbackErrors(

‎packages/nextjs/src/common/wrapApiHandlerWithSentry.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,6 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz
9191
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
9292
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
9393
},
94-
metadata: {
95-
request: req,
96-
},
9794
},
9895
async span => {
9996
// eslint-disable-next-line @typescript-eslint/unbound-method

‎packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
6868
{
6969
op: 'function.nextjs',
7070
name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`,
71-
data,
7271
attributes: {
7372
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
7473
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',

‎packages/nextjs/test/config/withSentry.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,13 @@ describe('withSentry', () => {
4747
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
4848
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
4949
},
50-
metadata: {
51-
request: expect.objectContaining({ url: 'http://dogs.are.great' }),
52-
},
5350
},
5451
expect.any(Function),
5552
);
53+
54+
expect(SentryCore.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({
55+
request: expect.objectContaining({ url: 'http://dogs.are.great' }),
56+
});
5657
});
5758
});
5859
});

‎packages/nextjs/test/edge/edgeWrapperUtils.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,6 @@ describe('withEdgeWrapping', () => {
8282
expect(startSpanSpy).toHaveBeenCalledTimes(1);
8383
expect(startSpanSpy).toHaveBeenCalledWith(
8484
expect.objectContaining({
85-
metadata: {
86-
request: { headers: {} },
87-
},
8885
attributes: {
8986
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
9087
[coreSdk.SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
@@ -94,6 +91,10 @@ describe('withEdgeWrapping', () => {
9491
}),
9592
expect.any(Function),
9693
);
94+
95+
expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({
96+
request: { headers: {} },
97+
});
9798
});
9899

99100
it("should return a function that doesn't crash when req isn't passed", async () => {

‎packages/nextjs/test/edge/withSentryAPI.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ describe('wrapApiHandlerWithSentry', () => {
5353
expect(startSpanSpy).toHaveBeenCalledTimes(1);
5454
expect(startSpanSpy).toHaveBeenCalledWith(
5555
expect.objectContaining({
56-
metadata: {
57-
request: { headers: {}, method: 'POST', url: 'https://sentry.io/' },
58-
},
5956
attributes: {
6057
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
6158
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
@@ -65,6 +62,10 @@ describe('wrapApiHandlerWithSentry', () => {
6562
}),
6663
expect.any(Function),
6764
);
65+
66+
expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({
67+
request: { headers: {}, method: 'POST', url: 'https://sentry.io/' },
68+
});
6869
});
6970

7071
it('should return a function that calls trace without throwing when no request is passed', async () => {
@@ -77,7 +78,6 @@ describe('wrapApiHandlerWithSentry', () => {
7778
expect(startSpanSpy).toHaveBeenCalledTimes(1);
7879
expect(startSpanSpy).toHaveBeenCalledWith(
7980
expect.objectContaining({
80-
metadata: {},
8181
attributes: {
8282
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
8383
[coreSdk.SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
@@ -87,5 +87,7 @@ describe('wrapApiHandlerWithSentry', () => {
8787
}),
8888
expect.any(Function),
8989
);
90+
91+
expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({});
9092
});
9193
});

‎packages/node/src/handlers.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,6 @@ export function tracingHandler(): (
7474
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
7575
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.node.tracingHandler',
7676
},
77-
metadata: {
78-
// The request should already have been stored in `scope.sdkProcessingMetadata` (which will become
79-
// `event.sdkProcessingMetadata` the same way the metadata here will) by `sentryRequestMiddleware`, but on the
80-
// off chance someone is using `sentryTracingMiddleware` without `sentryRequestMiddleware`, it doesn't hurt to
81-
// be sure
82-
request: req,
83-
},
8477
}) as Transaction;
8578
});
8679

‎packages/node/test/handlers.test.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as http from 'http';
22
import * as sentryCore from '@sentry/core';
33
import {
4-
Hub,
54
SEMANTIC_ATTRIBUTE_SENTRY_OP,
65
Transaction,
76
getClient,
@@ -469,24 +468,6 @@ describe('tracingHandler', () => {
469468
done();
470469
});
471470
});
472-
473-
it('stores request in transaction metadata', () => {
474-
const options = getDefaultNodeClientOptions({ tracesSampleRate: 1.0 });
475-
// eslint-disable-next-line deprecation/deprecation
476-
const hub = new Hub(new NodeClient(options));
477-
478-
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
479-
// eslint-disable-next-line deprecation/deprecation
480-
jest.spyOn(sentryCore, 'getCurrentScope').mockImplementation(() => hub.getScope());
481-
482-
sentryTracingMiddleware(req, res, next);
483-
484-
// eslint-disable-next-line deprecation/deprecation
485-
const transaction = getCurrentScope().getTransaction();
486-
487-
// eslint-disable-next-line deprecation/deprecation
488-
expect(transaction?.metadata.request).toEqual(req);
489-
});
490471
});
491472

492473
describe('errorHandler()', () => {

‎packages/opentelemetry/src/trace.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import { TraceState, suppressTracing } from '@opentelemetry/core';
66
import {
77
SDK_VERSION,
88
SEMANTIC_ATTRIBUTE_SENTRY_OP,
9-
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
109
getClient,
1110
getCurrentScope,
1211
getRootSpan,
@@ -141,16 +140,11 @@ function getTracer(): Tracer {
141140
}
142141

143142
function _applySentryAttributesToSpan(span: Span, options: OpenTelemetrySpanContext): void {
144-
// eslint-disable-next-line deprecation/deprecation
145-
const { op, source } = options;
143+
const { op } = options;
146144

147145
if (op) {
148146
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, op);
149147
}
150-
151-
if (source) {
152-
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
153-
}
154148
}
155149

156150
function getSpanContext(options: OpenTelemetrySpanContext): SpanOptions {

0 commit comments

Comments
 (0)