8000 feat(core): Update `continueTrace` to be callback-only (#11044) · jonator/sentry-javascript@2a1a036 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2a1a036

Browse files
mydeamydea
authored
feat(core): Update continueTrace to be callback-only (getsentry#11044)
Also update it to only update the propagation context in the callback. In some places (old node...) we need the legacy behaviour, but for now we can do that in-place.
1 parent 40c847c commit 2a1a036

File tree

6 files changed

+62
-223
lines changed

6 files changed

+62
-223
lines changed

‎packages/bun/src/integrations/bunserver.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,12 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
8686

8787
return continueTrace(
8888
{ sentryTrace: request.headers.get('sentry-trace') || '', baggage: request.headers.get('baggage') },
89-
ctx => {
89+
() => {
9090
return startSpan(
9191
{
9292
attributes,
9393
op: 'http.server',
9494
name: `${request.method} ${parsedUrl.path || '/'}`,
95-
...ctx,
9695
},
9796
async span => {
9897
try {

‎packages/core/src/tracing/trace.ts

Lines changed: 18 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import type {
88
TransactionContext,
99
} from '@sentry/types';
1010

11-
import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
11+
import { propagationContextFromHeaders } from '@sentry/utils';
1212
import type { AsyncContextStrategy } from '../asyncContext';
1313
import { getMainCarrier } from '../asyncContext';
1414
import { getClient, getCurrentScope, getIsolationScope, withScope } from '../currentScopes';
1515

16-
import { DEBUG_BUILD } from '../debug-build';
1716
import { getAsyncContextStrategy, getCurrentHub } from '../hub';
1817
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
1918
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
@@ -179,105 +178,28 @@ export function startInactiveSpan(context: StartSpanOptions): Span {
179178
});
180179
}
181180

182-
interface ContinueTrace {
183-
/**
184-
* Continue a trace from `sentry-trace` and `baggage` values.
185-
* These values can be obtained from incoming request headers,
186-
* or in the browser from `<meta name="sentry-trace">` and `<meta name="baggage">` HTML tags.
187-
*
188-
* @deprecated Use the version of this function taking a callback as second parameter instead:
189-
*
190-
* ```
191-
* Sentry.continueTrace(sentryTrace: '...', baggage: '...' }, () => {
192-
* // ...
193-
* })
194-
* ```
195-
*
196-
*/
197-
({
198-
sentryTrace,
199-
baggage,
200-
}: {
201-
// eslint-disable-next-line deprecation/deprecation
202-
sentryTrace: Parameters<typeof tracingContextFromHeaders>[0];
203-
// eslint-disable-next-line deprecation/deprecation
204-
baggage: Parameters<typeof tracingContextFromHeaders>[1];
205-
}): Partial<TransactionContext>;
206-
207-
/**
208-
* Continue a trace from `sentry-trace` and `baggage` values.
209-
* These values can be obtained from incoming request headers, or in the browser from `<meta name="sentry-trace">`
210-
* and `<meta name="baggage">` HTML tags.
211-
*
212-
* Spans started with `startSpan`, `startSpanManual` and `startInactiveSpan`, within the callback will automatically
213-
* be attached to the incoming trace.
214-
*
215-
* Deprecation notice: In the next major version of the SDK the provided callback will not receive a transaction
216-
* context argument.
217-
*/
218-
<V>(
219-
{
220-
sentryTrace,
221-
baggage,
222-
}: {
223-
// eslint-disable-next-line deprecation/deprecation
224-
sentryTrace: Parameters<typeof tracingContextFromHeaders>[0];
225-
// eslint-disable-next-line deprecation/deprecation
226-
baggage: Parameters<typeof tracingContextFromHeaders>[1];
227-
},
228-
// TODO(v8): Remove parameter from this callback.
229-
callback: (transactionContext: Partial<TransactionContext>) => V,
230-
): V;
231-
}
232-
233-
export const continueTrace: ContinueTrace = <V>(
181+
/**
182+
* Continue a trace from `sentry-trace` and `baggage` values.
183+
* These values can be obtained from incoming request headers, or in the browser from `<meta name="sentry-trace">`
184+
* and `<meta name="baggage">` HTML tags.
185+
*
186+
* Spans started with `startSpan`, `startSpanManual` and `startInactiveSpan`, within the callback will automatically
187+
* be attached to the incoming trace.
188+
*/
189+
export const continueTrace = <V>(
234190
{
235191
sentryTrace,
236192
baggage,
237193
}: {
238-
// eslint-disable-next-line deprecation/deprecation
239-
sentryTrace: Parameters<typeof tracingContextFromHeaders>[0];
240-
// eslint-disable-next-line deprecation/deprecation
241-
baggage: Parameters<typeof tracingContextFromHeaders>[1];
194+
sentryTrace: Parameters<typeof propagationContextFromHeaders>[0];
195+
baggage: Parameters<typeof propagationContextFromHeaders>[1];
242196
},
243-
callback?: (transactionContext: Partial<TransactionContext>) => V,
244-
): V | Partial<TransactionContext> => {
245-
// TODO(v8): Change this function so it doesn't do anything besides setting the propagation context on the current scope:
246-
/*
247-
return withScope((scope) => {
248-
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
249-
scope.setPropagationContext(propagationContext);
250-
return callback();
251-
})
252-
*/
253-
254-
const currentScope = getCurrentScope();
255-
256-
// eslint-disable-next-line deprecation/deprecation
257-
const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
258-
sentryTrace,
259-
baggage,
260-
);
261-
262-
currentScope.setPropagationContext(propagationContext);
263-
264-
if (DEBUG_BUILD && traceparentData) {
265-
logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
266-
}
267-
268-
const transactionContext: Partial<TransactionContext> = {
269-
...traceparentData,
270-
metadata: dropUndefinedKeys({
271-
dynamicSamplingContext,
272-
}),
273-
};
274-
275-
if (!callback) {
276-
return transactionContext;
277-
}
278-
279-
return withScope(() => {
280-
return callback(transactionContext);
197+
callback: () => V,
198+
): V => {
199+
return withScope(scope => {
200+
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
201+
scope.setPropagationContext(propagationContext);
202+
return callback();
281203
});
282204
};
283205

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

Lines changed: 14 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,19 +1112,10 @@ describe('continueTrace', () => {
11121112
});
11131113

11141114
it('works without trace & baggage data', () => {
1115-
const expectedContext = {
1116-
metadata: {},
1117-
};
1118-
1119-
const result = continueTrace({ sentryTrace: undefined, baggage: undefined }, ctx => {
1120-
expect(ctx).toEqual(expectedContext);
1121-
return ctx;
1115+
const scope = continueTrace({ sentryTrace: undefined, baggage: undefined }, () => {
1116+
return getCurrentScope();
11221117
});
11231118

1124-
expect(result).toEqual(expectedContext);
1125-
1126-
const scope = getCurrentScope();
1127-
11281119
expect(scope.getPropagationContext()).toEqual({
11291120
sampled: undefined,
11301121
spanId: expect.any(String),
@@ -1135,30 +1126,16 @@ describe('continueTrace', () => {
11351126
});
11361127

11371128
it('works with trace data', () => {
1138-
const expectedContext = {
1139-
metadata: {
1140-
dynamicSamplingContext: {},
1141-
},
1142-
parentSampled: false,
1143-
parentSpanId: '1121201211212012',
1144-
traceId: '12312012123120121231201212312012',
1145-
};
1146-
1147-
const result = continueTrace(
1129+
const scope = continueTrace(
11481130
{
11491131
sentryTrace: '12312012123120121231201212312012-1121201211212012-0',
11501132
baggage: undefined,
11511133
},
1152-
ctx => {
1153-
expect(ctx).toEqual(expectedContext);
1154-
return ctx;
1134+
() => {
1135+
return getCurrentScope();
11551136
},
11561137
);
11571138

1158-
expect(result).toEqual(expectedContext);
1159-
1160-
const scope = getCurrentScope();
1161-
11621139
expect(scope.getPropagationContext()).toEqual({
11631140
dsc: {}, // DSC should be an empty object (frozen), because there was an incoming trace
11641141
sampled: false,
@@ -1171,33 +1148,16 @@ describe('continueTrace', () => {
11711148
});
11721149

11731150
it('works with trace & baggage data', () => {
1174-
const expectedContext = {
1175-
metadata: {
1176-
dynamicSamplingContext: {
1177-
environment: 'production',
1178-
version: '1.0',
1179-
},
1180-
},
1181-
parentSampled: true,
1182-
parentSpanId: '1121201211212012',
1183-
traceId: '12312012123120121231201212312012',
1184-
};
1185-
1186-
const result = continueTrace(
1151+
const scope = continueTrace(
11871152
{
11881153
sentryTrace: '12312012123120121231201212312012-1121201211212012-1',
11891154
baggage: 'sentry-version=1.0,sentry-environment=production',
11901155
},
1191-
ctx => {
1192 10000 -
expect(ctx).toEqual(expectedContext);
1193-
return ctx;
1156+
() => {
1157+
return getCurrentScope();
11941158
},
11951159
);
11961160

1197-
expect(result).toEqual(expectedContext);
1198-
1199-
const scope = getCurrentScope();
1200-
12011161
expect(scope.getPropagationContext()).toEqual({
12021162
dsc: {
12031163
environment: 'production',
@@ -1213,33 +1173,16 @@ describe('continueTrace', () => {
12131173
});
12141174

12151175
it('works with trace & 3rd party baggage data', () => {
1216-
const expectedContext = {
1217-
metadata: {
1218-
dynamicSamplingContext: {
1219-
environment: 'production',
1220-
version: '1.0',
1221-
},
1222-
},
1223-
parentSampled: true,
1224-
parentSpanId: '1121201211212012',
1225-
traceId: '12312012123120121231201212312012',
1226-
};
1227-
1228-
const result = continueTrace(
1176+
const scope = continueTrace(
12291177
{
12301178
sentryTrace: '12312012123120121231201212312012-1121201211212012-1',
12311179
baggage: 'sentry-version=1.0,sentry-environment=production,dogs=great,cats=boring',
12321180
},
1233-
ctx => {
1234-
expect(ctx).toEqual(expectedContext);
1235-
return ctx;
1181+
() => {
1182+
return getCurrentScope();
12361183
},
12371184
);
12381185

1239-
expect(result).toEqual(expectedContext);
1240-
1241-
const scope = getCurrentScope();
1242-
12431186
expect(scope.getPropagationContext()).toEqual({
12441187
dsc: {
12451188
environment: 'production',
@@ -1255,45 +1198,17 @@ describe('continueTrace', () => {
12551198
});
12561199

12571200
it('returns response of callback', () => {
1258-
const expectedContext = {
1259-
metadata: {
1260-
dynamicSamplingContext: {},
1261-
},
1262-
parentSampled: false,
1263-
parentSpanId: '1121201211212012',
1264-
traceId: '12312012123120121231201212312012',
1265-
};
1266-
12671201
const result = continueTrace(
12681202
{
12691203
sentryTrace: '12312012123120121231201212312012-1121201211212012-0',
12701204
baggage: undefined,
12711205
},
1272-
ctx => {
1273-
return { ctx };
1206+
() => {
1207+
return 'aha';
12741208
},
12751209
);
12761210

1277-
expect(result).toEqual({ ctx: expectedContext });
1278-
});
1279-
1280-
it('works without a callback', () => {
1281-
const expectedContext = {
1282-
metadata: {
1283-
dynamicSamplingContext: {},
1284-
},
1285-
parentSampled: false,
1286-
parentSpanId: '1121201211212012',
1287-
traceId: '12312012123120121231201212312012',
1288-
};
1289-
1290-
// eslint-disable-next-line deprecation/deprecation
1291-
const ctx = continueTrace({
1292-
sentryTrace: '12312012123120121231201212312012-1121201211212012-0',
1293-
baggage: undefined,
1294-
});
1295-
1296-
expect(ctx).toEqual(expectedContext);
1211+
expect(result).toEqual('aha');
12971212
});
12981213
});
12991214

‎packages/node/src/handlers.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -58,34 +58,34 @@ export function tracingHandler(): (
5858
return next();
5959
}
6060

61+
// We depend here on the fact that we update the current scope...
62+
// so we keep this legacy behavior here for now
63+
const scope = getCurrentScope();
64+
6165
const [name, source] = extractPathForTransaction(req, { path: true, method: true });
62-
const transaction = continueTrace(
63-
{ sentryTrace, baggage },
64-
ctx =>
65-
startInactiveSpan({
66-
name,
67-
op: 'http.server',
68-
origin: 'auto.http.node.tracingHandler',
69-
forceTransaction: true,
70-
...ctx,
71-
attributes: {
72-
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
73-
},
74-
metadata: {
75-
// eslint-disable-next-line deprecation/deprecation
76-
...ctx.metadata,
77-
// The request should already have been stored in `scope.sdkProcessingMetadata` (which will become
78-
// `event.sdkProcessingMetadata` the same way the metadata here will) by `sentryRequestMiddleware`, but on the
79-
// off chance someone is using `sentryTracingMiddleware` without `sentryRequestMiddleware`, it doesn't hurt to
80-
// be sure
81-
request: req,
82-
},
83-
}) as Transaction,
84-
);
66+
const transaction = continueTrace({ sentryTrace, baggage }, () => {
67+
scope.setPropagationContext(getCurrentScope().getPropagationContext());
68+
return startInactiveSpan({
69+
name,
70+
op: 'http.server',
71+
origin: 'auto.http.node.tracingHandler',
72+
forceTransaction: true,
73+
attributes: {
74+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
75+
},
76+
metadata: {
77+
// The request should already have been stored in `scope.sdkProcessingMetadata` (which will become
78+
// `event.sdkProcessingMetadata` the same way the metadata here will) by `sentryRequestMiddleware`, but on the
79+
// off chance someone is using `sentryTracingMiddleware` without `sentryRequestMiddleware`, it doesn't hurt to
80+
// be sure
81+
request: req,
82+
},
83+
}) as Transaction;
84+
});
8585

8686
// We put the transaction on the scope so users can attach children to it
8787
// eslint-disable-next-line deprecation/deprecation
88-
getCurrentScope().setSpan(transaction);
88+
scope.setSpan(transaction);
8989

9090
// We also set __sentry_transaction on the response so people can grab the transaction there to add
9191
// spans to it later.

0 commit comments

Comments
 (0)
0