Skip to content

Commit b4b77d0

Browse files
committed
fix: Fixing missing OpenTelemetry::Context detach on Excon instrumentation
When a span is not recorded, for example when you are sampling traces, the OpenTelemetry::Context attached on `request_call` is never detached. This happens because `handle_response` is doing nothing if `span.recording?` is false. Here is example of the error that the missing detach generated: ``` ERROR -- : OpenTelemetry error: calls to detach should match corresponding calls to attach. ```
1 parent 29faa2b commit b4b77d0

File tree

6 files changed

+63
-3
lines changed

6 files changed

+63
-3
lines changed

instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/dup/tracer_middleware.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ def self.around_default_stack
8484

8585
def handle_response(datum)
8686
datum.delete(:otel_span)&.tap do |span|
87+
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
8788
return unless span.recording?
8889

8990
if datum.key?(:response)
@@ -99,7 +100,6 @@ def handle_response(datum)
99100
end
100101

101102
span.finish
102-
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
103103
end
104104
rescue StandardError => e
105105
OpenTelemetry.handle_error(e)

instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/old/tracer_middleware.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ def self.around_default_stack
7676

7777
def handle_response(datum)
7878
datum.delete(:otel_span)&.tap do |span|
79+
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
7980
return unless span.recording?
8081

8182
if datum.key?(:response)
@@ -90,7 +91,6 @@ def handle_response(datum)
9091
end
9192

9293
span.finish
93-
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
9494
end
9595
rescue StandardError => e
9696
OpenTelemetry.handle_error(e)

instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/stable/tracer_middleware.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ def self.around_default_stack
7676

7777
def handle_response(datum)
7878
datum.delete(:otel_span)&.tap do |span|
79+
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
7980
return unless span.recording?
8081

8182
if datum.key?(:response)
@@ -90,7 +91,6 @@ def handle_response(datum)
9091
end
9192

9293
span.finish
93-
OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
9494
end
9595
rescue StandardError => e
9696
OpenTelemetry.handle_error(e)

instrumentation/excon/test/opentelemetry/instrumentation/excon/dup/instrumentation_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,26 @@
199199

200200
_(span.attributes['peer.service']).must_equal 'example:custom'
201201
end
202+
203+
describe 'context detachment with non-recording spans' do
204+
before do
205+
@original_sampler = OpenTelemetry.tracer_provider.sampler
206+
OpenTelemetry.tracer_provider.sampler = OpenTelemetry::SDK::Trace::Samplers::ALWAYS_OFF
207+
end
208+
209+
after do
210+
OpenTelemetry.tracer_provider.sampler = @original_sampler
211+
end
212+
213+
it 'detaches context when span is not recorded' do
214+
initial_context = OpenTelemetry::Context.current
215+
Excon.get('http://example.com/success')
216+
final_context = OpenTelemetry::Context.current
217+
218+
_(final_context).must_equal initial_context
219+
_(exporter.finished_spans).must_be_empty
220+
end
221+
end
202222
end
203223

204224
describe 'untraced?' do

instrumentation/excon/test/opentelemetry/instrumentation/excon/old/instrumentation_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,26 @@
158158

159159
_(span.attributes['peer.service']).must_equal 'example:custom'
160160
end
161+
162+
describe 'context detachment with non-recording spans' do
163+
before do
164+
@original_sampler = OpenTelemetry.tracer_provider.sampler
165+
OpenTelemetry.tracer_provider.sampler = OpenTelemetry::SDK::Trace::Samplers::ALWAYS_OFF
166+
end
167+
168+
after do
169+
OpenTelemetry.tracer_provider.sampler = @original_sampler
170+
end
171+
172+
it 'detaches context when span is not recorded' do
173+
initial_context = OpenTelemetry::Context.current
174+
Excon.get('http://example.com/success')
175+
final_context = OpenTelemetry::Context.current
176+
177+
_(final_context).must_equal initial_context
178+
_(exporter.finished_spans).must_be_empty
179+
end
180+
end
161181
end
162182

163183
describe 'untraced?' do

instrumentation/excon/test/opentelemetry/instrumentation/excon/stable/instrumentation_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,26 @@
172172

173173
_(span.attributes['peer.service']).must_equal 'example:custom'
174174
end
175+
176+
describe 'context detachment with non-recording spans' do
177+
before do
178+
@original_sampler = OpenTelemetry.tracer_provider.sampler
179+
OpenTelemetry.tracer_provider.sampler = OpenTelemetry::SDK::Trace::Samplers::ALWAYS_OFF
180+
end
181+
182+
after do
183+
OpenTelemetry.tracer_provider.sampler = @original_sampler
184+
end
185+
186+
it 'detaches context when span is not recorded' do
187+
initial_context = OpenTelemetry::Context.current
188+
Excon.get('http://example.com/success')
189+
final_context = OpenTelemetry::Context.current
190+
191+
_(final_context).must_equal initial_context
192+
_(exporter.finished_spans).must_be_empty
193+
end
194+
end
175195
end
176196

177197
describe 'untraced?' do

0 commit comments

Comments
 (0)