diff --git a/lib/datadog/opentelemetry/sdk/propagator.rb b/lib/datadog/opentelemetry/sdk/propagator.rb index 35e9ea99b57..f45e4e9d7f4 100644 --- a/lib/datadog/opentelemetry/sdk/propagator.rb +++ b/lib/datadog/opentelemetry/sdk/propagator.rb @@ -41,8 +41,11 @@ 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. + # 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) @@ -78,14 +81,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..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) @@ -85,7 +86,6 @@ def start_datadog_span(span) name, kwargs = span_arguments(span, attributes) datadog_span = Tracing.trace(name, **kwargs) - datadog_span.set_error([nil, span.status.description]) unless span.status.ok? datadog_span.set_tags(span.attributes) @@ -143,6 +143,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 cb3edde7862..84052145095 100644 --- a/lib/datadog/tracing/span_operation.rb +++ b/lib/datadog/tracing/span_operation.rb @@ -48,7 +48,8 @@ def initialize( tags: nil, trace_id: nil, type: nil, - links: nil + links: nil, + id: nil ) # Ensure dynamically created strings are UTF-8 encoded. # @@ -60,7 +61,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..a16384ed857 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -114,6 +114,7 @@ def initialize( # @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 +131,7 @@ def trace( start_time: nil, tags: nil, type: nil, + id: nil, &block ) return skip_trace(name, &block) unless enabled @@ -162,6 +164,7 @@ def trace( tags: tags, type: type, _trace: trace, + id: id, &block ) end @@ -178,7 +181,8 @@ def trace( start_time: start_time, tags: tags, type: type, - _trace: trace + _trace: trace, + id: id ) end end @@ -375,6 +379,7 @@ def start_span( tags: nil, type: nil, _trace: nil, + id: nil, &block ) trace = _trace || start_trace(continue_from: continue_from) @@ -391,6 +396,7 @@ def start_span( service: service, tags: resolve_tags(tags), type: type, + id: id, &block ) else @@ -403,7 +409,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 f006a056fb9..0e0cfaa3286 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 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 @@ -698,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 @@ -802,7 +815,7 @@ def headers context 'with TraceContext headers' do let(:carrier) do { - 'traceparent' => '00-00000000000000001111111111111111-2222222222222222-01' + 'traceparent' => '00-11111111111111111111111111111111-2222222222222222-01' } end @@ -817,7 +830,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/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| diff --git a/spec/datadog/tracing/span_operation_spec.rb b/spec/datadog/tracing/span_operation_spec.rb index a8de6c79873..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 } 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)