Skip to content

Commit 16aa912

Browse files
rsamoilovclaude
rsamoilov
and
claude
committed
fix(rack): fiber-safe context detachment in EventHandler
Store fiber reference with context token to safely skip detachment when on_finish is called from a different fiber than on_start. Spans are always finished regardless of fiber. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 193d0af commit 16aa912

File tree

6 files changed

+138
-15
lines changed

6 files changed

+138
-15
lines changed

‎instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/dup/event_handler.rb‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Dup
4343
class EventHandler
4444
include ::Rack::Events::Abstract
4545

46-
OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span'
46+
OTEL_CONTEXT_INFO = 'otel.context_info'
4747
EMPTY_HASH = {}.freeze
4848

4949
# Creates a server span for this current request using the incoming parent context
@@ -62,7 +62,8 @@ def on_start(request, _)
6262
span = create_span(parent_context, request)
6363
span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context)
6464
rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx)
65-
request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span]
65+
token = OpenTelemetry::Context.attach(rack_ctx)
66+
request.env[OTEL_CONTEXT_INFO] = [Fiber.current, token, span]
6667
rescue StandardError => e
6768
OpenTelemetry.handle_error(exception: e)
6869
end
@@ -209,11 +210,17 @@ def request_span_attributes(env)
209210
end
210211

211212
def detach_context(request)
212-
return nil unless request.env[OTEL_TOKEN_AND_SPAN]
213+
otel_context_info = request.env[OTEL_CONTEXT_INFO]
214+
return unless otel_context_info
213215

214-
token, span = request.env[OTEL_TOKEN_AND_SPAN]
216+
original_fiber, token, span = otel_context_info
215217
span.finish
216-
OpenTelemetry::Context.detach(token)
218+
219+
if Fiber.current.equal?(original_fiber)
220+
OpenTelemetry::Context.detach(token)
221+
else
222+
OpenTelemetry.logger.debug { '[rack] Skipping context detach: response processed in different fiber than request.' }
223+
end
217224
rescue StandardError => e
218225
OpenTelemetry.handle_error(exception: e)
219226
end

‎instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/old/event_handler.rb‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Old
4343
class EventHandler
4444
include ::Rack::Events::Abstract
4545

46-
OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span'
46+
OTEL_CONTEXT_INFO = 'otel.context_info'
4747
EMPTY_HASH = {}.freeze
4848

4949
# Creates a server span for this current request using the incoming parent context
@@ -62,7 +62,8 @@ def on_start(request, _)
6262
span = create_span(parent_context, request)
6363
span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context)
6464
rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx)
65-
request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span]
65+
token = OpenTelemetry::Context.attach(rack_ctx)
66+
request.env[OTEL_CONTEXT_INFO] = [Fiber.current, token, span]
6667
rescue StandardError => e
6768
OpenTelemetry.handle_error(exception: e)
6869
end
@@ -198,11 +199,17 @@ def request_span_attributes(env)
198199
end
199200

200201
def detach_context(request)
201-
return nil unless request.env[OTEL_TOKEN_AND_SPAN]
202+
otel_context_info = request.env[OTEL_CONTEXT_INFO]
203+
return unless otel_context_info
202204

203-
token, span = request.env[OTEL_TOKEN_AND_SPAN]
205+
original_fiber, token, span = otel_context_info
204206
span.finish
205-
OpenTelemetry::Context.detach(token)
207+
208+
if Fiber.current.equal?(original_fiber)
209+
OpenTelemetry::Context.detach(token)
210+
else
211+
OpenTelemetry.logger.debug { '[rack] Skipping context detach: response processed in different fiber than request.' }
212+
end
206213
rescue StandardError => e
207214
OpenTelemetry.handle_error(exception: e)
208215
end

‎instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Stable
4343
class EventHandler
4444
include ::Rack::Events::Abstract
4545

46-
OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span'
46+
OTEL_CONTEXT_INFO = 'otel.context_info'
4747
EMPTY_HASH = {}.freeze
4848

4949
# Creates a server span for this current request using the incoming parent context
@@ -62,7 +62,8 @@ def on_start(request, _)
6262
span = create_span(parent_context, request)
6363
span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context)
6464
rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx)
65-
request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span]
65+
token = OpenTelemetry::Context.attach(rack_ctx)
66+
request.env[OTEL_CONTEXT_INFO] = [Fiber.current, token, span]
6667
rescue StandardError => e
6768
OpenTelemetry.handle_error(exception: e)
6869
end
@@ -197,11 +198,17 @@ def request_span_attributes(env)
197198
end
198199

199200
def detach_context(request)
200-
return nil unless request.env[OTEL_TOKEN_AND_SPAN]
201+
otel_context_info = request.env[OTEL_CONTEXT_INFO]
202+
return unless otel_context_info
201203

