From 9e88097440d86017af516dc3daa4e04c715ad5d5 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 26 Nov 2024 13:44:41 +0100 Subject: [PATCH] remove trace state handling --- packages/opentelemetry/src/constants.ts | 1 - packages/opentelemetry/src/propagator.ts | 5 ++--- .../opentelemetry/src/setupEventContextTrace.ts | 12 +----------- packages/opentelemetry/src/spanExporter.ts | 6 +----- packages/opentelemetry/src/trace.ts | 3 +-- .../generateSpanContextForPropagationContext.ts | 1 - packages/opentelemetry/src/utils/makeTraceState.ts | 14 +++----------- .../test/integration/transactions.test.ts | 1 - packages/opentelemetry/test/propagator.test.ts | 10 +++------- packages/opentelemetry/test/trace.test.ts | 2 -- 10 files changed, 11 insertions(+), 44 deletions(-) diff --git a/packages/opentelemetry/src/constants.ts b/packages/opentelemetry/src/constants.ts index 0f056d09a4ea..22e3d01c46a8 100644 --- a/packages/opentelemetry/src/constants.ts +++ b/packages/opentelemetry/src/constants.ts @@ -4,7 +4,6 @@ export const SENTRY_TRACE_HEADER = 'sentry-trace'; export const SENTRY_BAGGAGE_HEADER = 'baggage'; export const SENTRY_TRACE_STATE_DSC = 'sentry.dsc'; -export const SENTRY_TRACE_STATE_PARENT_SPAN_ID = 'sentry.parent_span_id'; export const SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING = 'sentry.sampled_not_recording'; export const SENTRY_TRACE_STATE_URL = 'sentry.url'; diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 6c4009888416..b7b9dffc1ac9 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -25,7 +25,6 @@ import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_DSC, - SENTRY_TRACE_STATE_PARENT_SPAN_ID, SENTRY_TRACE_STATE_URL, } from './constants'; import { DEBUG_BUILD } from './debug-build'; @@ -33,6 +32,7 @@ import { getScopesFromContext, setScopesOnContext } from './utils/contextData'; import { generateSpanContextForPropagationContext } from './utils/generateSpanContextForPropagationContext'; import { getSamplingDecision } from './utils/getSamplingDecision'; import { setIsSetup } from './utils/setupCheck'; +import { spanHasParentId } from './utils/spanTypes'; /** Get the Sentry propagation context from a span context. */ export function getPropagationContextFromSpan(span: Span): PropagationContext { @@ -44,8 +44,7 @@ export function getPropagationContextFromSpan(span: Span): PropagationContext { const dscString = traceState ? traceState.get(SENTRY_TRACE_STATE_DSC) : undefined; const traceStateDsc = dscString ? baggageHeaderToDynamicSamplingContext(dscString) : undefined; - const parentSpanId = traceState ? traceState.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID) || undefined : undefined; - + const parentSpanId = spanHasParentId(span) ? span.parentSpanId : undefined; const sampled = getSamplingDecision(spanContext); // No trace state? --> Take DSC from root span diff --git a/packages/opentelemetry/src/setupEventContextTrace.ts b/packages/opentelemetry/src/setupEventContextTrace.ts index 051b1cba1364..8e59698b44f1 100644 --- a/packages/opentelemetry/src/setupEventContextTrace.ts +++ b/packages/opentelemetry/src/setupEventContextTrace.ts @@ -1,7 +1,6 @@ import { getDynamicSamplingContextFromSpan, getRootSpan } from '@sentry/core'; import { dropUndefinedKeys } from '@sentry/core'; import type { Client } from '@sentry/types'; -import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { getActiveSpan } from './utils/getActiveSpan'; import { spanHasParentId } from './utils/spanTypes'; @@ -16,16 +15,7 @@ export function setupEventContextTrace(client: Client): void { } const spanContext = span.spanContext(); - - // If we have a parent span id from trace state, use that ('' means no parent should be used) - // Else, pick the one from the span - const parentSpanIdFromTraceState = spanContext.traceState?.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID); - const parent_span_id = - typeof parentSpanIdFromTraceState === 'string' - ? parentSpanIdFromTraceState || undefined - : spanHasParentId(span) - ? span.parentSpanId - : undefined; + const parent_span_id = spanHasParentId(span) ? span.parentSpanId : undefined; // If event has already set `trace` context, use that one. event.contexts = { diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 11d822f66ec1..41ed1e3d2ea5 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -19,7 +19,6 @@ import { } from '@sentry/core'; import { dropUndefinedKeys, logger } from '@sentry/core'; import type { SpanJSON, SpanOrigin, TraceContext, TransactionEvent, TransactionSource } from '@sentry/types'; -import { SENTRY_TRACE_STATE_PARENT_SPAN_ID } from './constants'; import { DEBUG_BUILD } from './debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE } from './semanticAttributes'; @@ -207,15 +206,12 @@ function createTransactionForOtelSpan(span: ReadableSpan): TransactionEvent { const { traceId: trace_id, spanId: span_id } = span.spanContext(); - const parentSpanIdFromTraceState = span.spanContext().traceState?.get(SENTRY_TRACE_STATE_PARENT_SPAN_ID); - // If parentSpanIdFromTraceState is defined at all, we want it to take precedence // In that case, an empty string should be interpreted as "no parent span id", // even if `span.parentSpanId` is set // this is the case when we are starting a new trace, where we have a virtual span based on the propagationContext // We only want to continue the traceId in this case, but ignore the parent span - const parent_span_id = - typeof parentSpanIdFromTraceState === 'string' ? parentSpanIdFromTraceState || undefined : span.parentSpanId; + const parent_span_id = span.parentSpanId; const status = mapStatus(span); diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index c74b26a81544..66b48adc8773 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -1,5 +1,5 @@ import type { Context, Span, SpanContext, SpanOptions, Tracer } from '@opentelemetry/api'; -import { INVALID_SPANID, SpanStatusCode, TraceFlags, context, trace } from '@opentelemetry/api'; +import { SpanStatusCode, TraceFlags, context, trace } from '@opentelemetry/api'; import { suppressTracing } from '@opentelemetry/core'; import { SDK_VERSION, @@ -209,7 +209,6 @@ function getContext(scope: Scope | undefined, forceTransaction: boolean | undefi const traceState = makeTraceState({ dsc, - parentSpanId: spanId !== INVALID_SPANID ? spanId : undefined, sampled, }); diff --git a/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts b/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts index d2aa470213f7..c16a3bfd6805 100644 --- a/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts +++ b/packages/opentelemetry/src/utils/generateSpanContextForPropagationContext.ts @@ -10,7 +10,6 @@ import { makeTraceState } from './makeTraceState'; export function generateSpanContextForPropagationContext(propagationContext: PropagationContext): SpanContext { // We store the DSC as OTEL trace state on the span context const traceState = makeTraceState({ - parentSpanId: propagationContext.parentSpanId, dsc: propagationContext.dsc, sampled: propagationContext.sampled, }); diff --git a/packages/opentelemetry/src/utils/makeTraceState.ts b/packages/opentelemetry/src/utils/makeTraceState.ts index e292274cade0..fea5d48fb4b8 100644 --- a/packages/opentelemetry/src/utils/makeTraceState.ts +++ b/packages/opentelemetry/src/utils/makeTraceState.ts @@ -1,20 +1,16 @@ import { TraceState } from '@opentelemetry/core'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/core'; import type { DynamicSamplingContext } from '@sentry/types'; -import { - SENTRY_TRACE_STATE_DSC, - SENTRY_TRACE_STATE_PARENT_SPAN_ID, - SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, -} from '../constants'; +import { SENTRY_TRACE_STATE_DSC, SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING } from '../constants'; /** * Generate a TraceState for the given data. */ export function makeTraceState({ - parentSpanId, dsc, sampled, }: { + /** @deprecated parentSpanId does not do anything anymore and will be removed in v9. */ parentSpanId?: string; dsc?: Partial; sampled?: boolean; @@ -22,11 +18,7 @@ export function makeTraceState({ // We store the DSC as OTEL trace state on the span context const dscString = dsc ? dynamicSamplingContextToSentryBaggageHeader(dsc) : undefined; - // We _always_ set the parent span ID, even if it is empty - // If we'd set this to 'undefined' we could not know if the trace state was set, but there was no parentSpanId, - // vs the trace state was not set at all (in which case we want to do fallback handling) - // If `''`, it should be considered "no parent" - const traceStateBase = new TraceState().set(SENTRY_TRACE_STATE_PARENT_SPAN_ID, parentSpanId || ''); + const traceStateBase = new TraceState(); const traceStateWithDsc = dscString ? traceStateBase.set(SENTRY_TRACE_STATE_DSC, dscString) : traceStateBase; diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index b66147a413d7..acf4134d2c0e 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -327,7 +327,6 @@ describe('Integration | Transactions', () => { const parentSpanId = '6e0c63257de34c92'; const traceState = makeTraceState({ - parentSpanId, dsc: undefined, sampled: true, }); diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index d3b9483674a1..1b0360aa766e 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -181,7 +181,6 @@ describe('SentryPropagator', () => { traceFlags: TraceFlags.SAMPLED, isRemote: true, traceState: makeTraceState({ - parentSpanId: '6e0c63257de34c92', dsc: { transaction: 'sampled-transaction', sampled: 'true', @@ -256,7 +255,6 @@ describe('SentryPropagator', () => { traceFlags: TraceFlags.NONE, isRemote: true, traceState: makeTraceState({ - parentSpanId: '6e0c63257de34c92', dsc: { transaction: 'sampled-transaction', sampled: 'false', @@ -291,7 +289,6 @@ describe('SentryPropagator', () => { isRemote: true, traceState: makeTraceState({ sampled: false, - parentSpanId: '6e0c63257de34c92', dsc: { transaction: 'sampled-transaction', trace_id: 'dsc_trace_id', @@ -557,7 +554,7 @@ describe('SentryPropagator', () => { spanId: '6e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, traceId: 'd4cda95b652f4a1592b449d5929fda1b', - traceState: makeTraceState({ parentSpanId: '6e0c63257de34c92' }), + traceState: makeTraceState({}), }); expect(getSamplingDecision(trace.getSpanContext(context)!)).toBe(true); }); @@ -571,7 +568,7 @@ describe('SentryPropagator', () => { spanId: '6e0c63257de34c92', traceFlags: TraceFlags.NONE, traceId: 'd4cda95b652f4a1592b449d5929fda1b', - traceState: makeTraceState({ parentSpanId: '6e0c63257de34c92', sampled: false }), + traceState: makeTraceState({ sampled: false }), }); expect(getSamplingDecision(trace.getSpanContext(context)!)).toBe(false); }); @@ -585,7 +582,7 @@ describe('SentryPropagator', () => { spanId: '6e0c63257de34c92', traceFlags: TraceFlags.NONE, traceId: 'd4cda95b652f4a1592b449d5929fda1b', - traceState: makeTraceState({ parentSpanId: '6e0c63257de34c92' }), + traceState: makeTraceState({}), }); expect(getSamplingDecision(trace.getSpanContext(context)!)).toBe(undefined); }); @@ -635,7 +632,6 @@ describe('SentryPropagator', () => { traceFlags: TraceFlags.SAMPLED, traceId: 'd4cda95b652f4a1592b449d5929fda1b', traceState: makeTraceState({ - parentSpanId: '6e0c63257de34c92', dsc: { environment: 'production', release: '1.0.0', diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 290c7576dee1..f6f8c45223e2 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -1084,7 +1084,6 @@ describe('trace', () => { isRemote: false, traceFlags: TraceFlags.SAMPLED, traceState: makeTraceState({ - parentSpanId: '1121201211212011', dsc: { release: '1.0', environment: 'production', @@ -1114,7 +1113,6 @@ describe('trace', () => { isRemote: true, traceFlags: TraceFlags.SAMPLED, traceState: makeTraceState({ - parentSpanId: '1121201211212011', dsc: { release: '1.0', environment: 'production',