Merge pull request #3709 from DataDog/munir/fix-otel-span-ids

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

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
 

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

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
 

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}" }

lib/datadog/tracing/tracer.rb

@@ -114,6 +114,7 @@ def initialize(
       # @param [Time] start_time time which the span should have started.
       # @param [Hash<String,String>] 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)

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

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|

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 }

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)