From 0712e3f65ef13d4e9f7d528e7e09abb9af113aaa Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Wed, 12 Jun 2024 01:45:46 -0400 Subject: [PATCH 1/8] fix: ensure the correct ids are serialized and propagated profiling test updates fix profiling tests 2 revert Apply suggestions from code review lint --- lib/datadog/opentelemetry/sdk/propagator.rb | 13 +++---------- .../opentelemetry/sdk/span_processor.rb | 4 ++++ lib/datadog/tracing/span_operation.rb | 12 ++---------- spec/datadog/opentelemetry_spec.rb | 15 +++++++++++++-- .../collectors/thread_context_spec.rb | 18 +++++++++--------- spec/datadog/tracing/span_operation_spec.rb | 18 ++++++++++++++++++ 6 files changed, 49 insertions(+), 31 deletions(-) diff --git a/lib/datadog/opentelemetry/sdk/propagator.rb b/lib/datadog/opentelemetry/sdk/propagator.rb index 35e9ea99b57..aa2f6577c5f 100644 --- a/lib/datadog/opentelemetry/sdk/propagator.rb +++ b/lib/datadog/opentelemetry/sdk/propagator.rb @@ -41,8 +41,9 @@ def extract( digest = @datadog_propagator.extract(carrier) return context unless digest - trace_id = to_otel_id(digest.trace_id) - span_id = to_otel_id(digest.span_id) + # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. + trace_id = [format('%032x', digest.trace_id)].pack('H32') + span_id = [format('%016x', digest.span_id)].pack('H16') if digest.trace_state || digest.trace_flags trace_flags = ::OpenTelemetry::Trace::TraceFlags.from_byte(digest.trace_flags) @@ -78,14 +79,6 @@ def extract( def fields [] end - - private - - # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. - # This method currently converts an unsigned 64-bit Integer to a binary String. - def to_otel_id(dd_id) - Array(dd_id).pack('Q') - end end end end diff --git a/lib/datadog/opentelemetry/sdk/span_processor.rb b/lib/datadog/opentelemetry/sdk/span_processor.rb index ab7ef49ba2f..b6b368b582e 100644 --- a/lib/datadog/opentelemetry/sdk/span_processor.rb +++ b/lib/datadog/opentelemetry/sdk/span_processor.rb @@ -85,6 +85,10 @@ def start_datadog_span(span) name, kwargs = span_arguments(span, attributes) datadog_span = Tracing.trace(name, **kwargs) + # The Datadog span must have the same ID as the OpenTelemetry span + # DEV: We need to set the span ID after the span is created + datadog_span.id = span.context.hex_span_id.to_i(16) + datadog_span.trace_id = span.context.hex_trace_id.to_i(16) datadog_span.set_error([nil, span.status.description]) unless span.status.ok? datadog_span.set_tags(span.attributes) diff --git a/lib/datadog/tracing/span_operation.rb b/lib/datadog/tracing/span_operation.rb index cb3edde7862..fd3173a53c8 100644 --- a/lib/datadog/tracing/span_operation.rb +++ b/lib/datadog/tracing/span_operation.rb @@ -25,16 +25,8 @@ class SpanOperation # Span attributes # NOTE: In the future, we should drop the me - attr_reader \ - :end_time, - :id, - :name, - :parent_id, - :resource, - :service, - :start_time, - :trace_id, - :type + attr_accessor :id, :trace_id + attr_reader :end_time, :name, :parent_id, :resource, :service, :start_time, :type attr_accessor :links, :status def initialize( diff --git a/spec/datadog/opentelemetry_spec.rb b/spec/datadog/opentelemetry_spec.rb index f006a056fb9..6770eb5bdc7 100644 --- a/spec/datadog/opentelemetry_spec.rb +++ b/spec/datadog/opentelemetry_spec.rb @@ -329,6 +329,17 @@ expect(parent).to be_root_span expect(child.parent_id).to eq(parent.id) end + + it 'the underlying datadog spans has the same span_id as the otel spans' do + existing_span.finish + start_span.finish + # parent otel span generates a Datadog span with the same ids + expect(existing_span.context.hex_trace_id.to_i(16)).to eq(parent.trace_id) + expect(existing_span.context.hex_span_id.to_i(16)).to eq(parent.id) + # child otel span generates a Datadog span with the same ids + expect(start_span.context.hex_trace_id.to_i(16)).to eq(child.trace_id) + expect(start_span.context.hex_span_id.to_i(16)).to eq(child.id) + end end end @@ -802,7 +813,7 @@ def headers context 'with TraceContext headers' do let(:carrier) do { - 'traceparent' => '00-00000000000000001111111111111111-2222222222222222-01' + 'traceparent' => '00-11111111111111111111111111111111-2222222222222222-01' } end @@ -817,7 +828,7 @@ def headers otel_tracer.in_span('otel') {} end - expect(span.trace_id).to eq(0x00000000000000001111111111111111) + expect(span.trace_id).to eq(0x11111111111111111111111111111111) expect(span.parent_id).to eq(0x2222222222222222) end end diff --git a/spec/datadog/profiling/collectors/thread_context_spec.rb b/spec/datadog/profiling/collectors/thread_context_spec.rb index 44323ce1665..5d7d9edf772 100644 --- a/spec/datadog/profiling/collectors/thread_context_spec.rb +++ b/spec/datadog/profiling/collectors/thread_context_spec.rb @@ -547,9 +547,9 @@ def self.otel_sdk_available? let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do - @t1_span_id = Datadog::Tracing.correlation.span_id - @t1_local_root_span_id = Datadog::Tracing.correlation.span_id + otel_tracer.in_span('profiler.test') do |span| + @t1_span_id = span.context.hex_span_id.to_i(16) + @t1_local_root_span_id = span.context.hex_span_id.to_i(16) ready_queue << true sleep end @@ -574,12 +574,12 @@ def self.otel_sdk_available? context 'when there are multiple otel spans nested' do let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do - @t1_local_root_span_id = Datadog::Tracing.correlation.span_id + otel_tracer.in_span('profiler.test') do |root| + @t1_local_root_span_id = root.context.hex_span_id.to_i(16) otel_tracer.in_span('profiler.test.nested.1') do otel_tracer.in_span('profiler.test.nested.2') do - otel_tracer.in_span('profiler.test.nested.3') do - @t1_span_id = Datadog::Tracing.correlation.span_id + otel_tracer.in_span('profiler.test.nested.3') do |leaf| + @t1_span_id = leaf.context.hex_span_id.to_i(16) ready_queue << true sleep end @@ -641,8 +641,8 @@ def self.otel_sdk_available? context 'when top-level span is started from otel' do let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do - @t1_local_root_span_id = Datadog::Tracing.correlation.span_id + otel_tracer.in_span('profiler.test') do |root| + @t1_local_root_span_id = root.context.hex_span_id.to_i(16) otel_tracer.in_span('profiler.test.nested.1') do Datadog::Tracing.trace('profiler.test.nested.2') do otel_tracer.in_span('profiler.test.nested.3') do diff --git a/spec/datadog/tracing/span_operation_spec.rb b/spec/datadog/tracing/span_operation_spec.rb index a8de6c79873..6892996b2d8 100644 --- a/spec/datadog/tracing/span_operation_spec.rb +++ b/spec/datadog/tracing/span_operation_spec.rb @@ -319,6 +319,24 @@ end end + describe '#id=' do + subject!(:id=) { span_op.id = id } + + context 'with a valid 64 bit integer' do + let(:id) { 2 ^ 64 - 1 } + it { expect(span_op.id).to eq(id) } + end + end + + describe '#trace_id=' do + subject!(:trace_id=) { span_op.trace_id = trace_id } + + context 'with a valid 128 bit integer' do + let(:trace_id) { 2 ^ 128 - 1 } + it { expect(span_op.trace_id).to eq(trace_id) } + end + end + describe '#measure' do subject(:measure) { span_op.measure(&block) } From 467940515d24eca872a24a0a00236dd0c0223143 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 13 Jun 2024 15:32:09 -0400 Subject: [PATCH 2/8] pass id down to span_operation --- lib/datadog/opentelemetry/sdk/span_processor.rb | 8 +++----- lib/datadog/tracing.rb | 2 ++ lib/datadog/tracing/span_operation.rb | 5 +++-- lib/datadog/tracing/trace_operation.rb | 10 +++++++--- lib/datadog/tracing/tracer.rb | 12 +++++++++--- spec/datadog/opentelemetry_spec.rb | 4 ---- spec/datadog/tracing_spec.rb | 5 ++++- 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/lib/datadog/opentelemetry/sdk/span_processor.rb b/lib/datadog/opentelemetry/sdk/span_processor.rb index b6b368b582e..2dd37d4694f 100644 --- a/lib/datadog/opentelemetry/sdk/span_processor.rb +++ b/lib/datadog/opentelemetry/sdk/span_processor.rb @@ -85,11 +85,6 @@ def start_datadog_span(span) name, kwargs = span_arguments(span, attributes) datadog_span = Tracing.trace(name, **kwargs) - # The Datadog span must have the same ID as the OpenTelemetry span - # DEV: We need to set the span ID after the span is created - datadog_span.id = span.context.hex_span_id.to_i(16) - datadog_span.trace_id = span.context.hex_trace_id.to_i(16) - datadog_span.set_error([nil, span.status.description]) unless span.status.ok? datadog_span.set_tags(span.attributes) @@ -147,6 +142,9 @@ def span_arguments(span, attributes) kwargs[:tags] = attributes.to_h + # DEV: The datadog span must have the same ID as the OpenTelemetry span + kwargs[:id] = span.context.hex_span_id.to_i(16) + [name, kwargs] end diff --git a/lib/datadog/tracing.rb b/lib/datadog/tracing.rb index 4acb15e2ffe..9be37a5da4b 100644 --- a/lib/datadog/tracing.rb +++ b/lib/datadog/tracing.rb @@ -23,6 +23,7 @@ def trace( start_time: nil, tags: nil, type: nil, + id: nil, &block ) @@ -35,6 +36,7 @@ def trace( start_time: start_time, tags: tags, type: type, + id: id, &block ) end diff --git a/lib/datadog/tracing/span_operation.rb b/lib/datadog/tracing/span_operation.rb index fd3173a53c8..c381a798f5c 100644 --- a/lib/datadog/tracing/span_operation.rb +++ b/lib/datadog/tracing/span_operation.rb @@ -40,7 +40,8 @@ def initialize( tags: nil, trace_id: nil, type: nil, - links: nil + links: nil, + id: nil ) # Ensure dynamically created strings are UTF-8 encoded. # @@ -52,7 +53,7 @@ def initialize( self.type = type self.resource = resource - @id = Tracing::Utils.next_id + @id = id.nil? ? Tracing::Utils.next_id : id @parent_id = parent_id || 0 @trace_id = trace_id || Tracing::Utils::TraceId.next_id diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 615f8c2d68b..9ebaf46ef85 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -181,6 +181,7 @@ def measure( start_time: nil, tags: nil, type: nil, + id: nil, &block ) # Don't allow more span measurements if the @@ -197,7 +198,8 @@ def measure( service: service, start_time: start_time, tags: tags, - type: type + type: type, + id: id ) # Start span measurement @@ -212,7 +214,8 @@ def build_span( service: nil, start_time: nil, tags: nil, - type: nil + type: nil, + id: nil ) begin # Resolve span options: @@ -249,7 +252,8 @@ def build_span( start_time: start_time, tags: tags, trace_id: trace_id, - type: type + type: type, + id: id ) rescue StandardError => e Datadog.logger.debug { "Failed to build new span: #{e}" } diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index b9f076bb346..d88b150a1c1 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -113,7 +113,7 @@ def initialize( # @param [String] service the service name for this span. # @param [Time] start_time time which the span should have started. # @param [Hash] tags extra tags which should be added to the span. - # @param [String] type the type of the span. See {Datadog::Tracing::Metadata::Ext::AppTypes}. + # @param [Integer] the id of the new span. # @return [Object] If a block is provided, returns the result of the block execution. # @return [Datadog::Tracing::SpanOperation] If no block is provided, returns the active, # unfinished {Datadog::Tracing::SpanOperation}. @@ -130,6 +130,7 @@ def trace( start_time: nil, tags: nil, type: nil, + id: nil, &block ) return skip_trace(name, &block) unless enabled @@ -162,6 +163,7 @@ def trace( tags: tags, type: type, _trace: trace, + id: id, &block ) end @@ -178,7 +180,8 @@ def trace( start_time: start_time, tags: tags, type: type, - _trace: trace + _trace: trace, + id: id ) end end @@ -375,6 +378,7 @@ def start_span( tags: nil, type: nil, _trace: nil, + id: nil, &block ) trace = _trace || start_trace(continue_from: continue_from) @@ -391,6 +395,7 @@ def start_span( service: service, tags: resolve_tags(tags), type: type, + id: id, &block ) else @@ -403,7 +408,8 @@ def start_span( service: service, start_time: start_time, tags: resolve_tags(tags), - type: type + type: type, + id: id ) span.start(start_time) diff --git a/spec/datadog/opentelemetry_spec.rb b/spec/datadog/opentelemetry_spec.rb index 6770eb5bdc7..b5233559cb7 100644 --- a/spec/datadog/opentelemetry_spec.rb +++ b/spec/datadog/opentelemetry_spec.rb @@ -333,11 +333,7 @@ it 'the underlying datadog spans has the same span_id as the otel spans' do existing_span.finish start_span.finish - # parent otel span generates a Datadog span with the same ids - expect(existing_span.context.hex_trace_id.to_i(16)).to eq(parent.trace_id) expect(existing_span.context.hex_span_id.to_i(16)).to eq(parent.id) - # child otel span generates a Datadog span with the same ids - expect(start_span.context.hex_trace_id.to_i(16)).to eq(child.trace_id) expect(start_span.context.hex_span_id.to_i(16)).to eq(child.id) end end diff --git a/spec/datadog/tracing_spec.rb b/spec/datadog/tracing_spec.rb index 08f0633dc2f..20237017b6e 100644 --- a/spec/datadog/tracing_spec.rb +++ b/spec/datadog/tracing_spec.rb @@ -69,6 +69,7 @@ start_time: start_time, tags: tags, type: type, + id: id, &block ) end @@ -81,6 +82,7 @@ let(:start_time) { double('start_time') } let(:tags) { double('tags') } let(:type) { double('type') } + let(:id) { double(1) } let(:block) { -> {} } it 'delegates to the tracer' do @@ -93,7 +95,8 @@ service: service, start_time: start_time, tags: tags, - type: type + type: type, + id: id ) { |&b| expect(b).to be(block) } .and_return(returned) expect(trace).to eq(returned) From a70c1fceb30f053033154581f10cfcbbddde664c Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 13 Jun 2024 15:40:46 -0400 Subject: [PATCH 3/8] pack integers not strings --- lib/datadog/opentelemetry/sdk/propagator.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/datadog/opentelemetry/sdk/propagator.rb b/lib/datadog/opentelemetry/sdk/propagator.rb index aa2f6577c5f..9696c22c87a 100644 --- a/lib/datadog/opentelemetry/sdk/propagator.rb +++ b/lib/datadog/opentelemetry/sdk/propagator.rb @@ -41,9 +41,9 @@ def extract( digest = @datadog_propagator.extract(carrier) return context unless digest - # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. - trace_id = [format('%032x', digest.trace_id)].pack('H32') - span_id = [format('%016x', digest.span_id)].pack('H16') + # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. + trace_id = [digest.trace_id >> 64, digest.trace_id & 0xFFFFFFFFFFFFFFFF].pack('Q>Q>') # 128-bit unsigned, big-endian integer + span_id = [digest.span_id].pack('Q>') # 64-bit unsigned, big-endian integer if digest.trace_state || digest.trace_flags trace_flags = ::OpenTelemetry::Trace::TraceFlags.from_byte(digest.trace_flags) From d3e25d7e5a237416a846f10b4fc4a9cf908d66fa Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 13 Jun 2024 16:30:41 -0400 Subject: [PATCH 4/8] ensure the correct trace_id is propagated --- lib/datadog/opentelemetry/sdk/propagator.rb | 8 +++++--- lib/datadog/opentelemetry/sdk/span_processor.rb | 3 ++- spec/datadog/opentelemetry_spec.rb | 12 +++++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/datadog/opentelemetry/sdk/propagator.rb b/lib/datadog/opentelemetry/sdk/propagator.rb index 9696c22c87a..f45e4e9d7f4 100644 --- a/lib/datadog/opentelemetry/sdk/propagator.rb +++ b/lib/datadog/opentelemetry/sdk/propagator.rb @@ -41,9 +41,11 @@ def extract( digest = @datadog_propagator.extract(carrier) return context unless digest - # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. - trace_id = [digest.trace_id >> 64, digest.trace_id & 0xFFFFFFFFFFFFFFFF].pack('Q>Q>') # 128-bit unsigned, big-endian integer - span_id = [digest.span_id].pack('Q>') # 64-bit unsigned, big-endian integer + # Converts the {Numeric} Datadog id object to OpenTelemetry's byte array format. + # 128-bit unsigned, big-endian integer + trace_id = [digest.trace_id >> 64, digest.trace_id & 0xFFFFFFFFFFFFFFFF].pack('Q>Q>') + # 64-bit unsigned, big-endian integer + span_id = [digest.span_id].pack('Q>') if digest.trace_state || digest.trace_flags trace_flags = ::OpenTelemetry::Trace::TraceFlags.from_byte(digest.trace_flags) diff --git a/lib/datadog/opentelemetry/sdk/span_processor.rb b/lib/datadog/opentelemetry/sdk/span_processor.rb index 2dd37d4694f..65af6e621da 100644 --- a/lib/datadog/opentelemetry/sdk/span_processor.rb +++ b/lib/datadog/opentelemetry/sdk/span_processor.rb @@ -70,7 +70,8 @@ def create_matching_datadog_span(span, parent_context) if parent_context.trace Tracing.send(:tracer).send(:call_context).activate!(parent_context.ensure_trace) else - Tracing.continue_trace!(nil) + otel_trace_id = span.context.hex_trace_id.to_i(16) + Tracing.continue_trace!(Datadog::Tracing::TraceDigest.new(trace_id: otel_trace_id, span_remote: false)) end datadog_span = start_datadog_span(span) diff --git a/spec/datadog/opentelemetry_spec.rb b/spec/datadog/opentelemetry_spec.rb index b5233559cb7..0e0cfaa3286 100644 --- a/spec/datadog/opentelemetry_spec.rb +++ b/spec/datadog/opentelemetry_spec.rb @@ -330,11 +330,15 @@ expect(child.parent_id).to eq(parent.id) end - it 'the underlying datadog spans has the same span_id as the otel spans' do + it 'the underlying datadog spans has the same ids as the otel spans' do existing_span.finish start_span.finish + # Verify Span IDs are the same expect(existing_span.context.hex_span_id.to_i(16)).to eq(parent.id) expect(start_span.context.hex_span_id.to_i(16)).to eq(child.id) + # Verify Trace IDs are the same + expect(existing_span.context.hex_trace_id.to_i(16)).to eq(parent.trace_id) + expect(start_span.context.hex_trace_id.to_i(16)).to eq(child.trace_id) end end end @@ -705,12 +709,14 @@ ::OpenTelemetry.propagation.inject(carrier) end let(:carrier) { {} } + let(:trace_id) { Datadog::Tracing.active_trace.id } def headers { 'x-datadog-parent-id' => Datadog::Tracing.active_span.id.to_s, 'x-datadog-sampling-priority' => '1', - 'x-datadog-tags' => '_dd.p.dm=-0,_dd.p.tid=' + - high_order_hex_trace_id(Datadog::Tracing.active_trace.id), + 'x-datadog-tags' => '_dd.p.dm=-0' + ( + trace_id < 2**64 ? '' : ",_dd.p.tid=#{high_order_hex_trace_id(Datadog::Tracing.active_trace.id)}" + ), 'x-datadog-trace-id' => low_order_trace_id(Datadog::Tracing.active_trace.id).to_s, } end From 6aa2dafdea7f65ae41dc42f417813721870570a7 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 13 Jun 2024 16:31:30 -0400 Subject: [PATCH 5/8] revert profiling test changes --- .../collectors/thread_context_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/datadog/profiling/collectors/thread_context_spec.rb b/spec/datadog/profiling/collectors/thread_context_spec.rb index 5d7d9edf772..44323ce1665 100644 --- a/spec/datadog/profiling/collectors/thread_context_spec.rb +++ b/spec/datadog/profiling/collectors/thread_context_spec.rb @@ -547,9 +547,9 @@ def self.otel_sdk_available? let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do |span| - @t1_span_id = span.context.hex_span_id.to_i(16) - @t1_local_root_span_id = span.context.hex_span_id.to_i(16) + otel_tracer.in_span('profiler.test') do + @t1_span_id = Datadog::Tracing.correlation.span_id + @t1_local_root_span_id = Datadog::Tracing.correlation.span_id ready_queue << true sleep end @@ -574,12 +574,12 @@ def self.otel_sdk_available? context 'when there are multiple otel spans nested' do let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do |root| - @t1_local_root_span_id = root.context.hex_span_id.to_i(16) + otel_tracer.in_span('profiler.test') do + @t1_local_root_span_id = Datadog::Tracing.correlation.span_id otel_tracer.in_span('profiler.test.nested.1') do otel_tracer.in_span('profiler.test.nested.2') do - otel_tracer.in_span('profiler.test.nested.3') do |leaf| - @t1_span_id = leaf.context.hex_span_id.to_i(16) + otel_tracer.in_span('profiler.test.nested.3') do + @t1_span_id = Datadog::Tracing.correlation.span_id ready_queue << true sleep end @@ -641,8 +641,8 @@ def self.otel_sdk_available? context 'when top-level span is started from otel' do let(:t1) do Thread.new(ready_queue, otel_tracer) do |ready_queue, otel_tracer| - otel_tracer.in_span('profiler.test') do |root| - @t1_local_root_span_id = root.context.hex_span_id.to_i(16) + otel_tracer.in_span('profiler.test') do + @t1_local_root_span_id = Datadog::Tracing.correlation.span_id otel_tracer.in_span('profiler.test.nested.1') do Datadog::Tracing.trace('profiler.test.nested.2') do otel_tracer.in_span('profiler.test.nested.3') do From 5c9ee721cb9bf5c30f5c1fb21ac832c542dd2a7d Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 14 Jun 2024 14:28:43 -0400 Subject: [PATCH 6/8] revert changes --- lib/datadog/tracing/span_operation.rb | 12 ++++++++++-- lib/datadog/tracing/tracer.rb | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/datadog/tracing/span_operation.rb b/lib/datadog/tracing/span_operation.rb index c381a798f5c..84052145095 100644 --- a/lib/datadog/tracing/span_operation.rb +++ b/lib/datadog/tracing/span_operation.rb @@ -25,8 +25,16 @@ class SpanOperation # Span attributes # NOTE: In the future, we should drop the me - attr_accessor :id, :trace_id - attr_reader :end_time, :name, :parent_id, :resource, :service, :start_time, :type + attr_reader \ + :end_time, + :id, + :name, + :parent_id, + :resource, + :service, + :start_time, + :trace_id, + :type attr_accessor :links, :status def initialize( diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index d88b150a1c1..a16384ed857 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -113,6 +113,7 @@ def initialize( # @param [String] service the service name for this span. # @param [Time] start_time time which the span should have started. # @param [Hash] tags extra tags which should be added to the span. + # @param [String] type the type of the span. See {Datadog::Tracing::Metadata::Ext::AppTypes}. # @param [Integer] the id of the new span. # @return [Object] If a block is provided, returns the result of the block execution. # @return [Datadog::Tracing::SpanOperation] If no block is provided, returns the active, From a22a49cd175e3ec6309521321cb92b170c606d57 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 14 Jun 2024 15:04:27 -0400 Subject: [PATCH 7/8] update test --- spec/datadog/tracing/span_operation_spec.rb | 32 +++++++++------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/spec/datadog/tracing/span_operation_spec.rb b/spec/datadog/tracing/span_operation_spec.rb index 6892996b2d8..6b10bebeb22 100644 --- a/spec/datadog/tracing/span_operation_spec.rb +++ b/spec/datadog/tracing/span_operation_spec.rb @@ -224,6 +224,20 @@ end end + describe ':id' do + let(:options) { { id: id } } + + context 'that is nil' do + let(:id) { nil } + it { is_expected.to have_attributes(id: kind_of(Integer)) } + end + + context 'that is an Integer' do + let(:id) { instance_double(Integer) } + it { is_expected.to have_attributes(id: id) } + end + end + describe ':resource' do it_behaves_like 'a string property' do let(:property) { :resource } @@ -319,24 +333,6 @@ end end - describe '#id=' do - subject!(:id=) { span_op.id = id } - - context 'with a valid 64 bit integer' do - let(:id) { 2 ^ 64 - 1 } - it { expect(span_op.id).to eq(id) } - end - end - - describe '#trace_id=' do - subject!(:trace_id=) { span_op.trace_id = trace_id } - - context 'with a valid 128 bit integer' do - let(:trace_id) { 2 ^ 128 - 1 } - it { expect(span_op.trace_id).to eq(trace_id) } - end - end - describe '#measure' do subject(:measure) { span_op.measure(&block) } From a99f0e51af09d45e57a851640c7407fdb2aa7f80 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Mon, 17 Jun 2024 14:45:29 +0100 Subject: [PATCH 8/8] Fix matching of decoded span ids from profiles Pprof decodes numeric labels as signed 64-bit values BUT as a slight cheat both the profiler and the backend interpret them as unsigned 64-bit values so we can represent the full 64-bit values used for span ids in traces. But we were not accounting for this in our tests, so the decoded pprofs we use for testing were not matching the expected values. I've tweaked the pprof test reading code to account for this. --- spec/datadog/profiling/spec_helper.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/spec/datadog/profiling/spec_helper.rb b/spec/datadog/profiling/spec_helper.rb index 24757fab0d6..beaf0b5c67d 100644 --- a/spec/datadog/profiling/spec_helper.rb +++ b/spec/datadog/profiling/spec_helper.rb @@ -53,10 +53,8 @@ def samples_from_pprof(pprof_data) sample.location_id.map { |location_id| decode_frame_from_pprof(decoded_profile, location_id) }, pretty_sample_types.zip(sample.value).to_h, sample.label.map do |it| - [ - string_table[it.key].to_sym, - it.num == 0 ? string_table[it.str] : it.num, - ] + key = string_table[it.key].to_sym + [key, (it.num == 0 ? string_table[it.str] : ProfileHelpers.maybe_fix_label_range(key, it.num))] end.to_h, ).freeze end @@ -103,6 +101,16 @@ def build_stack_recorder( timeline_enabled: timeline_enabled, ) end + + def self.maybe_fix_label_range(key, value) + if [:'local root span id', :'span id'].include?(key) && value < 0 + # pprof labels are defined to be decoded as signed values BUT the backend explicitly interprets these as unsigned + # 64-bit numbers so we can still use them for these ids without having to fall back to strings + value + 2**64 + else + value + end + end end RSpec.configure do |config|