Skip to content

Commit b59ce07

Browse files
authored
feat: Streamline sentry-trace, baggage and DSC handling (#14364)
As a first step for #12385, I looked though our current implementations, and tried to streamline this. We have a bunch of somewhat duplicate code there that handles sentry-trace & baggage headers _mostly_ the same but not _fully_ the same, as well as relatedly the DSC. To streamline this, I added some new methods in core: * `getTraceData` already exists, but was extended to also allow to pass a specific `span` to it. I updated some code to use this method that hasn't used it before, and also added some more tests - also around the ACS and the otel-specific implementation for this. * For this and edge cases, there are new primitives `getDynamicSamplingContextFromScope` and `getTraceContextFromScope` which handle picking these things from given scope etc. While working on this, I also noticed that `captureCheckIn` in ServerRuntimeClient was actually not properly working in Node (OTEL), as it relied on the core way of scope-span coupling. So I also updated this to actually work as expected.
1 parent cf368b1 commit b59ce07

File tree

22 files changed

+588
-355
lines changed

22 files changed

+588
-355
lines changed

.size-limit.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ module.exports = [
132132
path: 'packages/vue/build/esm/index.js',
133133
import: createImport('init'),
134134
gzip: true,
135-
limit: '28 KB',
135+
limit: '29 KB',
136136
},
137137
{
138138
name: '@sentry/vue (incl. Tracing)',

dev-packages/browser-integration-tests/suites/integrations/ContextLines/noAddedLines/test.ts

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ sentryTest('should not add source context lines to errors from script files', as
77
const url = await getLocalTestUrl({ testDir: __dirname });
88

99
const eventReqPromise = waitForErrorRequestOnUrl(page, url);
10+
await page.waitForFunction('window.Sentry');
1011

1112
const clickPromise = page.locator('#script-error-btn').click();
1213

docs/migration/draft-v9-migration-guide.md

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
- Deprecated `transactionNamingScheme` option in `requestDataIntegration`.
4343
- Deprecated `debugIntegration`. To log outgoing events, use [Hook Options](https://docs.sentry.io/platforms/javascript/configuration/options/#hooks) (`beforeSend`, `beforeSendTransaction`, ...).
4444
- Deprecated `sessionTimingIntegration`. To capture session durations alongside events, use [Context](https://docs.sentry.io/platforms/javascript/enriching-events/context/) (`Sentry.setContext()`).
45+
- Deprecated `addTracingHeadersToFetchRequest` method - this was only meant for internal use and is not needed anymore.
4546

4647
## `@sentry/nestjs`
4748

packages/browser/src/tracing/request.ts

+11-29
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,17 @@ import {
99
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
1010
SentryNonRecordingSpan,
1111
getActiveSpan,
12-
getClient,
13-
getCurrentScope,
14-
getDynamicSamplingContextFromClient,
15-
getDynamicSamplingContextFromSpan,
16-
getIsolationScope,
12+
getTraceData,
1713
hasTracingEnabled,
1814
instrumentFetchRequest,
1915
setHttpStatus,
2016
spanToJSON,
21-
spanToTraceHeader,
2217
startInactiveSpan,
2318
} from '@sentry/core';
2419
import {
2520
addFetchEndInstrumentationHandler,
2621
addFetchInstrumentationHandler,
2722
browserPerformanceTimeOrigin,
28-
dynamicSamplingContextToSentryBaggageHeader,
29-
generateSentryTraceHeader,
3023
parseUrl,
3124
stringMatchesSomePattern,
3225
} from '@sentry/core';
@@ -76,15 +69,17 @@ export interface RequestInstrumentationOptions {
7669
*
7770
* Default: true
7871
*/
79-
traceXHR: boolean /**
72+
traceXHR: boolean;
73+
74+
/**
8075
* Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch.
8176
* Do not enable this in case you have live streams or very long running streams.
8277
*
8378
* Disabled by default since it can lead to issues with streams using the `cancel()` api
8479
* (https://github.com/getsentry/sentry-javascript/issues/13950)
8580
*
8681
* Default: false
87-
*/;
82+
*/
8883
trackFetchStreamPerformance: boolean;
8984

9085
/**
@@ -401,12 +396,9 @@ export function xhrCallback(
401396
xhr.__sentry_xhr_span_id__ = span.spanContext().spanId;
402397
spans[xhr.__sentry_xhr_span_id__] = span;
403398

404-
const client = getClient();
405-
406-
if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url) && client) {
399+
if (shouldAttachHeaders(sentryXhrData.url)) {
407400
addTracingHeadersToXhrRequest(
408401
xhr,
409-
client,
410402
// If performance is disabled (TWP) or there's no active root span (pageload/navigation/interaction),
411403
// we do not want to use the span as base for the trace headers,
412404
// which means that the headers will be generated from the scope and the sampling decision is deferred
@@ -417,22 +409,12 @@ export function xhrCallback(
417409
return span;
418410
}
419411

420-
function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, client: Client, span?: Span): void {
421-
const scope = getCurrentScope();
422-
const isolationScope = getIsolationScope();
423-
const { traceId, spanId, sampled, dsc } = {
424-
...isolationScope.getPropagationContext(),
425-
...scope.getPropagationContext(),
426-
};
412+
function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, span?: Span): void {
413+
const { 'sentry-trace': sentryTrace, baggage } = getTraceData({ span });
427414

428-
const sentryTraceHeader =
429-
span && hasTracingEnabled() ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, spanId, sampled);
430-
431-
const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(
432-
dsc || (span ? getDynamicSamplingContextFromSpan(span) : getDynamicSamplingContextFromClient(traceId, client)),
433-
);
434-
435-
setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader);
415+
if (sentryTrace) {
416+
setHeaderOnXhr(xhr, sentryTrace, baggage);
417+
}
436418
}
437419

438420
function setHeaderOnXhr(

packages/core/src/baseclient.ts

+11-24
Original file line numberDiff line numberDiff line change
@@ -32,23 +32,22 @@ import type {
3232
} from '@sentry/types';
3333

3434
import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
35-
import { getIsolationScope } from './currentScopes';
35+
import { getCurrentScope, getIsolationScope, getTraceContextFromScope } from './currentScopes';
3636
import { DEBUG_BUILD } from './debug-build';
3737
import { createEventEnvelope, createSessionEnvelope } from './envelope';
3838
import type { IntegrationIndex } from './integration';
3939
import { afterSetupIntegrations } from './integration';
4040
import { setupIntegration, setupIntegrations } from './integration';
4141
import type { Scope } from './scope';
4242
import { updateSession } from './session';
43-
import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext';
43+
import { getDynamicSamplingContextFromScope } from './tracing/dynamicSamplingContext';
4444
import { createClientReportEnvelope } from './utils-hoist/clientreport';
4545
import { dsnToString, makeDsn } from './utils-hoist/dsn';
4646
import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils-hoist/envelope';
4747
import { SentryError } from './utils-hoist/error';
4848
import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils-hoist/is';
4949
import { consoleSandbox, logger } from './utils-hoist/logger';
5050
import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc';
51-
import { dropUndefinedKeys } from './utils-hoist/object';
5251
import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise';
5352
import { parseSampleRate } from './utils/parseSampleRate';
5453
import { prepareEvent } from './utils/prepareEvent';
@@ -672,7 +671,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
672671
protected _prepareEvent(
673672
event: Event,
674673
hint: EventHint,
675-
currentScope?: Scope,
674+
currentScope = getCurrentScope(),
676675
isolationScope = getIsolationScope(),
677676
): PromiseLike<Event | null> {
678677
const options = this.getOptions();
@@ -692,30 +691,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
692691
return evt;
693692
}
694693

695-
const propagationContext = {
696-
...isolationScope.getPropagationContext(),
697-
...(currentScope ? currentScope.getPropagationContext() : undefined),
694+
evt.contexts = {
695+
trace: getTraceContextFromScope(currentScope),
696+
...evt.contexts,
698697
};
699698

700-
const trace = evt.contexts && evt.contexts.trace;
701-
if (!trace && propagationContext) {
702-
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext;
703-
evt.contexts = {
704-
trace: dropUndefinedKeys({
705-
trace_id,
706-
span_id: spanId,
707-
parent_span_id: parentSpanId,
708-
}),
709-
...evt.contexts,
710-
};
699+
const dynamicSamplingContext = getDynamicSamplingContextFromScope(this, currentScope);
711700

712-
const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this);
701+
evt.sdkProcessingMetadata = {
702+
dynamicSamplingContext,
703+
...evt.sdkProcessingMetadata,
704+
};
713705

714-
evt.sdkProcessingMetadata = {
715-
dynamicSamplingContext,
716-
...evt.sdkProcessingMetadata,
717-
};
718-
}
719706
return evt;
720707
});
721708
}

packages/core/src/currentScopes.ts

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import type { Scope } from '@sentry/types';
1+
import type { Scope, TraceContext } from '@sentry/types';
22
import type { Client } from '@sentry/types';
33
import { getAsyncContextStrategy } from './asyncContext';
44
import { getMainCarrier } from './carrier';
55
import { Scope as ScopeClass } from './scope';
6+
import { dropUndefinedKeys } from './utils-hoist/object';
67
import { getGlobalSingleton } from './utils-hoist/worldwide';
78

89
/**
@@ -120,3 +121,20 @@ export function withIsolationScope<T>(
120121
export function getClient<C extends Client>(): C | undefined {
121122
return getCurrentScope().getClient<C>();
122123
}
124+
125+
/**
126+
* Get a trace context for the given scope.
127+
*/
128+
export function getTraceContextFromScope(scope: Scope): TraceContext {
129+
const propagationContext = scope.getPropagationContext();
130+
131+
const { traceId, spanId, parentSpanId } = propagationContext;
132+
133+
const traceContext: TraceContext = dropUndefinedKeys({
134+
trace_id: traceId,
135+
span_id: spanId,
136+
parent_span_id: parentSpanId,
137+
});
138+
139+
return traceContext;
140+
}

0 commit comments

Comments
 (0)