Skip to content

Commit

Permalink
feat: Streamline sentry-trace, baggage and DSC handling (#14364)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mydea authored Nov 26, 2024
1 parent cf368b1 commit b59ce07
Show file tree
Hide file tree
Showing 22 changed files with 588 additions and 355 deletions.
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ module.exports = [
path: 'packages/vue/build/esm/index.js',
import: createImport('init'),
gzip: true,
limit: '28 KB',
limit: '29 KB',
},
{
name: '@sentry/vue (incl. Tracing)',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ sentryTest('should not add source context lines to errors from script files', as
const url = await getLocalTestUrl({ testDir: __dirname });

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

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

Expand Down
1 change: 1 addition & 0 deletions docs/migration/draft-v9-migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- Deprecated `transactionNamingScheme` option in `requestDataIntegration`.
- Deprecated `debugIntegration`. To log outgoing events, use [Hook Options](https://docs.sentry.io/platforms/javascript/configuration/options/#hooks) (`beforeSend`, `beforeSendTransaction`, ...).
- Deprecated `sessionTimingIntegration`. To capture session durations alongside events, use [Context](https://docs.sentry.io/platforms/javascript/enriching-events/context/) (`Sentry.setContext()`).
- Deprecated `addTracingHeadersToFetchRequest` method - this was only meant for internal use and is not needed anymore.

## `@sentry/nestjs`

Expand Down
40 changes: 11 additions & 29 deletions packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,17 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SentryNonRecordingSpan,
getActiveSpan,
getClient,
getCurrentScope,
getDynamicSamplingContextFromClient,
getDynamicSamplingContextFromSpan,
getIsolationScope,
getTraceData,
hasTracingEnabled,
instrumentFetchRequest,
setHttpStatus,
spanToJSON,
spanToTraceHeader,
startInactiveSpan,
} from '@sentry/core';
import {
addFetchEndInstrumentationHandler,
addFetchInstrumentationHandler,
browserPerformanceTimeOrigin,
dynamicSamplingContextToSentryBaggageHeader,
generateSentryTraceHeader,
parseUrl,
stringMatchesSomePattern,
} from '@sentry/core';
Expand Down Expand Up @@ -76,15 +69,17 @@ export interface RequestInstrumentationOptions {
*
* Default: true
*/
traceXHR: boolean /**
traceXHR: boolean;

/**
* Flag to disable tracking of long-lived streams, like server-sent events (SSE) via fetch.
* Do not enable this in case you have live streams or very long running streams.
*
* Disabled by default since it can lead to issues with streams using the `cancel()` api
* (https://github.com/getsentry/sentry-javascript/issues/13950)
*
* Default: false
*/;
*/
trackFetchStreamPerformance: boolean;

/**
Expand Down Expand Up @@ -401,12 +396,9 @@ export function xhrCallback(
xhr.__sentry_xhr_span_id__ = span.spanContext().spanId;
spans[xhr.__sentry_xhr_span_id__] = span;

const client = getClient();

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

function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, client: Client, span?: Span): void {
const scope = getCurrentScope();
const isolationScope = getIsolationScope();
const { traceId, spanId, sampled, dsc } = {
...isolationScope.getPropagationContext(),
...scope.getPropagationContext(),
};
function addTracingHeadersToXhrRequest(xhr: SentryWrappedXMLHttpRequest, span?: Span): void {
const { 'sentry-trace': sentryTrace, baggage } = getTraceData({ span });

const sentryTraceHeader =
span && hasTracingEnabled() ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, spanId, sampled);

const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(
dsc || (span ? getDynamicSamplingContextFromSpan(span) : getDynamicSamplingContextFromClient(traceId, client)),
);

setHeaderOnXhr(xhr, sentryTraceHeader, sentryBaggageHeader);
if (sentryTrace) {
setHeaderOnXhr(xhr, sentryTrace, baggage);
}
}

function setHeaderOnXhr(
Expand Down
35 changes: 11 additions & 24 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,22 @@ import type {
} from '@sentry/types';

import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
import { getIsolationScope } from './currentScopes';
import { getCurrentScope, getIsolationScope, getTraceContextFromScope } from './currentScopes';
import { DEBUG_BUILD } from './debug-build';
import { createEventEnvelope, createSessionEnvelope } from './envelope';
import type { IntegrationIndex } from './integration';
import { afterSetupIntegrations } from './integration';
import { setupIntegration, setupIntegrations } from './integration';
import type { Scope } from './scope';
import { updateSession } from './session';
import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext';
import { getDynamicSamplingContextFromScope } from './tracing/dynamicSamplingContext';
import { createClientReportEnvelope } from './utils-hoist/clientreport';
import { dsnToString, makeDsn } from './utils-hoist/dsn';
import { addItemToEnvelope, createAttachmentEnvelopeItem } from './utils-hoist/envelope';
import { SentryError } from './utils-hoist/error';
import { isParameterizedString, isPlainObject, isPrimitive, isThenable } from './utils-hoist/is';
import { consoleSandbox, logger } from './utils-hoist/logger';
import { checkOrSetAlreadyCaught, uuid4 } from './utils-hoist/misc';
import { dropUndefinedKeys } from './utils-hoist/object';
import { SyncPromise, rejectedSyncPromise, resolvedSyncPromise } from './utils-hoist/syncpromise';
import { parseSampleRate } from './utils/parseSampleRate';
import { prepareEvent } from './utils/prepareEvent';
Expand Down Expand Up @@ -672,7 +671,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
protected _prepareEvent(
event: Event,
hint: EventHint,
currentScope?: Scope,
currentScope = getCurrentScope(),
isolationScope = getIsolationScope(),
): PromiseLike<Event | null> {
const options = this.getOptions();
Expand All @@ -692,30 +691,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
return evt;
}

const propagationContext = {
...isolationScope.getPropagationContext(),
...(currentScope ? currentScope.getPropagationContext() : undefined),
evt.contexts = {
trace: getTraceContextFromScope(currentScope),
...evt.contexts,
};

const trace = evt.contexts && evt.contexts.trace;
if (!trace && propagationContext) {
const { traceId: trace_id, spanId, parentSpanId, dsc } = propagationContext;
evt.contexts = {
trace: dropUndefinedKeys({
trace_id,
span_id: spanId,
parent_span_id: parentSpanId,
}),
...evt.contexts,
};
const dynamicSamplingContext = getDynamicSamplingContextFromScope(this, currentScope);

const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this);
evt.sdkProcessingMetadata = {
dynamicSamplingContext,
...evt.sdkProcessingMetadata,
};

evt.sdkProcessingMetadata = {
dynamicSamplingContext,
...evt.sdkProcessingMetadata,
};
}
return evt;
});
}
Expand Down
20 changes: 19 additions & 1 deletion packages/core/src/currentScopes.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { Scope } from '@sentry/types';
import type { Scope, TraceContext } from '@sentry/types';
import type { Client } from '@sentry/types';
import { getAsyncContextStrategy } from './asyncContext';
import { getMainCarrier } from './carrier';
import { Scope as ScopeClass } from './scope';
import { dropUndefinedKeys } from './utils-hoist/object';
import { getGlobalSingleton } from './utils-hoist/worldwide';

/**
Expand Down Expand Up @@ -120,3 +121,20 @@ export function withIsolationScope<T>(
export function getClient<C extends Client>(): C | undefined {
return getCurrentScope().getClient<C>();
}

/**
* Get a trace context for the given scope.
*/
export function getTraceContextFromScope(scope: Scope): TraceContext {
const propagationContext = scope.getPropagationContext();

const { traceId, spanId, parentSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
trace_id: traceId,
span_id: spanId,
parent_span_id: parentSpanId,
});

return traceContext;
}
Loading

0 comments on commit b59ce07

Please sign in to comment.