From 3ca9678dd716ef4e8817988c9cb90999f931e683 Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Tue, 28 Jan 2025 14:40:26 -0800 Subject: [PATCH 1/4] remove trace state propagation and add lineage validation --- .../propagator/xray/text_map_propagator.rb | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb index 1041643ac7..a224325d3a 100644 --- a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb +++ b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb @@ -18,7 +18,7 @@ module XRay # Propagates context in carriers in the xray single header format class TextMapPropagator XRAY_CONTEXT_KEY = 'X-Amzn-Trace-Id' - XRAY_CONTEXT_REGEX = /\ARoot=(?([a-z0-9\-]{35}))(?:;Parent=(?([a-z0-9]{16})))?(?:;Sampled=(?[01d](?![0-9a-f])))?(?:;(?.*))?\Z/ # rubocop:disable Lint/MixedRegexpCaptureTypes + XRAY_CONTEXT_REGEX = /\A(?:(?:Root=(?[a-z0-9\-]{35})|Parent=(?[a-z0-9]{16})|Sampled=(?[01d](?![0-9a-f]))|Lineage=(?[^;]+))(?:;|$)){1,4}\Z/ # rubocop:disable Lint/MixedRegexpCaptureTypes SAMPLED_VALUES = %w[1 d].freeze FIELDS = [XRAY_CONTEXT_KEY].freeze @@ -47,12 +47,14 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text trace_id: to_trace_id(match['trace_id']), span_id: to_span_id(match['span_id']), trace_flags: to_trace_flags(match['sampling_state']), - tracestate: to_trace_state(match['trace_state']), remote: true ) span = OpenTelemetry::Trace.non_recording_span(span_context) context = XRay.context_with_debug(context) if match['sampling_state'] == 'd' + if match['lineage'] && is_valid_lineage?(match['lineage']) + context = OpenTelemetry::Baggage.set_value('Lineage', match['lineage']) + end Trace.context_with_span(span, parent_context: context) rescue OpenTelemetry::Error context @@ -116,10 +118,19 @@ def to_trace_flags(sampling_state) end end - def to_trace_state(trace_state) - return nil unless trace_state + def is_valid_lineage?(lineage) + lineageSubstrings = lineage.split(':') + return false unless lineageSubstrings.length == 3 + + requestCounter = lineageSubstrings[0].to_i + hashedResourceId = lineageSubstrings[1] + loopCounter = lineageSubstrings[2].to_i - Trace::Tracestate.from_string(trace_state.tr(';', ',')) + hashedResourceId.match?(/\A[a-zA-Z0-9]{8}\z/) && + requestCounter >= 0 && + requestCounter <= 32767 && + loopCounter >= 0 && + loopCounter <= 255 end end end From f2731ac07ffb78cb82effe1e817aa669f9d7ac61 Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Tue, 28 Jan 2025 15:35:10 -0800 Subject: [PATCH 2/4] add lineage injection to trace header --- .../opentelemetry/propagator/xray/text_map_propagator.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb index a224325d3a..1e43a12bee 100644 --- a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb +++ b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb @@ -85,6 +85,12 @@ def inject(carrier, context: Context.current, setter: Context::Propagation.text_ xray_value = "Root=#{xray_trace_id};Parent=#{parent_id};Sampled=#{sampling_state}" + # Add lineage to xray_value if present in baggage + baggage = OpenTelemetry::Baggage.values(context: context) + if baggage.key?('Lineage') + xray_value += ";Lineage=#{baggage['Lineage']}" + end + setter.set(carrier, XRAY_CONTEXT_KEY, xray_value) nil end From 328f1f1a834832b5513e77cc52749727a601126b Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Wed, 29 Jan 2025 13:31:02 -0800 Subject: [PATCH 3/4] add unit tests --- .../xray/test/text_map_propagator_test.rb | 58 +++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/propagator/xray/test/text_map_propagator_test.rb b/propagator/xray/test/text_map_propagator_test.rb index 656d294fb0..6aad947cf2 100644 --- a/propagator/xray/test/text_map_propagator_test.rb +++ b/propagator/xray/test/text_map_propagator_test.rb @@ -14,9 +14,9 @@ let(:propagator) { OpenTelemetry::Propagator::XRay::TextMapPropagator.new } describe('#extract') do - it 'extracts context with trace id, span id, sampling flag, trace state' do + it 'extracts context with trace id, span id, sampling flag' do parent_context = OpenTelemetry::Context.empty - carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=1;Foo=Bar;Fizz=Buzz' } + carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=1' } context = propagator.extract(carrier, context: parent_context) extracted_context = OpenTelemetry::Trace.current_span(context).context @@ -24,34 +24,32 @@ _(extracted_context.hex_trace_id).must_equal('80f198eae56343ba864fe8b2a57d3eff') _(extracted_context.hex_span_id).must_equal('e457b5a2e4d86bd1') _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::SAMPLED) - _(extracted_context.tracestate.to_s).must_equal(OpenTelemetry::Trace::Tracestate.from_string('Foo=Bar,Fizz=Buzz').to_s) _(extracted_context).must_be(:remote?) end - it 'extracts context with trace id, span id, sampling flag' do + it 'extracts context with trace id, span id' do parent_context = OpenTelemetry::Context.empty - carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=1' } + carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1' } context = propagator.extract(carrier, context: parent_context) extracted_context = OpenTelemetry::Trace.current_span(context).context _(extracted_context.hex_trace_id).must_equal('80f198eae56343ba864fe8b2a57d3eff') _(extracted_context.hex_span_id).must_equal('e457b5a2e4d86bd1') - _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::SAMPLED) + _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::DEFAULT) _(extracted_context).must_be(:remote?) end - it 'extracts context with trace id, span id' do + it 'extracts context with lineage in header' do parent_context = OpenTelemetry::Context.empty - carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1' } + carrier = { 'X-Amzn-Trace-Id' => 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=1;Lineage=100:e3b0c442:11' } context = propagator.extract(carrier, context: parent_context) extracted_context = OpenTelemetry::Trace.current_span(context).context - _(extracted_context.hex_trace_id).must_equal('80f198eae56343ba864fe8b2a57d3eff') _(extracted_context.hex_span_id).must_equal('e457b5a2e4d86bd1') - _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::DEFAULT) - _(extracted_context).must_be(:remote?) + _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::SAMPLED) + _(OpenTelemetry::Baggage.value('Lineage', context: context)).must_equal('100:e3b0c442:11') end it 'converts debug flag to sampled' do @@ -89,6 +87,29 @@ _(context).must_equal(parent_context) end + it 'handles invalid lineage' do + invalid_lineages = [ + "", + "::", + "1::", + "1::1", + "1:badc0de:13", + ":fbadc0de:13", + "65535:fbadc0de:255", + ] + + invalid_lineages.each do |lineage| + parent_context = OpenTelemetry::Context.empty + carrier = { 'X-Amzn-Trace-Id' => "Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=1;Lineage=65535:fbadc0de:255" } + + context = propagator.extract(carrier, context: parent_context) + extracted_context = OpenTelemetry::Trace.current_span(context).context + _(extracted_context.hex_trace_id).must_equal('80f198eae56343ba864fe8b2a57d3eff') + _(extracted_context.hex_span_id).must_equal('e457b5a2e4d86bd1') + _(extracted_context.trace_flags).must_equal(OpenTelemetry::Trace::TraceFlags::SAMPLED) + _(OpenTelemetry::Baggage.value('Lineage', context: context)).must_be_nil + end + end end describe '#inject' do @@ -134,6 +155,21 @@ _(carrier['X-Amzn-Trace-Id']).must_equal(expected_xray) end + it 'injects lineage from baggage' do + context = create_context( + trace_id: '80f198eae56343ba864fe8b2a57d3eff', + span_id: 'e457b5a2e4d86bd1', + trace_flags: TraceFlags::DEFAULT + ) + context = OpenTelemetry::Baggage.set_value('Lineage', '100:e3b0c442:11', context: context) + + carrier = {} + propagator.inject(carrier, context: context) + + expected_xray = 'Root=1-80f198ea-e56343ba864fe8b2a57d3eff;Parent=e457b5a2e4d86bd1;Sampled=0;Lineage=100:e3b0c442:11' + _(carrier['X-Amzn-Trace-Id']).must_equal(expected_xray) + end + it 'no-ops if trace id invalid' do context = create_context( trace_id: '0' * 32, From f2731f2763ba6a28ef6dca42fa8bcd99ca21cc7f Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Thu, 30 Jan 2025 14:14:57 -0800 Subject: [PATCH 4/4] change lineage variable names --- .../propagator/xray/text_map_propagator.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb index 1e43a12bee..68ef70ad7f 100644 --- a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb +++ b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb @@ -128,15 +128,15 @@ def is_valid_lineage?(lineage) lineageSubstrings = lineage.split(':') return false unless lineageSubstrings.length == 3 - requestCounter = lineageSubstrings[0].to_i - hashedResourceId = lineageSubstrings[1] - loopCounter = lineageSubstrings[2].to_i - - hashedResourceId.match?(/\A[a-zA-Z0-9]{8}\z/) && - requestCounter >= 0 && - requestCounter <= 32767 && - loopCounter >= 0 && - loopCounter <= 255 + lineageCounter1 = lineageSubstrings[0].to_i + hashedString = lineageSubstrings[1] + lineageCounter2 = lineageSubstrings[2].to_i + + hashedString.match?(/\A[a-zA-Z0-9]{8}\z/) && + lineageCounter1 >= 0 && + lineageCounter1 <= 32767 && + lineageCounter2 >= 0 && + lineageCounter2 <= 255 end end end