10000 ref(browser): Streamline pageload span creation and scope handling (#… · n4bb12/sentry-javascript@eb2f454 · GitHub
[go: up one dir, main page]
Skip to content

Commit eb2f454

Browse files
Lms24Lms24
authored
ref(browser): Streamline pageload span creation and scope handling (getsentry#11679)
Wwe can streamline our pageload span creation logic now that we actually want to keep the propagation context on the scope after the transaction finished (see getsentry#11599). Previously, we'd fork a new scope for the pageload span but IMHO (and according to all our tests) this is no longer necessary.
1 parent cf2d945 commit eb2f454

File tree

3 files changed

+56
-24
lines changed

3 files changed

+56
-24
lines changed

‎packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
1111
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
1212
TRACING_DEFAULTS,
13-
continueTrace,
1413
getActiveSpan,
1514
getClient,
1615
getCurrentScope,
@@ -21,11 +20,16 @@ import {
2120
spanIsSampled,
2221
spanToJSON,
2322
startIdleSpan,
24-
withScope,
2523
} from '@sentry/core';
2624
import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types';
2725
import type { Span } from '@sentry/types';
28-
import { browserPerformanceTimeOrigin, getDomElement, logger, uuid4 } from '@sentry/utils';
26+
import {
27+
browserPerformanceTimeOrigin,
28+
getDomElement,
29+
logger,
30+
propagationContextFromHeaders,
31+
uuid4,
32+
} from '@sentry/utils';
2933

3034
import { DEBUG_BUILD } from '../debug-build';
3135
import { WINDOW } from '../helpers';
@@ -263,31 +267,19 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
263267
const sentryTrace = traceOptions.sentryTrace || getMetaContent('sentry-trace');
264268
const baggage = traceOptions.baggage || getMetaContent('baggage');
265269

266-
// Continue trace updates the scope in the callback only, but we want to break out of it again...
267-
// This is a bit hacky, because we want to get the span to use both the correct scope _and_ the correct propagation context
268-
// but afterwards, we want to reset it to avoid this also applying to other spans
269-
const scope = getCurrentScope();
270+
const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
271+
getCurrentScope().setPropagationContext(propagationContext);
270272

271-
activeSpan = continueTrace({ sentryTrace, baggage }, () => {
272-
// We update the outer current scope to have the correct propagation context
273-
// this means, the scope active when the pageload span is created will continue to hold the
274-
// propagationContext from the incoming trace, even after the pageload span ended.
275-
scope.setPropagationContext(getCurrentScope().getPropagationContext());
276-
277-
// Ensure we are on the original current scope again, so the span is set as active on it
278-
return withScope(scope, 8000 () => {
279-
return _createRouteSpan(client, {
280-
op: 'pageload',
281-
...startSpanOptions,
282-
});
283-
});
273+
activeSpan = _createRouteSpan(client, {
274+
op: 'pageload',
275+
...startSpanOptions,
284276
});
285277
});
286278

287279
// A trace should to stay the consistent over the entire time span of one route.
288-
// Therefore, when the initial pageload or navigation transaction ends, we update the
280+
// Therefore, when the initial pageload or navigation root span ends, we update the
289281
// scope's propagation context to keep span-specific attributes like the `sampled` decision and
290-
// the dynamic sampling context valid, even after the transaction has ended.
282+
// the dynamic sampling context valid, even after the root span has ended.
291283
// This ensures that the trace data is consistent for the entire duration of the route.
292284
client.on('spanEnd', span => {
293285
const op = spanToJSON(span).op;

‎packages/browser/test/unit/tracing/browserTracingIntegration.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ describe('browserTracingIntegration', () => {
676676
expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId);
677677
});
678678

679-
it("saves the span's sampling decision and its DSC on the propagationContext when the span finishes", () => {
679+
it("saves the span's positive sampling decision and its DSC on the propagationContext when the span finishes", () => {
680680
const client = new BrowserClient(
681681
getDefaultBrowserClientOptions({
682682
tracesSampleRate: 1,
@@ -714,6 +714,45 @@ describe('browserTracingIntegration', () => {
714714
},
715715
});
716716
});
717+
718+
it("saves the span's negative sampling decision and its DSC on the propagationContext when the span finishes", () => {
719+
const client = new BrowserClient(
720+
getDefaultBrowserClientOptions({
721+
tracesSampleRate: 0,
722+
integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
723+
}),
724+
);
725+
setCurrentClient(client);
726+
client.init();
727+
728+
const navigationSpan = startBrowserTracingNavigationSpan(client, {
729+
name: 'mySpan',
730+
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' },
731+
});
732+
733+
const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
734+
expect(propCtxBeforeEnd).toStrictEqual({
735+
spanId: expect.stringMatching(/[a-f0-9]{16}/),
736+
traceId: expect.stringMatching(/[a-f0-9]{32}/),
737+
});
738+
739+
navigationSpan!.end();
740+
741+
const propCtxAfterEnd = getCurrentScope().getPropagationContext();
742+
expect(propCtxAfterEnd).toStrictEqual({
743+
spanId: propCtxBeforeEnd?.spanId,
744+
traceId: propCtxBeforeEnd?.traceId,
745+
sampled: false,
746+
dsc: {
747+
environment: 'production',
748+
public_key: 'examplePublicKey',
749+
sample_rate: '0',
750+
sampled: 'false',
751+
transaction: 'mySpan',
752+
trace_id: propCtxBeforeEnd?.traceId,
753+
},
754+
});
755+
});
717756
});
718757

719758
describe('using the <meta> tag data', () => {

‎packages/utils/src/tracing.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export function extractTraceparentData(traceparent?: string): TraceparentData |
4444
}
4545

4646
/**
47-
* Create a propagation context from incoming headers.
47+
* Create a propagation context from incoming headers or
48+
* creates a minimal new one if the headers are undefined.
4849
*/
4950
export function propagationContextFromHeaders(
5051
sentryTrace: string | undefined,

0 commit comments

Comments
 (0)
0