Skip to content

Commit

Permalink
feat!: Remove spanId from propagation context
Browse files Browse the repository at this point in the history
Closes #12385

This also deprecates `getPropagationContextFromSpan` as it is no longer used/needed. We may think about removing this in v9, but IMHO we can also just leave this for v9, it does not hurt too much to have it in there...
  • Loading branch information
mydea committed Dec 17, 2024
1 parent 031ef3a commit 05ca00e
Show file tree
Hide file tree
Showing 12 changed files with 21 additions and 56 deletions.
7 changes: 3 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { getAsyncContextStrategy } from './asyncContext';
import { getGlobalSingleton, getMainCarrier } from './carrier';
import { Scope } from './scope';
import type { Client, TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
Expand Down Expand Up @@ -126,13 +127,11 @@ export function getClient<C extends Client>(): C | undefined {
export function getTraceContextFromScope(scope: Scope): TraceContext {
const propagationContext = scope.getPropagationContext();

// TODO(v9): Use generateSpanId() instead of spanId
// eslint-disable-next-line deprecation/deprecation
const { traceId, spanId, parentSpanId } = propagationContext;
const { traceId, parentSpanId } = propagationContext;

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

Expand Down
13 changes: 3 additions & 10 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { updateSession } from './session';
import { isPlainObject } from './utils-hoist/is';
import { logger } from './utils-hoist/logger';
import { uuid4 } from './utils-hoist/misc';
import { generateSpanId, generateTraceId } from './utils-hoist/propagationContext';
import { generateTraceId } from './utils-hoist/propagationContext';
import { dateTimestampInSeconds } from './utils-hoist/time';
import { merge } from './utils/merge';
import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
Expand Down Expand Up @@ -156,7 +156,6 @@ export class Scope {
this._sdkProcessingMetadata = {};
this._propagationContext = {
traceId: generateTraceId(),
spanId: generateSpanId(),
};
}

Expand Down Expand Up @@ -575,14 +574,8 @@ export class Scope {
/**
* Add propagation context to the scope, used for distributed tracing
*/
public setPropagationContext(
context: Omit<PropagationContext, 'spanId'> & Partial<Pick<PropagationContext, 'spanId'>>,
): this {
this._propagationContext = {
// eslint-disable-next-line deprecation/deprecation
spanId: generateSpanId(),
...context,
};
public setPropagationContext(context: PropagationContext): this {
this._propagationContext = context;
return this;
}

Expand Down
9 changes: 0 additions & 9 deletions packages/core/src/types-hoist/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,6 @@ export interface PropagationContext {
* Either represents the incoming `traceId` or the `traceId` generated by the current SDK, if there was no incoming trace.
*/
traceId: string;
/**
* Represents the execution context of the current SDK. This acts as a fallback value to associate events with a
* particular execution context when performance monitoring is disabled.
*
* The ID of a current span (if one exists) should have precedence over this value when propagating trace data.
*
* @deprecated This value will not be used anymore in the future, and should not be set or read anymore.
*/
spanId: string;
/**
* Represents the sampling decision of the incoming trace.
*
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/utils-hoist/propagationContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { uuid4 } from './misc';
export function generatePropagationContext(): PropagationContext {
return {
traceId: generateTraceId(),
spanId: generateSpanId(),
};
}

Expand Down
5 changes: 1 addition & 4 deletions packages/core/src/utils-hoist/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,17 +55,14 @@ export function propagationContextFromHeaders(
const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(baggage);

if (!traceparentData || !traceparentData.traceId) {
return { traceId: generateTraceId(), spanId: generateSpanId() };
return { traceId: generateTraceId() };
}

const { traceId, parentSpanId, parentSampled } = traceparentData;

const virtualSpanId = generateSpanId();

return {
traceId,
parentSpanId,
spanId: virtualSpanId,
sampled: parentSampled,
dsc: dynamicSamplingContext || {}, // If we have traceparent data but no DSC it means we are not head of trace and we must freeze it
};
Expand Down
7 changes: 3 additions & 4 deletions packages/core/src/utils/traceData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { isEnabled } from '../exports';
import type { Scope } from '../scope';
import { getDynamicSamplingContextFromScope, getDynamicSamplingContextFromSpan } from '../tracing';
import type { SerializedTraceData, Span } from '../types-hoist';
import { generateSpanId } from '../utils-hoist';
import { dynamicSamplingContextToSentryBaggageHeader } from '../utils-hoist/baggage';
import { logger } from '../utils-hoist/logger';
import { TRACEPARENT_REGEXP, generateSentryTraceHeader } from '../utils-hoist/tracing';
Expand Down Expand Up @@ -55,8 +56,6 @@ export function getTraceData(options: { span?: Span } = {}): SerializedTraceData
* Get a sentry-trace header value for the given scope.
*/
function scopeToTraceHeader(scope: Scope): string {
// TODO(v9): Use generateSpanId() instead of spanId
// eslint-disable-next-line deprecation/deprecation
const { traceId, sampled, spanId } = scope.getPropagationContext();
return generateSentryTraceHeader(traceId, spanId, sampled);
const { traceId, sampled } = scope.getPropagationContext();
return generateSentryTraceHeader(traceId, generateSpanId(), sampled);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
SPAN_STATUS_OK,
Scope,
captureException,
generateSpanId,
generateTraceId,
getActiveSpan,
getCapturedScopesOnSpan,
Expand Down Expand Up @@ -90,7 +89,6 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
? propagationContextFromHeaders(headersDict['sentry-trace'], headersDict['baggage'])
: {
traceId: requestTraceId || generateTraceId(),
spanId: generateSpanId(),
},
);
scope.setPropagationContext(propagationContext);
Expand Down
2 changes: 0 additions & 2 deletions packages/nextjs/src/common/wrapServerComponentWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
SPAN_STATUS_OK,
Scope,
captureException,
generateSpanId,
generateTraceId,
getActiveSpan,
getCapturedScopesOnSpan,
Expand Down Expand Up @@ -69,7 +68,6 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
? propagationContextFromHeaders(headersDict['sentry-trace'], headersDict['baggage'])
: {
traceId: requestTraceId || generateTraceId(),
spanId: generateSpanId(),
},
);

Expand Down
7 changes: 3 additions & 4 deletions packages/node/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Session as InspectorSession } from 'node:inspector';
import { parentPort, workerData } from 'node:worker_threads';
import type { DebugImage, Event, ScopeData, Session, StackFrame } from '@sentry/core';
import { generateSpanId } from '@sentry/core';
import {
applyScopeDataToEvent,
callFrameToStackFrame,
Expand Down Expand Up @@ -121,13 +122,11 @@ function applyScopeToEvent(event: Event, scope: ScopeData): void {
applyScopeDataToEvent(event, scope);

if (!event.contexts?.trace) {
// TODO(v9): Use generateSpanId() instead of spanId
// eslint-disable-next-line deprecation/deprecation
const { traceId, spanId, parentSpanId } = scope.propagationContext;
const { traceId, parentSpanId } = scope.propagationContext;
event.contexts = {
trace: {
trace_id: traceId,
span_id: spanId,
span_id: generateSpanId(),
parent_span_id: parentSpanId,
},
...event.contexts,
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export { setOpenTelemetryContextAsyncContextStrategy } from './asyncContextStrat
export { wrapContextManagerClass } from './contextManager';
export {
SentryPropagator,
// eslint-disable-next-line deprecation/deprecation
getPropagationContextFromSpan,
shouldPropagateTraceForUrl,
} from './propagator';
Expand Down
12 changes: 6 additions & 6 deletions packages/opentelemetry/src/propagator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@ import { makeTraceState } from './utils/makeTraceState';
import { setIsSetup } from './utils/setupCheck';
import { spanHasParentId } from './utils/spanTypes';

/** Get the Sentry propagation context from a span context. */
/**
* Get the Sentry propagation context from a span context.
* @deprecated This method is not used anymore and may be removed in a future major.
*/
export function getPropagationContextFromSpan(span: Span): PropagationContext {
const spanContext = span.spanContext();
const { traceId, spanId, traceState } = spanContext;
const { traceId, traceState } = spanContext;

// When we have a dsc trace state, it means this came from the incoming trace
// Then this takes presedence over the root span
Expand All @@ -52,7 +55,6 @@ export function getPropagationContextFromSpan(span: Span): PropagationContext {

return {
traceId,
spanId,
sampled,
parentSpanId,
dsc,
Expand Down Expand Up @@ -234,9 +236,7 @@ export function getInjectionData(context: Context): {
return {
dynamicSamplingContext,
traceId: propagationContext.traceId,
// TODO(v9): Use generateSpanId() instead
// eslint-disable-next-line deprecation/deprecation
spanId: propagationContext.spanId,
spanId: generateSpanId(),
sampled: propagationContext.sampled,
};
}
Expand Down
11 changes: 1 addition & 10 deletions packages/opentelemetry/src/sampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
} from '@sentry/core';
import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants';
import { DEBUG_BUILD } from './debug-build';
import { getPropagationContextFromSpan } from './propagator';
import { getSamplingDecision } from './utils/getSamplingDecision';
import { inferSpanData } from './utils/parseSpanDescription';
import { setIsSetup } from './utils/setupCheck';
Expand Down Expand Up @@ -138,22 +137,14 @@ export class SentrySampler implements Sampler {
}
}

function getParentRemoteSampled(parentSpan: Span): boolean | undefined {
const traceId = parentSpan.spanContext().traceId;
const traceparentData = getPropagationContextFromSpan(parentSpan);

// Only inherit sampled if `traceId` is the same
return traceparentData && traceId === traceparentData.traceId ? traceparentData.sampled : undefined;
}

function getParentSampled(parentSpan: Span, traceId: string, spanName: string): boolean | undefined {
const parentContext = parentSpan.spanContext();

// Only inherit sample rate if `traceId` is the same
// Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones
if (isSpanContextValid(parentContext) && parentContext.traceId === traceId) {
if (parentContext.isRemote) {
const parentSampled = getParentRemoteSampled(parentSpan);
const parentSampled = getSamplingDecision(parentSpan.spanContext());
DEBUG_BUILD &&
logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`);
return parentSampled;
Expand Down

0 comments on commit 05ca00e

Please sign in to comment.