fix(tracing): use established propagation context's span_id

Author: solnic

sentry-rails/spec/sentry/rails/capture_context_spec.rb

@@ -3,19 +3,25 @@
 require "spec_helper"
 
 RSpec.describe Sentry::Rails::CaptureContext do
-  # Records the current scope's trace_id every time it's called, so specs can
+  # Records the current scope's trace context every time it's called, so specs can
   # compare what a piece of middleware would see at different points in the stack.
   class CaptureContextSpecProbe
     def self.captured_trace_ids
       @captured_trace_ids ||= []
     end
 
+    def self.captured_span_ids
+      @captured_span_ids ||= []
+    end
+
     def initialize(app)
       @app = app
     end
 
     def call(env)
-      self.class.captured_trace_ids << Sentry.get_current_scope.get_trace_context[:trace_id]
+      trace_context = Sentry.get_current_scope.get_trace_context
+      self.class.captured_trace_ids << trace_context[:trace_id]
+      self.class.captured_span_ids << trace_context[:span_id]
       @app.call(env)
     end
   end
@@ -98,6 +104,7 @@ def call(env)
   context "when composed with CaptureExceptions", type: :request do
     before do
       CaptureContextSpecProbe.captured_trace_ids.clear
+      CaptureContextSpecProbe.captured_span_ids.clear
     end
 
     context "without tracing enabled" do
@@ -127,6 +134,15 @@ def call(env)
         end
       end
 
+      it "points a span_id captured before CaptureExceptions at the started transaction" do
+        get "/world"
+
+        transaction = Sentry.get_current_client.transport.events.last
+        early_span_id = CaptureContextSpecProbe.captured_span_ids.first
+
+        expect(early_span_id).to eq(transaction.contexts.dig(:trace, :span_id))
+      end
+
       it "keeps the same trace_id from before CaptureExceptions through the started transaction" do
         get "/world"
 

sentry-ruby/lib/sentry/hub.rb

@@ -123,9 +123,11 @@ def start_transaction(transaction: nil, custom_sampling_context: {}, instrumente
       return unless instrumenter == configuration.instrumenter
 
       if transaction.nil? && !options.key?(:trace_id) && established
-        # reuse the already-established trace_id instead of generating an unrelated one
+        # adopt the already-established trace and span rather than generating unrelated
+        # ones, so anything already logged against them resolves to this transaction
         propagation_context = current_scope.propagation_context
         options[:trace_id] = propagation_context.trace_id
+        options[:span_id] ||= propagation_context.span_id
         options[:sample_rand] ||= propagation_context.sample_rand
       end
 
@@ -391,6 +393,7 @@ def continue_trace(env, established: false, **options)
 
       Transaction.new(
         trace_id: propagation_context.trace_id,
+        span_id: propagation_context.span_id,
         parent_span_id: propagation_context.parent_span_id,
         parent_sampled: propagation_context.parent_sampled,
         baggage: propagation_context.baggage,

sentry-ruby/spec/sentry_spec.rb

@@ -597,6 +597,24 @@
           expect(transaction.sample_rand).to eq(propagation_context.sample_rand)
         end
 
+        it "adopts the scope's propagation context span_id" do
+          propagation_context = Sentry.get_current_scope.propagation_context
+
+          transaction = described_class.start_transaction(
+            name: "test", op: "test.op", established: true
+          )
+
+          expect(transaction.span_id).to eq(propagation_context.span_id)
+        end
+
+        it "does not override an explicitly provided span_id" do
+          transaction = described_class.start_transaction(
+            name: "test", op: "test.op", span_id: "b" * 16, established: true
+          )
+
+          expect(transaction.span_id).to eq("b" * 16)
+        end
+
         it "does not override an explicitly provided trace_id" do
           transaction = described_class.start_transaction(
             name: "test", op: "test.op", trace_id: "a" * 32, established: true
@@ -1262,6 +1280,14 @@
         expect(transaction.baggage.mutable).to eq(false)
       end
 
+      it "gives the Transaction the propagation context's span_id" do
+        Sentry.configuration.traces_sample_rate = 1.0
+
+        transaction = described_class.continue_trace(env, name: "foobar")
+
+        expect(transaction.span_id).to eq(Sentry.get_current_scope.propagation_context.span_id)
+      end
+
       describe "sample_rand propagation" do
         before do
           Sentry.configuration.traces_sample_rate = 1.0

spec/features/trace_context_spec.rb

@@ -18,6 +18,13 @@ def request_transaction
       .to eq(request_transaction.dig("contexts", "trace", "trace_id"))
   end
 
+  it "points a log emitted before CaptureExceptions at the transaction's span" do
+    without_trace_propagation { make_request("/trace_context") }
+
+    expect(early_middleware_logs.first["span_id"])
+      .to eq(request_transaction.dig("contexts", "trace", "span_id"))
+  end
+
   it "continues an incoming distributed trace in a log emitted before CaptureExceptions" do
     incoming_trace_id = propagated_trace_id