Skip to content

Commit 709869e

Browse files
committed
feat(browser): Add option to sample linked traces consistently
1 parent ae8e59e commit 709869e

File tree

8 files changed

+264
-28
lines changed

8 files changed

+264
-28
lines changed

dev-packages/browser-integration-tests/utils/helpers.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
/* eslint-disable max-lines */
12
import type { Page, Request } from '@playwright/test';
23
import type {
4+
ClientReport,
35
Envelope,
46
EnvelopeItem,
57
EnvelopeItemType,
@@ -254,6 +256,31 @@ export function waitForTransactionRequest(
254256
});
255257
}
256258

259+
export function waitForClientReportRequest(page: Page, callback?: (report: ClientReport) => boolean): Promise<Request> {
260+
return page.waitForRequest(req => {
261+
const postData = req.postData();
262+
if (!postData) {
263+
return false;
264+
}
265+
266+
try {
267+
const maybeReport = envelopeRequestParser<Partial<ClientReport>>(req);
268+
269+
if (typeof maybeReport.discarded_events !== 'object') {
270+
return false;
271+
}
272+
273+
if (callback) {
274+
return callback(maybeReport as ClientReport);
275+
}
276+
277+
return true;
278+
} catch {
279+
return false;
280+
}
281+
});
282+
}
283+
257284
export async function waitForSession(page: Page): Promise<SessionContext> {
258285
const req = await page.waitForRequest(req => {
259286
const postData = req.postData();

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
registerSpanErrorInstrumentation,
1818
SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
1919
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
20+
SEMANTIC_ATTRIBUTE_SENTRY_PREVIOUS_TRACE_SAMPLE_RATE,
2021
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
2122
spanIsSampled,
2223
spanToJSON,
@@ -40,6 +41,7 @@ import type { PreviousTraceInfo } from './previousTrace';
4041
import {
4142
addPreviousTraceSpanLink,
4243
getPreviousTraceFromSessionStorage,
44+
spanContextSampled,
4345
storePreviousTraceInSessionStorage,
4446
} from './previousTrace';
4547
import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request';
@@ -173,6 +175,23 @@ export interface BrowserTracingOptions {
173175
*/
174176
linkPreviousTrace: 'in-memory' | 'session-storage' | 'off';
175177

178+
/**
179+
* If true, Sentry will consistently sample subsequent traces based on the
180+
* sampling decision of the initial trace. For example, if the initial page
181+
* load trace was sampled positively, all subsequent traces (e.g. navigations)
182+
* are also sampled positively. In case the initial trace was sampled negatively,
183+
* all subsequent traces are also sampled negatively.
184+
*
185+
* This option lets you get consistent, linked traces within a user journey
186+
* while maintaining an overall quota based on your trace sampling settings.
187+
*
188+
* This option is only effective if {@link BrowserTracingOptions.linkPreviousTrace}
189+
* is enabled (i.e. not set to `'off'`).
190+
*
191+
* @default `false` - this is an opt-in feature.
192+
*/
193+
sampleLinkedTracesConsistently: boolean;
194+
176195
/**
177196
* _experiments allows the user to send options to define how this integration works.
178197
*
@@ -214,6 +233,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
214233
enableLongAnimationFrame: true,
215234
enableInp: true,
216235
linkPreviousTrace: 'in-memory',
236+
sampleLinkedTracesConsistently: false,
217237
_experiments: {},
218238
...defaultRequestInstrumentationOptions,
219239
};
@@ -265,6 +285,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
265285
instrumentPageLoad,
266286
instrumentNavigation,
267287
linkPreviousTrace,
288+
sampleLinkedTracesConsistently,
268289
onRequestSpanStart,
269290
} = {
270291
...DEFAULT_BROWSER_TRACING_OPTIONS,
@@ -342,6 +363,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
342363
});
343364
},
344365
});
366+
345367
setActiveIdleSpan(client, idleSpan);
346368

347369
function emitFinish(): void {
@@ -409,20 +431,67 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
409431
});
410432

411433
if (linkPreviousTrace !== 'off') {
412-
let inMemoryPreviousTraceInfo: PreviousTraceInfo | undefined = undefined;
434+
const useSessionStorage = linkPreviousTrace === 'session-storage';
435+
436+
let inMemoryPreviousTraceInfo = useSessionStorage ? getPreviousTraceFromSessionStorage() : undefined;
413437

414438
client.on('spanStart', span => {
415439
if (getRootSpan(span) !== span) {
416440
return;
417441
}
418442

419-
if (linkPreviousTrace === 'session-storage') {
420-
const updatedPreviousTraceInfo = addPreviousTraceSpanLink(getPreviousTraceFromSessionStorage(), span);
421-
storePreviousTraceInSessionStorage(updatedPreviousTraceInfo);
422-
} else {
423-
inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span);
443+
const scope = getCurrentScope();
444+
const oldPropagationContext = scope.getPropagationContext();
445+
inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span, oldPropagationContext);
446+
447+
if (useSessionStorage) {
448+
storePreviousTraceInSessionStorage(inMemoryPreviousTraceInfo);
424449
}
425450
});
451+
452+
if (sampleLinkedTracesConsistently) {
453+
/*
454+
This is a massive hack I'm really not proud of:
455+
456+
When users opt into `sampleLinkedTracesConsistently`, we need to make sure that we "propagate"
457+
the previous trace's sample rate and rand to the current trace. This is necessary because otherwise, span
458+
metric extrapolation is off, as we'd be propagating a too high sample rate for the subsequent traces.
459+
460+
So therefore, we pretend that the previous trace was the parent trace of the newly started trace. To do that,
461+
we mutate the propagation context of the current trace and set the sample rate and sample rand of the previous trace.
462+
Timing-wise, it is fine because it happens before we even sample the root span.
463+
464+
@see https://github.com/getsentry/sentry-javascript/issues/15754
465+
*/
466+
client.on('beforeSampling', mutableSamplingContextData => {
467+
if (!inMemoryPreviousTraceInfo) {
468+
return;
469+
}
470+
471+
const scope = getCurrentScope();
472+
const currentPropagationContext = scope.getPropagationContext();
473+
474+
scope.setPropagationContext({
475+
...currentPropagationContext,
476+
dsc: {
477+
...currentPropagationContext.dsc,
478+
// The fallback to 0 should never happen; this is rather to satisfy the types
479+
sample_rate: String(inMemoryPreviousTraceInfo.sampleRate ?? 0),
480+
sampled: String(spanContextSampled(inMemoryPreviousTraceInfo.spanContext)),
481+
},
482+
sampleRand: inMemoryPreviousTraceInfo.sampleRand,
483+
});
484+
485+
mutableSamplingContextData.parentSampled = spanContextSampled(inMemoryPreviousTraceInfo.spanContext);
486+
mutableSamplingContextData.parentSampleRate = inMemoryPreviousTraceInfo.sampleRate;
487+
488+
mutableSamplingContextData.spanAttributes = {
489+
...mutableSamplingContextData.spanAttributes,
490+
// record an attribute that this span was "force-sampled", so that we can later check on this.
491+
[SEMANTIC_ATTRIBUTE_SENTRY_PREVIOUS_TRACE_SAMPLE_RATE]: inMemoryPreviousTraceInfo.sampleRate,
492+
};
493+
});
494+
}
426495
}
427496

428497
if (WINDOW.location) {

packages/browser/src/tracing/previousTrace.ts

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
1-
import type { Span } from '@sentry/core';
2-
import { type SpanContextData, logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, spanToJSON } from '@sentry/core';
1+
import type { PropagationContext, Span } from '@sentry/core';
2+
import {
3+
type SpanContextData,
4+
logger,
5+
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
6+
SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE,
7+
spanToJSON,
8+
} from '@sentry/core';
39
import { DEBUG_BUILD } from '../debug-build';
410
import { WINDOW } from '../exports';
511

@@ -13,6 +19,16 @@ export interface PreviousTraceInfo {
1319
* Timestamp in seconds when the previous trace was started
1420
*/
1521
startTimestamp: number;
22+
23+
/**
24+
* sample rate of the previous trace
25+
*/
26+
sampleRate: number;
27+
28+
/**
29+
* The sample rand of the previous trace
30+
*/
31+
sampleRand: number;
1632
}
1733

1834
// 1h in seconds
@@ -33,14 +49,29 @@ export const PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE = 'sentry.previous_trace';
3349
export function addPreviousTraceSpanLink(
3450
previousTraceInfo: PreviousTraceInfo | undefined,
3551
span: Span,
52+
oldPropagationContext: PropagationContext,
3653
): PreviousTraceInfo {
3754
const spanJson = spanToJSON(span);
3855

56+
function getSampleRate(): number {
57+
try {
58+
return (
59+
Number(oldPropagationContext.dsc?.sample_rate) ?? Number(spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE])
60+
);
61+
} catch {
62+
return 0;
63+
}
64+
}
65+
66+
const updatedPreviousTraceInfo = {
67+
spanContext: span.spanContext(),
68+
startTimestamp: spanJson.start_timestamp,
69+
sampleRate: getSampleRate(),
70+
sampleRand: oldPropagationContext.sampleRand,
71+
};
72+
3973
if (!previousTraceInfo) {
40-
return {
41-
spanContext: span.spanContext(),
42-
startTimestamp: spanJson.start_timestamp,
43-
};
74+
return updatedPreviousTraceInfo;
4475
}
4576

4677
const previousTraceSpanCtx = previousTraceInfo.spanContext;
@@ -80,15 +111,12 @@ export function addPreviousTraceSpanLink(
80111
span.setAttribute(
81112
PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
82113
`${previousTraceSpanCtx.traceId}-${previousTraceSpanCtx.spanId}-${
83-
previousTraceSpanCtx.traceFlags === 0x1 ? 1 : 0
114+
spanContextSampled(previousTraceSpanCtx) ? 1 : 0
84115
}`,
85116
);
86117
}
87118

88-
return {
89-
spanContext: span.spanContext(),
90-
startTimestamp: spanToJSON(span).start_timestamp,
91-
};
119+
return updatedPreviousTraceInfo;
92120
}
93121

94122
/**
@@ -115,3 +143,10 @@ export function getPreviousTraceFromSessionStorage(): PreviousTraceInfo | undefi
115143
return undefined;
116144
}
117145
}
146+
147+
/**
148+
* see {@link import('@sentry/core').spanIsSampled}
149+
*/
150+
export const spanContextSampled = (ctx: SpanContextData): boolean => {
151+
return ctx.traceFlags === 0x1;
152+
};

0 commit comments

Comments
 (0)