Skip to content

Commit 314f3b8

Browse files
authored
fix: Return the original context if the baggage header value is empty (#877)
We don't test for a zero-length string, because I can really only think of two ways to do it: regular expressions (can be slow) and `.strip.empty?` (which `dup`s the string and therefore allocates, I believe). And slowing down or allocating defeats the purpose of what this change is trying to achieve: *not* slowing down or allocating in the relatively common case of "there's no baggage to extract." Note: https://github.com/rails/rails/blob/83217025a171593547d1268651b446d3533e2019/activesupport/lib/active_support/core_ext/object/blank.rb#L122-L124 Fixes #811
1 parent b858031 commit 314f3b8

File tree

2 files changed

+26
-2
lines changed

2 files changed

+26
-2
lines changed

api/lib/opentelemetry/baggage/propagation/text_map_propagator.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ def inject(carrier, context: Context.current, setter: Context::Propagation.text_
3939
end
4040

4141
# Extract remote baggage from the supplied carrier.
42-
# If extraction fails, the original context will be returned
42+
# If extraction fails or there is no baggage to extract,
43+
# then the original context will be returned
4344
#
4445
# @param [Carrier] carrier The carrier to get the header from
4546
# @param [optional Context] context Context to be updated with the baggage
@@ -52,7 +53,7 @@ def inject(carrier, context: Context.current, setter: Context::Propagation.text_
5253
# if extraction fails
5354
def extract(carrier, context: Context.current, getter: Context::Propagation.text_map_getter)
5455
header = getter.get(carrier, BAGGAGE_KEY)
55-
return context unless header
56+
return context if header.nil? || header.empty?
5657

5758
entries = header.gsub(/\s/, '').split(',')
5859

api/test/opentelemetry/baggage/propagation/text_map_propagator_test.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,29 @@
4747
assert_value(context, 'key:2', 'val2,2')
4848
end
4949
end
50+
51+
describe 'invalid or no-op headers' do
52+
it 'returns the same context object when the headers are not present' do
53+
carrier = {}
54+
empty_context = Context.empty
55+
context = propagator.extract(carrier, context: empty_context)
56+
_(context.object_id).must_equal(empty_context.object_id)
57+
end
58+
59+
it 'returns the same context object when the baggage value is a 0-length string' do
60+
carrier = { header_key => '' }
61+
empty_context = Context.empty
62+
context = propagator.extract(carrier, context: empty_context)
63+
_(context.object_id).must_equal(empty_context.object_id)
64+
end
65+
66+
it 'does not test for an "empty" string and still replaces the context' do
67+
carrier = { header_key => ' ' }
68+
empty_context = Context.empty
69+
context = propagator.extract(carrier, context: empty_context)
70+
_(context.object_id).wont_equal(empty_context.object_id)
71+
end
72+
end
5073
end
5174

5275
describe '#inject' do

0 commit comments

Comments
 (0)