From 4392f48d7df84e37a0459a85b454399a5da55092 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Apr 2021 13:24:24 -0400 Subject: [PATCH 1/2] chore: do not inject span context when instrumentation is suppressed --- .../src/baggage/propagation/HttpBaggage.ts | 3 ++- .../context/propagation/HttpTraceContext.ts | 3 ++- .../test/context/HttpTraceContext.test.ts | 18 ++++++++++++++++++ .../src/B3MultiPropagator.ts | 8 +++++++- .../src/B3Propagator.ts | 4 ++++ .../src/B3SinglePropagator.ts | 8 +++++++- .../test/B3MultiPropagator.test.ts | 18 ++++++++++++++++++ .../test/B3Propagator.test.ts | 19 +++++++++++++++++++ .../test/B3SinglePropagator.test.ts | 17 +++++++++++++++++ .../src/JaegerHttpTracePropagator.ts | 3 ++- .../test/JaegerHttpTracePropagator.test.ts | 16 ++++++++++++++++ 11 files changed, 112 insertions(+), 5 deletions(-) diff --git a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts index 8b2b309ac3..9452623b99 100644 --- a/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts +++ b/packages/opentelemetry-core/src/baggage/propagation/HttpBaggage.ts @@ -25,6 +25,7 @@ import { TextMapSetter, createBaggage, baggageEntryMetadataFromString, + isInstrumentationSuppressed, } from '@opentelemetry/api'; const KEY_PAIR_SEPARATOR = '='; @@ -49,7 +50,7 @@ export const MAX_TOTAL_LENGTH = 8192; export class HttpBaggage implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const baggage = getBaggage(context); - if (!baggage) return; + if (!baggage || isInstrumentationSuppressed(context)) return; const keyPairs = this._getKeyPairs(baggage) .filter((pair: string) => { return pair.length <= MAX_PER_NAME_VALUE_PAIRS; diff --git a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts index 735e8d2e44..cc214c9966 100644 --- a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts +++ b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts @@ -17,6 +17,7 @@ import { Context, getSpanContext, + isInstrumentationSuppressed, setSpanContext, SpanContext, TextMapGetter, @@ -73,7 +74,7 @@ export function parseTraceParent(traceParent: string): SpanContext | null { export class HttpTraceContext implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const spanContext = getSpanContext(context); - if (!spanContext) return; + if (!spanContext || isInstrumentationSuppressed(context)) return; const traceParent = `${VERSION}-${spanContext.traceId}-${ spanContext.spanId diff --git a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts index a732a471f9..4570a3b2f1 100644 --- a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts +++ b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts @@ -21,6 +21,7 @@ import { TraceFlags, getSpanContext, setSpanContext, + suppressInstrumentation, } from '@opentelemetry/api'; import { ROOT_CONTEXT } from '@opentelemetry/api'; import * as assert from 'assert'; @@ -78,6 +79,23 @@ describe('HttpTraceContext', () => { ); assert.deepStrictEqual(carrier[TRACE_STATE_HEADER], 'foo=bar,baz=qux'); }); + + it('should not set traceparent and tracestate header if instrumentation is suppressed', () => { + const spanContext: SpanContext = { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + traceState: new TraceState('foo=bar,baz=qux'), + }; + + httpTraceContext.inject( + suppressInstrumentation(setSpanContext(ROOT_CONTEXT, spanContext)), + carrier, + defaultTextMapSetter + ); + assert.deepStrictEqual(carrier[TRACE_PARENT_HEADER], undefined); + assert.deepStrictEqual(carrier[TRACE_STATE_HEADER], undefined); + }); }); describe('.extract()', () => { diff --git a/packages/opentelemetry-propagator-b3/src/B3MultiPropagator.ts b/packages/opentelemetry-propagator-b3/src/B3MultiPropagator.ts index cea1f4a445..dba3a70de3 100644 --- a/packages/opentelemetry-propagator-b3/src/B3MultiPropagator.ts +++ b/packages/opentelemetry-propagator-b3/src/B3MultiPropagator.ts @@ -17,6 +17,7 @@ import { Context, getSpanContext, + isInstrumentationSuppressed, isSpanContextValid, isValidSpanId, isValidTraceId, @@ -95,7 +96,12 @@ function getTraceFlags( export class B3MultiPropagator implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const spanContext = getSpanContext(context); - if (!spanContext || !isSpanContextValid(spanContext)) return; + if ( + !spanContext || + !isSpanContextValid(spanContext) || + isInstrumentationSuppressed(context) + ) + return; const debug = context.getValue(B3_DEBUG_FLAG_KEY); setter.set(carrier, X_B3_TRACE_ID, spanContext.traceId); diff --git a/packages/opentelemetry-propagator-b3/src/B3Propagator.ts b/packages/opentelemetry-propagator-b3/src/B3Propagator.ts index d0f1c96540..7b40ef0527 100644 --- a/packages/opentelemetry-propagator-b3/src/B3Propagator.ts +++ b/packages/opentelemetry-propagator-b3/src/B3Propagator.ts @@ -16,6 +16,7 @@ import { Context, + isInstrumentationSuppressed, TextMapGetter, TextMapPropagator, TextMapSetter, @@ -53,6 +54,9 @@ export class B3Propagator implements TextMapPropagator { } inject(context: Context, carrier: unknown, setter: TextMapSetter) { + if (isInstrumentationSuppressed(context)) { + return; + } this._inject(context, carrier, setter); } diff --git a/packages/opentelemetry-propagator-b3/src/B3SinglePropagator.ts b/packages/opentelemetry-propagator-b3/src/B3SinglePropagator.ts index b75f4cd302..acd36820e8 100644 --- a/packages/opentelemetry-propagator-b3/src/B3SinglePropagator.ts +++ b/packages/opentelemetry-propagator-b3/src/B3SinglePropagator.ts @@ -17,6 +17,7 @@ import { Context, getSpanContext, + isInstrumentationSuppressed, isSpanContextValid, isValidSpanId, isValidTraceId, @@ -52,7 +53,12 @@ function convertToTraceFlags(samplingState: string | undefined): TraceFlags { export class B3SinglePropagator implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const spanContext = getSpanContext(context); - if (!spanContext || !isSpanContextValid(spanContext)) return; + if ( + !spanContext || + !isSpanContextValid(spanContext) || + isInstrumentationSuppressed(context) + ) + return; const samplingState = context.getValue(B3_DEBUG_FLAG_KEY) || spanContext.traceFlags & 0x1; diff --git a/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts b/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts index 759651b59d..5480ac87a5 100644 --- a/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts +++ b/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts @@ -20,6 +20,7 @@ import { getSpanContext, setSpanContext, SpanContext, + suppressInstrumentation, TraceFlags, } from '@opentelemetry/api'; import { ROOT_CONTEXT } from '@opentelemetry/api'; @@ -138,6 +139,23 @@ describe('B3MultiPropagator', () => { assert.deepStrictEqual(carrier[X_B3_FLAGS], undefined); assert.deepStrictEqual(carrier[X_B3_PARENT_SPAN_ID], undefined); }); + + it('should not inject if instrumentation suppressed', () => { + const spanContext = { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + }; + b3Propagator.inject( + suppressInstrumentation(setSpanContext(ROOT_CONTEXT, spanContext)), + carrier, + defaultTextMapSetter + ); + assert.deepStrictEqual(carrier[X_B3_TRACE_ID], undefined); + assert.deepStrictEqual(carrier[X_B3_SPAN_ID], undefined); + assert.deepStrictEqual(carrier[X_B3_FLAGS], undefined); + assert.deepStrictEqual(carrier[X_B3_PARENT_SPAN_ID], undefined); + }); }); describe('.extract()', () => { diff --git a/packages/opentelemetry-propagator-b3/test/B3Propagator.test.ts b/packages/opentelemetry-propagator-b3/test/B3Propagator.test.ts index a25bd468d3..bf0f89ff38 100644 --- a/packages/opentelemetry-propagator-b3/test/B3Propagator.test.ts +++ b/packages/opentelemetry-propagator-b3/test/B3Propagator.test.ts @@ -23,6 +23,7 @@ import { getSpanContext, setSpanContext, ROOT_CONTEXT, + suppressInstrumentation, } from '@opentelemetry/api'; import { B3Propagator } from '../src/B3Propagator'; import { B3InjectEncoding } from '../src/types'; @@ -88,6 +89,24 @@ describe('B3Propagator', () => { assert.strictEqual(carrier[X_B3_SPAN_ID], 'e457b5a2e4d86bd1'); assert.strictEqual(carrier[X_B3_SAMPLED], '1'); }); + + it('should not inject if instrumentation suppressed', () => { + propagator = new B3Propagator(); + + const spanContext: SpanContext = { + traceId: '80f198ee56343ba864fe8b2a57d3eff7', + spanId: 'e457b5a2e4d86bd1', + traceFlags: TraceFlags.SAMPLED, + }; + + propagator.inject( + suppressInstrumentation(setSpanContext(ROOT_CONTEXT, spanContext)), + carrier, + defaultTextMapSetter + ); + + assert.strictEqual(carrier[B3_CONTEXT_HEADER], undefined); + }); }); describe('.extract()', () => { diff --git a/packages/opentelemetry-propagator-b3/test/B3SinglePropagator.test.ts b/packages/opentelemetry-propagator-b3/test/B3SinglePropagator.test.ts index feb9c2a3e6..4a6a65b8ac 100644 --- a/packages/opentelemetry-propagator-b3/test/B3SinglePropagator.test.ts +++ b/packages/opentelemetry-propagator-b3/test/B3SinglePropagator.test.ts @@ -23,6 +23,7 @@ import { setSpanContext, SpanContext, TraceFlags, + suppressInstrumentation, } from '@opentelemetry/api'; import { ROOT_CONTEXT } from '@opentelemetry/api'; import * as assert from 'assert'; @@ -123,6 +124,22 @@ describe('B3SinglePropagator', () => { assert.strictEqual(carrier[B3_CONTEXT_HEADER], undefined); }); + + it('does not inject if instrumentation is suppressed', () => { + const spanContext: SpanContext = { + traceId: '80f198ee56343ba864fe8b2a57d3eff7', + spanId: 'e457b5a2e4d86bd1', + traceFlags: TraceFlags.SAMPLED, + }; + + propagator.inject( + suppressInstrumentation(setSpanContext(ROOT_CONTEXT, spanContext)), + carrier, + defaultTextMapSetter + ); + + assert.strictEqual(carrier[B3_CONTEXT_HEADER], undefined); + }); }); describe('.extract', () => { diff --git a/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts b/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts index 267cafba02..2afa35113f 100644 --- a/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts +++ b/packages/opentelemetry-propagator-jaeger/src/JaegerHttpTracePropagator.ts @@ -23,6 +23,7 @@ import { TextMapGetter, TextMapPropagator, TextMapSetter, + isInstrumentationSuppressed, } from '@opentelemetry/api'; export const UBER_TRACE_ID_HEADER = 'uber-trace-id'; @@ -54,7 +55,7 @@ export class JaegerHttpTracePropagator implements TextMapPropagator { inject(context: Context, carrier: unknown, setter: TextMapSetter) { const spanContext = getSpanContext(context); - if (!spanContext) return; + if (!spanContext || isInstrumentationSuppressed(context)) return; const traceFlags = `0${(spanContext.traceFlags || TraceFlags.NONE).toString( 16 diff --git a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts index 763d8b9399..e8f39c9208 100644 --- a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts +++ b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts @@ -21,6 +21,7 @@ import { ROOT_CONTEXT, setSpanContext, SpanContext, + suppressInstrumentation, TextMapGetter, TraceFlags, } from '@opentelemetry/api'; @@ -78,6 +79,21 @@ describe('JaegerHttpTracePropagator', () => { 'd4cda95b652f4a1592b449d5929fda1b:6e0c63257de34c92:0:01' ); }); + + it('should not set uber trace id header if instrumentation suppressed', () => { + const spanContext: SpanContext = { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + }; + + jaegerHttpTracePropagator.inject( + suppressInstrumentation(setSpanContext(ROOT_CONTEXT, spanContext)), + carrier, + defaultTextMapSetter + ); + assert.deepStrictEqual(carrier[UBER_TRACE_ID_HEADER], undefined); + }); }); describe('.extract()', () => { From 907acce05cc7999222416ac1a382dc1f0155ef52 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Wed, 7 Apr 2021 14:31:59 -0400 Subject: [PATCH 2/2] chore: review comments --- .../test/context/HttpTraceContext.test.ts | 4 ++-- .../test/B3MultiPropagator.test.ts | 8 ++++---- .../test/JaegerHttpTracePropagator.test.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts index 4570a3b2f1..757dc688f8 100644 --- a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts +++ b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts @@ -93,8 +93,8 @@ describe('HttpTraceContext', () => { carrier, defaultTextMapSetter ); - assert.deepStrictEqual(carrier[TRACE_PARENT_HEADER], undefined); - assert.deepStrictEqual(carrier[TRACE_STATE_HEADER], undefined); + assert.strictEqual(carrier[TRACE_PARENT_HEADER], undefined); + assert.strictEqual(carrier[TRACE_STATE_HEADER], undefined); }); }); diff --git a/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts b/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts index 5480ac87a5..a415c22603 100644 --- a/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts +++ b/packages/opentelemetry-propagator-b3/test/B3MultiPropagator.test.ts @@ -151,10 +151,10 @@ describe('B3MultiPropagator', () => { carrier, defaultTextMapSetter ); - assert.deepStrictEqual(carrier[X_B3_TRACE_ID], undefined); - assert.deepStrictEqual(carrier[X_B3_SPAN_ID], undefined); - assert.deepStrictEqual(carrier[X_B3_FLAGS], undefined); - assert.deepStrictEqual(carrier[X_B3_PARENT_SPAN_ID], undefined); + assert.strictEqual(carrier[X_B3_TRACE_ID], undefined); + assert.strictEqual(carrier[X_B3_SPAN_ID], undefined); + assert.strictEqual(carrier[X_B3_FLAGS], undefined); + assert.strictEqual(carrier[X_B3_PARENT_SPAN_ID], undefined); }); }); diff --git a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts index e8f39c9208..21f81bf10b 100644 --- a/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts +++ b/packages/opentelemetry-propagator-jaeger/test/JaegerHttpTracePropagator.test.ts @@ -92,7 +92,7 @@ describe('JaegerHttpTracePropagator', () => { carrier, defaultTextMapSetter ); - assert.deepStrictEqual(carrier[UBER_TRACE_ID_HEADER], undefined); + assert.strictEqual(carrier[UBER_TRACE_ID_HEADER], undefined); }); });