Skip to content

Commit

Permalink
fix(otel): Use span.toTraceParent to set sentry-trace in propagator (#…
Browse files Browse the repository at this point in the history 
…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 <aprasad@sentry.io>
  • Loading branch information
@sl0thentr0py @AbhiPrasad
sl0thentr0py and AbhiPrasad committed Nov 10, 2022
1 parent 7c61239 commit b4a4342
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 71 deletions.
19 changes: 9 additions & 10 deletions packages/opentelemetry-node/src/propagator.ts
View file
Expand Up @@ -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);
}
}
}
}
Expand Down
72 changes: 11 additions & 61 deletions packages/opentelemetry-node/test/propagator.test.ts
View file
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -166,6 +112,7 @@ describe('SentryPropagator', () => {
sampled: true,
},
undefined,
undefined,
],
[
'should NOT set baggage header when spanId is empty',
Expand All @@ -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',
Expand All @@ -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);
});
});
});
Expand Down

0 comments on commit b4a4342

Please sign in to comment.