Skip to content

Commit 984a0de

Browse files
mydeamydea
committed
fixes
1 parent 1b7e7f2 commit 984a0de

File tree

3 files changed

+85
-52
lines changed

3 files changed

+85
-52
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ import {
99
startTrackingLongTasks,
1010
startTrackingWebVitals,
1111
} from '@sentry-internal/browser-utils';
12-
import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource } from '@sentry/core';
12+
import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource} from '@sentry/core';
13+
import { dropUndefinedKeys } from '@sentry/core';
1314
import {
1415
GLOBAL_OBJ,
1516
SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
@@ -303,6 +304,22 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
303304

304305
emitFinish();
305306
}
307+
308+
// A trace should to stay the consistent over the entire time span of one route.
309+
// Therefore, we update the traceId/sampled properties on the propagation context.
310+
// The DSC is only set once the span has ended
311+
const scope = getCurrentScope();
312+
const oldPropagationContext = scope.getPropagationContext();
313+
314+
scope.setPropagationContext(
315+
dropUndefinedKeys({
316+
...oldPropagationContext,
317+
traceId: idleSpan.spanContext().traceId,
318+
sampled: spanIsSampled(idleSpan),
319+
// Reset the DSC, it should be read from the span while it is active
320+
dsc: undefined,
321+
}),
322+
);
306323
}
307324

