From b4a434260124c4e577abf9ad216e6efb077a8199 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 10 Nov 2022 14:12:03 +0100 Subject: [PATCH] fix(otel): Use span.toTraceParent to set sentry-trace in propagator (#6174) The trace flag is always set to SAMPLED, so this would not propagate the correct sentry-trace header if sentry sampling drops the transaction. Co-authored-by: Abhijeet Prasad --- packages/opentelemetry-node/src/propagator.ts | 19 +++-- .../test/propagator.test.ts | 72 +++---------------- 2 files changed, 20 insertions(+), 71 deletions(-) diff --git a/packages/opentelemetry-node/src/propagator.ts b/packages/opentelemetry-node/src/propagator.ts index 3cb1171c8c8e..4e0df90f912a 100644 --- a/packages/opentelemetry-node/src/propagator.ts +++ b/packages/opentelemetry-node/src/propagator.ts @@ -35,17 +35,16 @@ export class SentryPropagator implements TextMapPropagator { return; } - // eslint-disable-next-line no-bitwise - const samplingDecision = spanContext.traceFlags & TraceFlags.SAMPLED ? 1 : 0; - const traceparent = `${spanContext.traceId}-${spanContext.spanId}-${samplingDecision}`; - setter.set(carrier, SENTRY_TRACE_HEADER, traceparent); - const span = SENTRY_SPAN_PROCESSOR_MAP.get(spanContext.spanId); - if (span && span.transaction) { - const dynamicSamplingContext = span.transaction.getDynamicSamplingContext(); - const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); - if (sentryBaggageHeader) { - setter.set(carrier, SENTRY_BAGGAGE_HEADER, sentryBaggageHeader); + if (span) { + setter.set(carrier, SENTRY_TRACE_HEADER, span.toTraceparent()); + + if (span.transaction) { + const dynamicSamplingContext = span.transaction.getDynamicSamplingContext(); + const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); + if (sentryBaggageHeader) { + setter.set(carrier, SENTRY_BAGGAGE_HEADER, sentryBaggageHeader); + } } } } diff --git a/packages/opentelemetry-node/test/propagator.test.ts b/packages/opentelemetry-node/test/propagator.test.ts index 3e85a76033cf..49e2243fe8ee 100644 --- a/packages/opentelemetry-node/test/propagator.test.ts +++ b/packages/opentelemetry-node/test/propagator.test.ts @@ -30,63 +30,7 @@ describe('SentryPropagator', () => { }); describe('inject', () => { - describe('sentry-trace', () => { - it.each([ - [ - 'should set sentry-trace header when sampled', - { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.SAMPLED, - }, - 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1', - ], - [ - 'should set sentry-trace header when not sampled', - { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }, - 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-0', - ], - [ - 'should NOT set sentry-trace header when traceId is empty', - { - traceId: '', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.SAMPLED, - }, - undefined, - ], - [ - 'should NOT set sentry-trace header when spanId is empty', - { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '', - traceFlags: TraceFlags.NONE, - }, - undefined, - ], - ])('%s', (_name, spanContext, expected) => { - const context = trace.setSpanContext(ROOT_CONTEXT, spanContext); - propagator.inject(context, carrier, defaultTextMapSetter); - expect(carrier[SENTRY_TRACE_HEADER]).toBe(expected); - }); - - it('should NOT set sentry-trace header if instrumentation is supressed', () => { - const spanContext = { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.SAMPLED, - }; - const context = suppressTracing(trace.setSpanContext(ROOT_CONTEXT, spanContext)); - propagator.inject(context, carrier, defaultTextMapSetter); - expect(carrier[SENTRY_TRACE_HEADER]).toBe(undefined); - }); - }); - - describe('baggage', () => { + describe('baggage and sentry-trace', () => { const client = { getOptions: () => ({ environment: 'production', @@ -123,7 +67,7 @@ describe('SentryPropagator', () => { describe.each([PerfType.Transaction, PerfType.Span])('with active %s', type => { it.each([ [ - 'should set baggage header when sampled', + 'should set baggage and header when sampled', { traceId: 'd4cda95b652f4a1592b449d5929fda1b', spanId: '6e0c63257de34c92', @@ -136,6 +80,7 @@ describe('SentryPropagator', () => { sampled: true, }, 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=sampled-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1', ], [ 'should NOT set baggage header when not sampled', @@ -151,6 +96,7 @@ describe('SentryPropagator', () => { sampled: false, }, 'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=not-sampled-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-0', ], [ 'should NOT set baggage header when traceId is empty', @@ -166,6 +112,7 @@ describe('SentryPropagator', () => { sampled: true, }, undefined, + undefined, ], [ 'should NOT set baggage header when spanId is empty', @@ -181,15 +128,17 @@ describe('SentryPropagator', () => { sampled: true, }, undefined, + undefined, ], - ])('%s', (_name, spanContext, transactionContext, expected) => { + ])('%s', (_name, spanContext, transactionContext, baggage, sentryTrace) => { createTransactionAndMaybeSpan(type, transactionContext); const context = trace.setSpanContext(ROOT_CONTEXT, spanContext); propagator.inject(context, carrier, defaultTextMapSetter); - expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(expected); + expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(baggage); + expect(carrier[SENTRY_TRACE_HEADER]).toBe(sentryTrace); }); - it('should NOT set sentry-trace header if instrumentation is supressed', () => { + it('should NOT set baggage and sentry-trace header if instrumentation is supressed', () => { const spanContext = { traceId: 'd4cda95b652f4a1592b449d5929fda1b', spanId: '6e0c63257de34c92', @@ -205,6 +154,7 @@ describe('SentryPropagator', () => { const context = suppressTracing(trace.setSpanContext(ROOT_CONTEXT, spanContext)); propagator.inject(context, carrier, defaultTextMapSetter); expect(carrier[SENTRY_TRACE_HEADER]).toBe(undefined); + expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(undefined); }); }); });