Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ class TextMapPropagator
FIELDS = [IDENTITY_KEY].freeze
TRACE_SPAN_IDENTITY_REGEX = /\A(?<trace_id>(?:[0-9a-f]){1,32}):(?<span_id>(?:[0-9a-f]){1,16}):(?:[0-9a-f]){1,16}:(?<sampling_flags>[0-9a-f]{1,2})\z/
ZERO_ID_REGEX = /^0+$/
BAGGAGE_KEY_PREFIX = 'uberctx-'
MAX_BAGGAGE_ENTRIES = 180
MAX_BAGGAGE_ENTRY_BYTES = 4096
MAX_BAGGAGE_TOTAL_BYTES = 8192

private_constant \
:IDENTITY_KEY, :DEFAULT_FLAG_BIT, :SAMPLED_FLAG_BIT, :DEBUG_FLAG_BIT,
:FIELDS, :TRACE_SPAN_IDENTITY_REGEX, :ZERO_ID_REGEX
:FIELDS, :TRACE_SPAN_IDENTITY_REGEX, :ZERO_ID_REGEX, :BAGGAGE_KEY_PREFIX,
:MAX_BAGGAGE_ENTRIES, :MAX_BAGGAGE_ENTRY_BYTES, :MAX_BAGGAGE_TOTAL_BYTES

# Extract trace context from the supplied carrier.
# If extraction fails, the original context will be returned
Expand Down Expand Up @@ -103,15 +108,24 @@ def build_span(match, sampling_flags)
end

def context_with_extracted_baggage(carrier, context, getter)
baggage_key_prefix = 'uberctx-'
OpenTelemetry::Baggage.build(context: context) do |b|
count = 0
total_bytes = 0
getter.keys(carrier).each do |carrier_key|
baggage_key = carrier_key.start_with?(baggage_key_prefix) && carrier_key[baggage_key_prefix.length..]
break unless count < MAX_BAGGAGE_ENTRIES

baggage_key = carrier_key.start_with?(BAGGAGE_KEY_PREFIX) && carrier_key[BAGGAGE_KEY_PREFIX.length..]
next unless baggage_key

raw_value = getter.get(carrier, carrier_key)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add next unless raw_value to ignore the case when raw_value=nil?
This can also prevent the undefined method 'bytesize' issue when raw_value is nil

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you're right. Fixed and covered it with a test.

Rubocop then hit 8/7 on Metrics/CyclomaticComplexity and 101/100 on Metrics/ClassLength, so the limit check moved into within_baggage_limits? and the private_constant list is reflowed. bundle exec rake is green.

value = URI.decode_uri_component(raw_value)
b.set_value(baggage_key, value)
# Limits are byte-denominated, not character-denominated.
entry_bytes = baggage_key.bytesize + raw_value.bytesize
next unless entry_bytes <= MAX_BAGGAGE_ENTRY_BYTES &&
total_bytes + entry_bytes <= MAX_BAGGAGE_TOTAL_BYTES

b.set_value(baggage_key, URI.decode_uri_component(raw_value))
count += 1
total_bytes += entry_bytes
end
end
end
Expand Down
43 changes: 43 additions & 0 deletions propagator/jaeger/test/text_map_propagator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,49 @@ def extracted_context_must_equal_parent_context(header)
_(OpenTelemetry::Baggage.value('key-2', context: context)).must_equal('value2')
end

it 'enforces the max of 180 baggage entries, keeping the first ones' do
carrier = { 'uber-trace-id' => '80f198ee56343ba864fe8b2a57d3eff7:e457b5a2e4d86bd1:0:1' }
200.times { |i| carrier["uberctx-k#{i}"] = "v#{i}" }
context = propagator.extract(carrier, context: OpenTelemetry::Context.empty)
_(OpenTelemetry::Baggage.values(context: context).size).must_equal(180)
_(OpenTelemetry::Baggage.value('k0', context: context)).must_equal('v0')
_(OpenTelemetry::Baggage.value('k180', context: context)).must_be_nil
end

it 'drops a baggage entry over 4096 bytes, keeping the rest' do
carrier = {
'uber-trace-id' => '80f198ee56343ba864fe8b2a57d3eff7:e457b5a2e4d86bd1:0:1',
'uberctx-ok' => 'value',
'uberctx-big' => 'x' * 5000
}
context = propagator.extract(carrier, context: OpenTelemetry::Context.empty)
_(OpenTelemetry::Baggage.value('ok', context: context)).must_equal('value')
_(OpenTelemetry::Baggage.value('big', context: context)).must_be_nil
end

it 'measures the per-entry limit in bytes, not characters' do
carrier = {
'uber-trace-id' => '80f198ee56343ba864fe8b2a57d3eff7:e457b5a2e4d86bd1:0:1',
'uberctx-ok' => 'value',
# 2100 multibyte chars = 4200 bytes: under 4096 chars, over 4096 bytes
'uberctx-u' => 'é' * 2100
}
context = propagator.extract(carrier, context: OpenTelemetry::Context.empty)
_(OpenTelemetry::Baggage.value('ok', context: context)).must_equal('value')
_(OpenTelemetry::Baggage.value('u', context: context)).must_be_nil
end

it 'enforces the max total of 8192 bytes, keeping the earlier entries' do
carrier = { 'uber-trace-id' => '80f198ee56343ba864fe8b2a57d3eff7:e457b5a2e4d86bd1:0:1' }
100.times { |i| carrier["uberctx-k#{i}"] = 'y' * 200 } # ~100 * ~205 bytes = ~20k
context = propagator.extract(carrier, context: OpenTelemetry::Context.empty)
size = OpenTelemetry::Baggage.values(context: context).size
_(size).must_be(:positive?)
_(size).must_be(:<, 100)
_(OpenTelemetry::Baggage.value('k0', context: context)).wont_be_nil
_(OpenTelemetry::Baggage.value('k99', context: context)).must_be_nil
end

it 'handles trace ids and span ids that are too long' do
extracted_context_must_equal_parent_context(
'80f198ee56343ba864fe8b2a57d3eff7eff7:e457b5a2e4d86bd1:0:1'
Expand Down