308325
return {

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,7 @@ describe('browserTracingIntegration', () => {
658658
});
659659
expect(newCurrentScopePropCtx).toEqual({
660660
traceId: expect.stringMatching(/[a-f0-9]{32}/),
661+
sampled: false,
661662
});
662663
expect(newIsolationScopePropCtx).toEqual({
663664
traceId: expect.stringMatching(/[a-f0-9]{32}/),
@@ -674,10 +675,19 @@ describe('browserTracingIntegration', () => {
674675
expect(newCurrentScopePropCtx2).toEqual({
675676
traceId: traceId2,
676677
sampled: false,
678+
});
679+
680+
span2?.end();
681+
682+
const newCurrentScopePropCtx2After = getCurrentScope().getPropagationContext();
683+
expect(newCurrentScopePropCtx2After).toEqual({
684+
traceId: traceId2,
685+
sampled: false,
677686
dsc: {
678687
environment: 'production',
679688
public_key: 'examplePublicKey',
680689
sample_rate: '0',
690+
sampled: 'false',
681691
trace_id: traceId2,
682692
},
683693
});
@@ -701,6 +711,7 @@ describe('browserTracingIntegration', () => {
701711
const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
702712
expect(propCtxBeforeEnd).toStrictEqual({
703713
traceId: expect.stringMatching(/[a-f0-9]{32}/),
714+
sampled: true,
704715
});
705716

706717
updateSpanName(navigationSpan!, 'mySpan2');
@@ -739,6 +750,7 @@ describe('browserTracingIntegration', () => {
739750
const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
740751
expect(propCtxBeforeEnd).toStrictEqual({
741752
traceId: expect.stringMatching(/[a-f0-9]{32}/),
753+
sampled: false,
742754
});
743755

744756
navigationSpan!.end();

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

Lines changed: 55 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,59 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
109109

110110
const client = getClient();
111111

112+
function patchSpanEnd(span: Span): void {
113+
// We patch span.end to ensure we can run some things before the span is ended
114+
// eslint-disable-next-line @typescript-eslint/unbound-method
115+
span.end = new Proxy(span.end, {
116+
apply(target, thisArg, args: Parameters<Span['end']>) {
117+
if (beforeSpanEnd) {
118+
beforeSpanEnd(span);
119+
}
120+
121+
// If the span is non-recording, nothing more to do here...
122+
// This is the case if tracing is enabled but this specific span was not sampled
123+
if (thisArg instanceof SentryNonRecordingSpan) {
124+
return;
125+
}
126+
127+
// Just ensuring that this keeps working, even if we ever have more arguments here
128+
const [definedEndTimestamp, ...rest] = args;
129+
const timestamp = definedEndTimestamp || timestampInSeconds();
130+
const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
131+
132+
// Ensure we end with the last span timestamp, if possible
133+
const spans = getSpanDescendants(span).filter(child => child !== span);
134+
135+
// If we have no spans, we just end, nothing else to do here
136+
if (!spans.length) {
137+
onIdleSpanEnded(spanEndTimestamp);
138+
return Reflect.apply(target, thisArg, [spanEndTimestamp, ...rest]);
139+
}
140+
141+
const childEndTimestamps = spans
142+
.map(span => spanToJSON(span).timestamp)
143+
.filter(timestamp => !!timestamp) as number[];
144+
const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
145+
146+
// In reality this should always exist here, but type-wise it may be undefined...
147+
const spanStartTimestamp = spanToJSON(span).start_timestamp;
148+
149+
// The final endTimestamp should:
150+
// * Never be before the span start timestamp
151+
// * Be the latestSpanEndTimestamp, if there is one, and it is smaller than the passed span end timestamp
152+
// * Otherwise be the passed end timestamp
153+
// Final timestamp can never be after finalTimeout
154+
const endTimestamp = Math.min(
155+
spanStartTimestamp ? spanStartTimestamp + finalTimeout / 1000 : Infinity,
156+
Math.max(spanStartTimestamp || -Infinity, Math.min(spanEndTimestamp, latestSpanEndTimestamp || Infinity)),
157+
);
158+
159+
onIdleSpanEnded(endTimestamp);
160+
return Reflect.apply(target, thisArg, [endTimestamp, ...rest]);
161+
},
162+
});
163+
}
164+
112165
if (!client || !hasTracingEnabled()) {
113166
const span = new SentryNonRecordingSpan();
114167

@@ -118,64 +171,15 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
118171
...getDynamicSamplingContextFromSpan(span),
119172
} satisfies Partial<DynamicSamplingContext>;
120173
freezeDscOnSpan(span, dsc);
121-
174+
patchSpanEnd(span);
122175
return span;
123176
}
124177

125178
const scope = getCurrentScope();
126179
const previousActiveSpan = getActiveSpan();
127180
const span = _startIdleSpan(startSpanOptions);
128181

129-
// We patch span.end to ensure we can run some things before the span is ended
130-
// eslint-disable-next-line @typescript-eslint/unbound-method
131-
span.end = new Proxy(span.end, {
132-
apply(target, thisArg, args: Parameters<Span['end']>) {
133-
if (beforeSpanEnd) {
134-
beforeSpanEnd(span);
135-
}
136-
137-
// If the span is non-recording, nothing more to do here...
138-
// This is the case if tracing is enabled but this specific span was not sampled
139-
if (thisArg instanceof SentryNonRecordingSpan) {
140-
return;
141-
}
142-
143-
// Just ensuring that this keeps working, even if we ever have more arguments here
144-
const [definedEndTimestamp, ...rest] = args;
145-
const timestamp = definedEndTimestamp || timestampInSeconds();
146-
const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
147-
148-
// Ensure we end with the last span timestamp, if possible
149-
const spans = getSpanDescendants(span).filter(child => child !== span);
150-
151-
// If we have no spans, we just end, nothing else to do here
152-
if (!spans.length) {
153-
onIdleSpanEnded(spanEndTimestamp);
154-
return Reflect.apply(target, thisArg, [spanEndTimestamp, ...rest]);
155-
}
156-
157-
const childEndTimestamps = spans
158-
.map(span => spanToJSON(span).timestamp)
159-
.filter(timestamp => !!timestamp) as number[];
160-
const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
161-
162-
// In reality this should always exist here, but type-wise it may be undefined...
163-
const spanStartTimestamp = spanToJSON(span).start_timestamp;
164-
165-
// The final endTimestamp should:
166-
// * Never be before the span start timestamp
167-
// * Be the latestSpanEndTimestamp, if there is one, and it is smaller than the passed span end timestamp
168-
// * Otherwise be the passed end timestamp
169-
// Final timestamp can never be after finalTimeout
170-
const endTimestamp = Math.min(
171-
spanStartTimestamp ? spanStartTimestamp + finalTimeout / 1000 : Infinity,
172-
Math.max(spanStartTimestamp || -Infinity, Math.min(spanEndTimestamp, latestSpanEndTimestamp || Infinity)),
173-
);
174-
175-
onIdleSpanEnded(endTimestamp);
176-
return Reflect.apply(target, thisArg, [endTimestamp, ...rest]);
177-
},
178-
});
182+
patchSpanEnd(span);
179183

180184
/**
181185
* Cancels the existing idle timeout, if there is one.

0 commit comments

Comments
 (0)