From 097d163c12c6098213370cd8bd4dc33ec8d2522b Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 5 Feb 2025 14:48:15 -0500 Subject: [PATCH 1/7] remove ability to propagate baggage on its own to bandage app crash --- .../src/opentracing/propagation/text_map.js | 6 ++++-- .../test/opentracing/propagation/text_map.spec.js | 14 -------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 82bc9f2b30f..84fbfa61ddd 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -325,6 +325,9 @@ class TextMapPropagator { if (context === null) { context = extractedContext if (this._config.tracePropagationExtractFirst) { + if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { + if (context) this._extractBaggageItems(carrier, context) + } return context } } else { @@ -345,8 +348,7 @@ class TextMapPropagator { } if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { - context = context || new DatadogSpanContext() - this._extractBaggageItems(carrier, context) + if (context) this._extractBaggageItems(carrier, context) } return context || this._extractSqsdContext(carrier) diff --git a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js index 3e4f6aed3e8..624315c6cfa 100644 --- a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js +++ b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js @@ -451,20 +451,6 @@ describe('TextMapPropagator', () => { expect(spanContextD._baggageItems).to.deep.equal({}) }) - it('should extract baggage when it is the only propagation style', () => { - config = new Config({ - tracePropagationStyle: { - extract: ['baggage'] - } - }) - propagator = new TextMapPropagator(config) - const carrier = { - baggage: 'foo=bar' - } - const spanContext = propagator.extract(carrier) - expect(spanContext._baggageItems).to.deep.equal({ foo: 'bar' }) - }) - it('should convert signed IDs to unsigned', () => { textMap['x-datadog-trace-id'] = '-123' textMap['x-datadog-parent-id'] = '-456' From 70435b2242d46440b7015a418256d709d6eb2b79 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 5 Feb 2025 15:57:02 -0500 Subject: [PATCH 2/7] modify unit test --- .../test/opentracing/propagation/text_map.spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js index 624315c6cfa..1f8e28ec31d 100644 --- a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js +++ b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js @@ -451,6 +451,20 @@ describe('TextMapPropagator', () => { expect(spanContextD._baggageItems).to.deep.equal({}) }) + it('should not extract baggage when it is the only propagation style', () => { + config = new Config({ + tracePropagationStyle: { + extract: ['baggage'] + } + }) + propagator = new TextMapPropagator(config) + const carrier = { + baggage: 'foo=bar' + } + const spanContext = propagator.extract(carrier) + expect(spanContext).to.be.null + }) + it('should convert signed IDs to unsigned', () => { textMap['x-datadog-trace-id'] = '-123' textMap['x-datadog-parent-id'] = '-456' From 23fa992daa5499e62c126d0cf38ea4cb8c4900a0 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 5 Feb 2025 16:56:54 -0500 Subject: [PATCH 3/7] tidy up code --- packages/dd-trace/src/opentracing/propagation/text_map.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 84fbfa61ddd..166f5391685 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -326,7 +326,7 @@ class TextMapPropagator { context = extractedContext if (this._config.tracePropagationExtractFirst) { if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { - if (context) this._extractBaggageItems(carrier, context) + this._extractBaggageItems(carrier, context) } return context } @@ -348,7 +348,7 @@ class TextMapPropagator { } if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { - if (context) this._extractBaggageItems(carrier, context) + this._extractBaggageItems(carrier, context) } return context || this._extractSqsdContext(carrier) @@ -598,6 +598,7 @@ class TextMapPropagator { } _extractBaggageItems (carrier, spanContext) { + if (!carrier) return const baggages = carrier.baggage.split(',') for (const keyValue of baggages) { if (!keyValue.includes('=')) { From 1b0e21842fee61b867376352307d74ab186029fc Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 5 Feb 2025 17:03:09 -0500 Subject: [PATCH 4/7] add clarifying comment --- packages/dd-trace/test/opentracing/propagation/text_map.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js index 1f8e28ec31d..2c699b107a1 100644 --- a/packages/dd-trace/test/opentracing/propagation/text_map.spec.js +++ b/packages/dd-trace/test/opentracing/propagation/text_map.spec.js @@ -451,6 +451,7 @@ describe('TextMapPropagator', () => { expect(spanContextD._baggageItems).to.deep.equal({}) }) + // temporary test. On the contrary, it SHOULD extract baggage it('should not extract baggage when it is the only propagation style', () => { config = new Config({ tracePropagationStyle: { From afd99de82e991886d067fd29bc139ffef0dae75c Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Wed, 5 Feb 2025 20:46:07 -0500 Subject: [PATCH 5/7] code fix --- packages/dd-trace/src/opentracing/propagation/text_map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 166f5391685..6d84a7986cf 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -598,7 +598,7 @@ class TextMapPropagator { } _extractBaggageItems (carrier, spanContext) { - if (!carrier) return + if (!spanContext) return const baggages = carrier.baggage.split(',') for (const keyValue of baggages) { if (!keyValue.includes('=')) { From 9a1fa20131a94674799dc0d76844ac0186d53b16 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Thu, 6 Feb 2025 11:00:58 -0500 Subject: [PATCH 6/7] code clean up season 2 --- .../dd-trace/src/opentracing/propagation/text_map.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 6d84a7986cf..84574d61cfe 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -325,9 +325,7 @@ class TextMapPropagator { if (context === null) { context = extractedContext if (this._config.tracePropagationExtractFirst) { - if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { - this._extractBaggageItems(carrier, context) - } + this._extractBaggageItems(carrier, context) return context } } else { @@ -347,9 +345,7 @@ class TextMapPropagator { } } - if (this._hasPropagationStyle('extract', 'baggage') && carrier.baggage) { - this._extractBaggageItems(carrier, context) - } + this._extractBaggageItems(carrier, context) return context || this._extractSqsdContext(carrier) } @@ -598,6 +594,8 @@ class TextMapPropagator { } _extractBaggageItems (carrier, spanContext) { + if (!this._hasPropagationStyle('extract', 'baggage')) return + if (!carrier.baggage) return if (!spanContext) return const baggages = carrier.baggage.split(',') for (const keyValue of baggages) { From 213913f8cd18c3dcc53334836b492ae0f6aa7640 Mon Sep 17 00:00:00 2001 From: "Ida.Liu" Date: Thu, 6 Feb 2025 13:35:47 -0500 Subject: [PATCH 7/7] attempted code fix --- packages/dd-trace/src/opentracing/propagation/text_map.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dd-trace/src/opentracing/propagation/text_map.js b/packages/dd-trace/src/opentracing/propagation/text_map.js index 84574d61cfe..fd7d32760cb 100644 --- a/packages/dd-trace/src/opentracing/propagation/text_map.js +++ b/packages/dd-trace/src/opentracing/propagation/text_map.js @@ -595,7 +595,7 @@ class TextMapPropagator { _extractBaggageItems (carrier, spanContext) { if (!this._hasPropagationStyle('extract', 'baggage')) return - if (!carrier.baggage) return + if (!carrier || !carrier.baggage) return if (!spanContext) return const baggages = carrier.baggage.split(',') for (const keyValue of baggages) {