-
Notifications
You must be signed in to change notification settings - Fork 255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix!: refactor propagators to add #fields #638
Changes from 16 commits
d384200
9c38b49
7c12d68
e8b7669
8a67ceb
0eb349c
f786820
1271c86
0b95531
00ae7c7
afe557c
f0b6d4a
23332c6
3b8ea38
13c5787
2dde087
d417fa4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,109 @@ | ||||||
# frozen_string_literal: true | ||||||
|
||||||
# Copyright The OpenTelemetry Authors | ||||||
# | ||||||
# SPDX-License-Identifier: Apache-2.0 | ||||||
|
||||||
require 'cgi' | ||||||
|
||||||
module OpenTelemetry | ||||||
module Baggage | ||||||
module Propagation | ||||||
# Propagates baggage using the W3C Baggage format | ||||||
class TextMapPropagator | ||||||
# Maximums according to W3C Baggage spec | ||||||
MAX_ENTRIES = 180 | ||||||
MAX_ENTRY_LENGTH = 4096 | ||||||
MAX_TOTAL_LENGTH = 8192 | ||||||
|
||||||
BAGGAGE_KEY = 'baggage' | ||||||
FIELDS = [BAGGAGE_KEY].freeze | ||||||
|
||||||
private_constant :BAGGAGE_KEY, :FIELDS, :MAX_ENTRIES, :MAX_ENTRY_LENGTH, :MAX_TOTAL_LENGTH | ||||||
|
||||||
# Inject in-process baggage into the supplied carrier. | ||||||
# | ||||||
# @param [Carrier] carrier The mutable carrier to inject baggage into | ||||||
# @param [Context] context The context to read baggage from | ||||||
# @param [optional Setter] setter If the optional setter is provided, it | ||||||
# will be used to write context into the carrier, otherwise the default | ||||||
# text map setter will be used. | ||||||
def inject(carrier, context: Context.current, setter: Context::Propagation.text_map_setter) | ||||||
baggage = OpenTelemetry.baggage.raw_entries(context: context) | ||||||
|
||||||
return if baggage.nil? || baggage.empty? | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The results of those expressions differ: irb(main):001:0> b = ''
=> ""
irb(main):002:0> b.nil? || b.empty?
=> true
irb(main):003:0> b&.empty?
=> true
irb(main):004:0> b = nil
=> nil
irb(main):005:0> b.nil? || b.empty?
=> true
irb(main):006:0> b&.empty?
=> nil There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
|
||||||
encoded_baggage = encode(baggage) | ||||||
setter.set(carrier, BAGGAGE_KEY, encoded_baggage) unless encoded_baggage&.empty? | ||||||
nil | ||||||
end | ||||||
|
||||||
# Extract remote baggage from the supplied carrier. | ||||||
# If extraction fails, the original context will be returned | ||||||
# | ||||||
# @param [Carrier] carrier The carrier to get the header from | ||||||
# @param [optional Context] context Context to be updated with the baggage | ||||||
# extracted from the carrier. Defaults to +Context.current+. | ||||||
# @param [optional Getter] getter If the optional getter is provided, it | ||||||
# will be used to read the header from the carrier, otherwise the default | ||||||
# text map getter will be used. | ||||||
# | ||||||
# @return [Context] context updated with extracted baggage, or the original context | ||||||
# if extraction fails | ||||||
def extract(carrier, context: Context.current, getter: Context::Propagation.text_map_getter) | ||||||
header = getter.get(carrier, BAGGAGE_KEY) | ||||||
|
||||||
entries = header.gsub(/\s/, '').split(',') | ||||||
|
||||||
OpenTelemetry.baggage.build(context: context) do |builder| | ||||||
entries.each do |entry| | ||||||
# Note metadata is currently unused in OpenTelemetry, but is part | ||||||
# the W3C spec where it's referred to as properties. We preserve | ||||||
# the properties (as-is) so that they can be propagated elsewhere. | ||||||
kv, meta = entry.split(';', 2) | ||||||
k, v = kv.split('=').map!(&CGI.method(:unescape)) | ||||||
builder.set_value(k, v, metadata: meta) | ||||||
end | ||||||
end | ||||||
rescue StandardError => e | ||||||
OpenTelemetry.logger.debug "Error extracting W3C baggage: #{e.message}" | ||||||
context | ||||||
end | ||||||
|
||||||
# Returns the predefined propagation fields. If your carrier is reused, you | ||||||
# should delete the fields returned by this method before calling +inject+. | ||||||
# | ||||||
# @return [Array<String>] a list of fields that will be used by this propagator. | ||||||
def fields | ||||||
FIELDS | ||||||
end | ||||||
|
||||||
private | ||||||
|
||||||
def encode(baggage) | ||||||
result = +'' | ||||||
encoded_count = 0 | ||||||
baggage.each_pair do |key, entry| | ||||||
break unless encoded_count < MAX_ENTRIES | ||||||
|
||||||
encoded_entry = encode_value(key, entry) | ||||||
next unless encoded_entry.size <= MAX_ENTRY_LENGTH && | ||||||
encoded_entry.size + result.size <= MAX_TOTAL_LENGTH | ||||||
|
||||||
result << encoded_entry << ',' | ||||||
encoded_count += 1 | ||||||
end | ||||||
result.chop! | ||||||
end | ||||||
|
||||||
def encode_value(key, entry) | ||||||
result = +"#{CGI.escape(key.to_s)}=#{CGI.escape(entry.value.to_s)}" | ||||||
# We preserve metadata recieved on extract and assume it's already formatted | ||||||
# for transport. It's sent as-is without further processing. | ||||||
result << ";#{entry.metadata}" if entry.metadata | ||||||
result | ||||||
end | ||||||
end | ||||||
end | ||||||
end | ||||||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you feel about using
keyword
args for all of the parameters toinject/extract
?I know for some it is idiomatic to have treated some arguments as
options
hash but I personally prefer keeping method arguments symmetric.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, we've opted to use keyword args for optional args or where a method has "many" args. In this case, the most common usage is
inject(carrier)
.inject(carrier: carrier)
doesn't increase clarity, but increases verbosity.