diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb index 338bd95dd..8bb502a4a 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb @@ -42,7 +42,7 @@ module Middlewares class EventHandler include ::Rack::Events::Abstract - TOKENS_KEY = 'otel.context.tokens' + OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span' GOOD_HTTP_STATUSES = (100..499) # Creates a server span for this current request using the incoming parent context @@ -56,7 +56,9 @@ def on_start(request, _) parent_context = extract_remote_context(request) span = create_span(parent_context, request) - request.env[TOKENS_KEY] = register_current_span(span) + span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context) + rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx) + request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span] rescue StandardError => e OpenTelemetry.handle_error(exception: e) end @@ -108,7 +110,7 @@ def on_finish(request, response) rescue StandardError => e OpenTelemetry.handle_error(exception: e) ensure - detach_contexts(request) + detach_context(request) end private @@ -191,11 +193,12 @@ def request_span_attributes(env) attributes end - def detach_contexts(request) - request.env[TOKENS_KEY]&.reverse_each do |token| - OpenTelemetry::Context.detach(token) - OpenTelemetry::Trace.current_span.finish - end + def detach_context(request) + return nil unless request.env[OTEL_TOKEN_AND_SPAN] + + token, span = request.env[OTEL_TOKEN_AND_SPAN] + span.finish + OpenTelemetry::Context.detach(token) rescue StandardError => e OpenTelemetry.handle_error(exception: e) end @@ -244,15 +247,6 @@ def config OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.config end - def register_current_span(span) - ctx = OpenTelemetry::Trace.context_with_span(span) - rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: ctx) - - contexts = [ctx, rack_ctx] - contexts.compact! - contexts.map { |context| OpenTelemetry::Context.attach(context) } - end - def create_span(parent_context, request) span = tracer.start_span( create_request_span_name(request), diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_test.rb index d08857637..a70b486c0 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_test.rb @@ -67,6 +67,27 @@ instrumentation.install(config) end + # Simulating buggy instrumentation that starts a span, sets the ctx + # but fails to detach or close the span + describe 'broken instrumentation' do + let(:service) do + lambda do |_env| + span = OpenTelemetry.tracer_provider.tracer('buggy').start_span('I never close') + OpenTelemetry::Context.attach(OpenTelemetry::Trace.context_with_span(span)) + [200, { 'Content-Type' => 'text/plain' }, response_body] + end + end + + it 'still closes the rack span' do + assert_raises OpenTelemetry::Context::DetachError do + get uri, {}, headers + end + _(finished_spans.size).must_equal 1 + _(rack_span.name).must_equal 'HTTP GET' + OpenTelemetry::Context.clear + end + end + describe '#call' do before do get uri, {}, headers @@ -84,6 +105,25 @@ _(proxy_event).must_be_nil end + describe 'when baggage is set' do + let(:headers) do + Hash( + 'baggage' => 'foo=123' + ) + end + + let(:service) do + lambda do |_env| + _(OpenTelemetry::Baggage.raw_entries['foo'].value).must_equal('123') + [200, { 'Content-Type' => 'text/plain' }, response_body] + end + end + + it 'sets baggage in the request context' do + _(rack_span.name).must_equal 'HTTP GET' + end + end + describe 'when a query is passed in' do let(:uri) { '/endpoint?query=true' }