fix: store the propagation context in env instead of a boolean flag

The established flag lived in the request-scoped env but certified a
thread-local hub, so any middleware doing `@app.call` on another thread
lost the incoming trace. The env now carries the context itself, trusted
only while it is still the current scope's.

Author: solnic

sentry-rails/lib/sentry/rails/capture_context.rb

@@ -14,7 +14,7 @@ def call(env)
 
         Sentry.clone_hub_to_current_thread
         Sentry.get_current_scope.generate_propagation_context(env)
-        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = true
+        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = Sentry.get_current_scope.propagation_context
 
         @app.call(env)
       end

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

@@ -20,6 +20,20 @@ def call(env)
     end
   end
 
+  # The shape used by hard-timeout and bulkhead middleware: the downstream stack
+  # runs on a different thread than the one that entered. Permitting concurrent
+  # loads around the join is what ActionController::Live does for the same reason.
+  class ThreadHandoffMiddleware
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      thread = Thread.new { @app.call(env) }
+      ActiveSupport::Dependencies.interlock.permit_concurrent_loads { thread.value }
+    end
+  end
+
   describe "#call" do
     before do
       make_basic_app
@@ -36,7 +50,8 @@ def call(env)
       env = Rack::MockRequest.env_for("/test")
       described_class.new(app).call(env)
 
-      expect(env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY]).to eq(true)
+      expect(env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY])
+        .to be(Sentry.get_current_scope.propagation_context)
       expect(trace_id_in_app).to be_a(String)
     end
 
@@ -57,6 +72,29 @@ def call(env)
     end
   end
 
+  context "when a middleware hands the request to another thread", type: :request do
+    let(:transport) { Sentry.get_current_client.transport }
+
+    let(:incoming_transaction) do
+      Sentry::Transaction.new(op: "pageload", status: "ok", sampled: true, name: "a/path")
+    end
+
+    before do
+      make_basic_app do |config, app|
+        config.traces_sample_rate = 1.0
+        app.config.middleware.insert_before(Sentry::Rails::CaptureExceptions, ThreadHandoffMiddleware)
+      end
+    end
+
+    it "continues the incoming trace" do
+      get "/world", headers: { "sentry-trace" => incoming_transaction.to_sentry_trace }
+
+      trace = transport.events.last.contexts[:trace]
+      expect(trace[:trace_id]).to eq(incoming_transaction.trace_id)
+      expect(trace[:parent_span_id]).to eq(incoming_transaction.span_id)
+    end
+  end
+
   context "when composed with CaptureExceptions", type: :request do
     before do
       CaptureContextSpecProbe.captured_trace_ids.clear

sentry-ruby/lib/sentry/propagation_context.rb

@@ -13,9 +13,10 @@ class PropagationContext
       "-?([01])?\\z"         # sampled
     )
 
-    # Rack env key signaling that trace context was already established earlier in
-    # the middleware stack (e.g. by +Sentry::Rails::CaptureContext+); consumed once by
-    # +Sentry::Rack::CaptureExceptions+.
+    # Rack env key carrying the PropagationContext established earlier in the middleware
+    # stack (e.g. by +Sentry::Rails::CaptureContext+); consumed once by
+    # +Sentry::Rack::CaptureExceptions+, which only trusts it when it still belongs to the
+    # current execution context.
     ESTABLISHED_ENV_KEY = "sentry.trace_context_established"
 
     # An uuid that can be used to identify a trace.

sentry-ruby/lib/sentry/rack/capture_exceptions.rb

@@ -16,8 +16,11 @@ def initialize(app)
       def call(env)
         return @app.call(env) unless Sentry.initialized?
 
-        # consumed atomically, first, so it can't leak into later reuses of this env
-        established = env.delete(Sentry::PropagationContext::ESTABLISHED_ENV_KEY)
+        # consumed atomically, first, so it can't leak into later reuses of this env.
+        # the env is request-scoped but the hub it certifies is not, so the context only
+        # counts as established while it still belongs to the execution context we are on
+        context = env.delete(Sentry::PropagationContext::ESTABLISHED_ENV_KEY)
+        established = !context.nil? && context.equal?(Sentry.get_current_scope&.propagation_context)
 
         # make sure the current thread has a clean hub, unless it was already established
         Sentry.clone_hub_to_current_thread unless established

sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb

@@ -97,7 +97,7 @@
       it "does not re-clone the hub and reuses the existing propagation context" do
         Sentry.clone_hub_to_current_thread
         Sentry.get_current_scope.generate_propagation_context(env)
-        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = true
+        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = Sentry.get_current_scope.propagation_context
 
         established_propagation_context = Sentry.get_current_scope.propagation_context
 
@@ -118,7 +118,7 @@
       it "deletes the established flag from env so it doesn't leak into later reuses of the same env" do
         Sentry.clone_hub_to_current_thread
         Sentry.get_current_scope.generate_propagation_context(env)
-        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = true
+        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = Sentry.get_current_scope.propagation_context
 
         app = ->(_e) { [200, {}, ['okay']] }
         stack = Sentry::Rack::CaptureExceptions.new(app)
@@ -127,13 +127,33 @@
         expect(env.key?(Sentry::PropagationContext::ESTABLISHED_ENV_KEY)).to eq(false)
       end
 
+      it "honors the incoming trace when the established context belongs to another execution context" do
+        external_transaction = Sentry::Transaction.new(op: "pageload", status: "ok", sampled: true, name: "a/path")
+        env["HTTP_SENTRY_TRACE"] = external_transaction.to_sentry_trace
+
+        Sentry.clone_hub_to_current_thread
+        Sentry.get_current_scope.generate_propagation_context(env)
+        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = Sentry.get_current_scope.propagation_context
+
+        trace_id_in_app = nil
+        app = lambda do |_e|
+          trace_id_in_app = Sentry.get_current_scope.get_trace_context[:trace_id]
+          [200, {}, ['okay']]
+        end
+
+        stack = Sentry::Rack::CaptureExceptions.new(app)
+        Thread.new { stack.call(env) }.join
+
+        expect(trace_id_in_app).to eq(external_transaction.trace_id)
+      end
+
       it "does not reuse a stale established context on a later, unrelated call with the same env" do
         # Simulates a long-lived connection (e.g. Action Cable) that stores the handshake's
         # env and reuses it for many separate operations over its lifetime - only the very
         # first operation immediately following CaptureContext should honor the flag.
         Sentry.clone_hub_to_current_thread
         Sentry.get_current_scope.generate_propagation_context(env)
-        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = true
+        env[Sentry::PropagationContext::ESTABLISHED_ENV_KEY] = Sentry.get_current_scope.propagation_context
 
         app = ->(_e) { [200, {}, ['okay']] }
         stack = Sentry::Rack::CaptureExceptions.new(app)