Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ export const continueTrace = <V>(
return withScope(scope => {
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
scope.setPropagationContext(propagationContext);
_setSpanForScope(scope, undefined);
return callback();
});
};
Expand Down
69 changes: 69 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
getGlobalScope,
getIsolationScope,
getMainCarrier,
getTraceData,
Scope,
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
Expand Down Expand Up @@ -2021,6 +2022,74 @@ describe('continueTrace', () => {
});
});
});

it('updates the propagation context when called inside an active span', () => {
const client = new TestClient(
getDefaultTestClientOptions({
dsn: 'https://username@domain/123',
tracesSampleRate: 1,
}),
);
setCurrentClient(client);
client.init();

const sentryTrace = '12312012123120121231201212312012-1121201211212012-1';
const sentryTraceId = '12312012123120121231201212312012';
const sentryBaggage = 'sentry-org_id=123';

startSpan({ name: 'outer' }, () => {
continueTrace(
{
sentryTrace: sentryTrace,
baggage: sentryBaggage,
},
() => {
const traceDataInContinuedTrace = getTraceData();
const traceIdInContinuedTrace = traceDataInContinuedTrace['sentry-trace']?.split('-')[0];
const traceIdInCurrentScope = getCurrentScope().getPropagationContext().traceId;

expect(getActiveSpan()).toBeUndefined();
expect(traceIdInContinuedTrace).toBe(sentryTraceId);
expect(traceIdInCurrentScope).toBe(sentryTraceId);
},
);
});
});
Comment on lines +2056 to +2057
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: Let's also add a second test that shows that when you now start another span within the continueTrace callback, it has the traceId and parentSpanId that's passed to continueTrace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, added!


it('sets the correct trace and parent span ids when called inside an active span and a new span is started from within the callback', () => {
const client = new TestClient(
getDefaultTestClientOptions({
dsn: 'https://username@domain/123',
tracesSampleRate: 1,
}),
);
setCurrentClient(client);
client.init();

const sentryTrace = '12312012123120121231201212312012-1121201211212012-1';
const sentryTraceId = '12312012123120121231201212312012';
const sentrySpanId = '1121201211212012';
const sentryBaggage = 'sentry-org_id=123';

startSpan({ name: 'outer' }, () => {
continueTrace(
{
sentryTrace: sentryTrace,
baggage: sentryBaggage,
},
() => {
startSpan({ name: 'inner' }, span => {
const innerSpanJson = spanToJSON(span);
const innerTraceId = innerSpanJson.trace_id;
const innerParentSpanId = innerSpanJson.parent_span_id;

expect(innerTraceId).toBe(sentryTraceId);
expect(innerParentSpanId).toBe(sentrySpanId);
});
},
);
});
});
});

describe('getActiveSpan', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/test/server-common/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe('sentryHandle', () => {
resolve: async _ => {
// simulating a nested load call:
await sentryHandle()({
event: mockEvent({ route: { id: 'api/users/details/[id]', isSubRequest: true } }),
event: mockEvent({ route: { id: 'api/users/details/[id]' }, isSubRequest: true }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, and very interesting that this passed beforehand 🤔 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it is! didn't investigate further though

resolve: resolve(type, isError),
});
return mockResponse;
Expand Down