202-
token, span = request.env[OTEL_TOKEN_AND_SPAN]
204+
original_fiber, token, span = otel_context_info
203205
span.finish
204-
OpenTelemetry::Context.detach(token)
206+
207+
if Fiber.current.equal?(original_fiber)
208+
OpenTelemetry::Context.detach(token)
209+
else
210+
OpenTelemetry.logger.debug { '[rack] Skipping context detach: response processed in different fiber than request.' }
211+
end
205212
rescue StandardError => e
206213
OpenTelemetry.handle_error(exception: e)
207214
end

‎instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/dup/event_handler_test.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,4 +508,38 @@ class EventHandlerError < StandardError
508508
_(proxy_event).must_be_nil
509509
end
510510
end
511+
512+
describe 'cross-fiber context detachment' do
513+
it 'finishes span without error when on_finish is called from different fiber' do
514+
request = Rack::MockRequest.env_for('/')
515+
rack_request = Rack::Request.new(request)
516+
response = Rack::Response.new
517+
518+
fiber1 = Fiber.new do
519+
handler.on_start(rack_request, nil)
520+
Fiber.yield
521+
end
522+
fiber1.resume
523+
524+
fiber2 = Fiber.new do
525+
handler.on_finish(rack_request, response)
526+
end
527+
fiber2.resume
528+
529+
_(finished_spans.size).must_equal 1
530+
_(rack_span.name).must_equal 'GET'
531+
end
532+
533+
it 'detaches context normally when same fiber' do
534+
request = Rack::MockRequest.env_for('/')
535+
rack_request = Rack::Request.new(request)
536+
response = Rack::Response.new
537+
538+
handler.on_start(rack_request, nil)
539+
handler.on_finish(rack_request, response)
540+
541+
_(finished_spans.size).must_equal 1
542+
_(rack_span.name).must_equal 'GET'
543+
end
544+
end
511545
end

‎instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/old/event_handler_test.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,4 +483,38 @@ class EventHandlerError < StandardError
483483
_(proxy_event).must_be_nil
484484
end
485485
end
486+
487+
describe 'cross-fiber context detachment' do
488+
it 'finishes span without error when on_finish is called from different fiber' do
489+
request = Rack::MockRequest.env_for('/')
490+
rack_request = Rack::Request.new(request)
491+
response = Rack::Response.new
492+
493+
fiber1 = Fiber.new do
494+
handler.on_start(rack_request, nil)
495+
Fiber.yield
496+
end
497+
fiber1.resume
498+
499+
fiber2 = Fiber.new do
500+
handler.on_finish(rack_request, response)
501+
end
502+
fiber2.resume
503+
504+
_(finished_spans.size).must_equal 1
505+
_(rack_span.name).must_equal 'HTTP GET'
506+
end
507+
508+
it 'detaches context normally when same fiber' do
509+
request = Rack::MockRequest.env_for('/')
510+
rack_request = Rack::Request.new(request)
511+
response = Rack::Response.new
512+
513+
handler.on_start(rack_request, nil)
514+
handler.on_finish(rack_request, response)
515+
516+
_(finished_spans.size).must_equal 1
517+
_(rack_span.name).must_equal 'HTTP GET'
518+
end
519+
end
486520
end

‎instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/stable/event_handler_test.rb‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,4 +479,38 @@ class EventHandlerError < StandardError
479479
_(proxy_event).must_be_nil
480480
end
481481
end
482+
483+
describe 'cross-fiber context detachment' do
484+
it 'finishes span without error when on_finish is called from different fiber' do
485+
request = Rack::MockRequest.env_for('/')
486+
rack_request = Rack::Request.new(request)
487+
response = Rack::Response.new
488+
489+
fiber1 = Fiber.new do
490+
handler.on_start(rack_request, nil)
491+
Fiber.yield
492+
end
493+
fiber1.resume
494+
495+
fiber2 = Fiber.new do
496+
handler.on_finish(rack_request, response)
497+
end
498+
fiber2.resume
499+
500+
_(finished_spans.size).must_equal 1
501+
_(rack_span.name).must_equal 'GET'
502+
end
503+
504+
it 'detaches context normally when same fiber' do
505+
request = Rack::MockRequest.env_for('/')
506+
rack_request = Rack::Request.new(request)
507+
response = Rack::Response.new
508+
509+
handler.on_start(rack_request, nil)
510+
handler.on_finish(rack_request, response)
511+
512+
_(finished_spans.size).must_equal 1
513+
_(rack_span.name).must_equal 'GET'
514+
end
515+
end
482516
end

0 commit comments

Comments
 (0)