diff --git a/CHANGELOG.md b/CHANGELOG.md index f2fe37cd1..bab586171 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ ```rb config.enabled_patches += [:sidekiq_scheduler] ``` +- Passing a distributed trace ID in SENTRY_TRACE and baggage in SENTRY_BAGGAGE environment variables is now supported [#2179](https://github.com/getsentry/sentry-ruby/pull/2179) ### Bug Fixes diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index b65d2436c..db54adb90 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -52,7 +52,7 @@ def extract( baggage_header = getter.get(carrier, BAGGAGE_HEADER_NAME) baggage = if baggage_header && !baggage_header.empty? - Baggage.from_incoming_header(baggage_header) + Baggage.from_baggage_string(baggage_header) else # If there's an incoming sentry-trace but no incoming baggage header, # for instance in traces coming from older SDKs, diff --git a/sentry-ruby/lib/sentry/baggage.rb b/sentry-ruby/lib/sentry/baggage.rb index d766e999d..4eade98d4 100644 --- a/sentry-ruby/lib/sentry/baggage.rb +++ b/sentry-ruby/lib/sentry/baggage.rb @@ -26,11 +26,11 @@ def initialize(items, mutable: true) # # @param header [String] The incoming Baggage header string. # @return [Baggage, nil] - def self.from_incoming_header(header) + def self.from_baggage_string(baggage_string) items = {} mutable = true - header.split(',').each do |item| + baggage_string.split(',').each do |item| item = item.strip key, val = item.split('=') diff --git a/sentry-ruby/lib/sentry/propagation_context.rb b/sentry-ruby/lib/sentry/propagation_context.rb index 1fe5bc1ea..b907d3c9e 100644 --- a/sentry-ruby/lib/sentry/propagation_context.rb +++ b/sentry-ruby/lib/sentry/propagation_context.rb @@ -40,31 +40,37 @@ def initialize(scope, env = nil) @baggage = nil @incoming_trace = false - if env - sentry_trace_header = env["HTTP_SENTRY_TRACE"] || env[SENTRY_TRACE_HEADER_NAME] - baggage_header = env["HTTP_BAGGAGE"] || env[BAGGAGE_HEADER_NAME] - - if sentry_trace_header - sentry_trace_data = self.class.extract_sentry_trace(sentry_trace_header) - - if sentry_trace_data - @trace_id, @parent_span_id, @parent_sampled = sentry_trace_data - - @baggage = if baggage_header && !baggage_header.empty? - Baggage.from_incoming_header(baggage_header) - else - # If there's an incoming sentry-trace but no incoming baggage header, - # for instance in traces coming from older SDKs, - # baggage will be empty and frozen and won't be populated as head SDK. - Baggage.new({}) - end - - @baggage.freeze! - @incoming_trace = true - end + # Invoking code could pass a nil env, so let's ||= it so it's easier to work with. + env ||= {} + + # Trace string could be passed from the invoking code, + # or via the environment variable. + sentry_trace_string = env["HTTP_SENTRY_TRACE"] || env[SENTRY_TRACE_HEADER_NAME] || ENV["SENTRY_TRACE"] + + # Baggage string could be in the HTTP header (env), or it could be exposed in an ENV variable. + baggage_string = env["HTTP_BAGGAGE"] || env[BAGGAGE_HEADER_NAME] || ENV["SENTRY_BAGGAGE"] + + if sentry_trace_string + sentry_trace_data = self.class.extract_sentry_trace(sentry_trace_string) + + if sentry_trace_data + @trace_id, @parent_span_id, @parent_sampled = sentry_trace_data + + @baggage = if baggage_string && !baggage_string.empty? + Baggage.from_baggage_string(baggage_string) + else + # If there's an incoming sentry-trace but no incoming baggage header, + # for instance in traces coming from older SDKs, + # baggage will be empty and frozen and won't be populated as head SDK. + Baggage.new({}) + end + + @baggage.freeze! + @incoming_trace = true end end + # If the trace_id was not provided, generate a new one. @trace_id ||= SecureRandom.uuid.delete("-") @span_id = SecureRandom.uuid.delete("-").slice(0, 16) end diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index b154f71fb..f6b9ff2f6 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -111,7 +111,7 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h trace_id, parent_span_id, parent_sampled = sentry_trace_data baggage = if baggage && !baggage.empty? - Baggage.from_incoming_header(baggage) + Baggage.from_baggage_string(baggage) else # If there's an incoming sentry-trace but no incoming baggage header, # for instance in traces coming from older SDKs, diff --git a/sentry-ruby/spec/sentry/baggage_spec.rb b/sentry-ruby/spec/sentry/baggage_spec.rb index 3d72a5687..0866c7b25 100644 --- a/sentry-ruby/spec/sentry/baggage_spec.rb +++ b/sentry-ruby/spec/sentry/baggage_spec.rb @@ -17,21 +17,21 @@ describe "#dynamic_sampling_context" do context "when malformed baggage" do it "is empty" do - baggage = described_class.from_incoming_header(malformed_baggage) + baggage = described_class.from_baggage_string(malformed_baggage) expect(baggage.dynamic_sampling_context).to eq({}) end end context "when only third party baggage" do it "is empty" do - baggage = described_class.from_incoming_header(third_party_baggage) + baggage = described_class.from_baggage_string(third_party_baggage) expect(baggage.dynamic_sampling_context).to eq({}) end end context "when mixed baggage" do it "populates DSC" do - baggage = described_class.from_incoming_header(mixed_baggage) + baggage = described_class.from_baggage_string(mixed_baggage) expect(baggage.dynamic_sampling_context).to eq({ "sample_rate" => "0.01337", @@ -48,21 +48,21 @@ context "default args (without third party)" do context "when malformed baggage" do it "is empty string" do - baggage = described_class.from_incoming_header(malformed_baggage) + baggage = described_class.from_baggage_string(malformed_baggage) expect(baggage.serialize).to eq("") end end context "when only third party baggage" do it "is empty" do - baggage = described_class.from_incoming_header(third_party_baggage) + baggage = described_class.from_baggage_string(third_party_baggage) expect(baggage.serialize).to eq("") end end context "when mixed baggage" do it "populates DSC" do - baggage = described_class.from_incoming_header(mixed_baggage) + baggage = described_class.from_baggage_string(mixed_baggage) expect(baggage.serialize).to eq( "sentry-trace_id=771a43a4192642f0b136d5159a501700,"\ @@ -79,14 +79,14 @@ describe "#mutable" do context "when only third party baggage" do it "is mutable" do - baggage = described_class.from_incoming_header(third_party_baggage) + baggage = described_class.from_baggage_string(third_party_baggage) expect(baggage.mutable).to eq(true) end end context "when has sentry baggage" do it "is immutable" do - baggage = described_class.from_incoming_header(mixed_baggage) + baggage = described_class.from_baggage_string(mixed_baggage) expect(baggage.mutable).to eq(false) end end @@ -94,7 +94,7 @@ describe "#freeze!" do it "makes it immutable" do - baggage = described_class.from_incoming_header(third_party_baggage) + baggage = described_class.from_baggage_string(third_party_baggage) expect(baggage.mutable).to eq(true) baggage.freeze! expect(baggage.mutable).to eq(false) diff --git a/sentry-ruby/spec/sentry/client_spec.rb b/sentry-ruby/spec/sentry/client_spec.rb index 8ffc43a44..822417bde 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -129,7 +129,7 @@ def sentry_context end it "correct dynamic_sampling_context when incoming baggage header" do - baggage = Sentry::Baggage.from_incoming_header( + baggage = Sentry::Baggage.from_baggage_string( "other-vendor-value-1=foo;bar;baz, "\ "sentry-trace_id=771a43a4192642f0b136d5159a501700, "\ "sentry-public_key=49d0f7386ad645858ae85020e393bef3, "\ @@ -599,7 +599,7 @@ module ExcTag; end let(:string_io) { StringIO.new } let(:logger) { ::Logger.new(string_io) } let(:baggage) do - Sentry::Baggage.from_incoming_header( + Sentry::Baggage.from_baggage_string( "other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, "\ "sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, "\ "sentry-user_id=Am%C3%A9lie, other-vendor-value-2=foo;bar;" diff --git a/sentry-ruby/spec/sentry/propagation_context_spec.rb b/sentry-ruby/spec/sentry/propagation_context_spec.rb index 644fc9530..0d92ff1a9 100644 --- a/sentry-ruby/spec/sentry/propagation_context_spec.rb +++ b/sentry-ruby/spec/sentry/propagation_context_spec.rb @@ -72,6 +72,53 @@ }) end + it "generates correct attributes when reading from ENV['SENTRY_TRACE'] and ENV['SENTRY_BAGGAGE]" do + begin + ENV["SENTRY_TRACE"] = "771a43a4192642f0b136d5159a501700-7c51afd529da4a2a" + ENV["SENTRY_BAGGAGE"] = "other-vendor-value-1=foo;bar;baz, "\ + "sentry-trace_id=771a43a4192642f0b136d5159a501700, "\ + "sentry-public_key=49d0f7386ad645858ae85020e393bef3, "\ + "sentry-sample_rate=0.01337, "\ + "sentry-user_id=Am%C3%A9lie, "\ + "other-vendor-value-2=foo;bar;" + + subject = described_class.new(scope) + expect(subject.trace_id).to eq("771a43a4192642f0b136d5159a501700") + expect(subject.span_id.length).to eq(16) + expect(subject.parent_span_id).to eq("7c51afd529da4a2a") + expect(subject.parent_sampled).to eq(nil) + expect(subject.incoming_trace).to eq(true) + expect(subject.baggage).to be_a(Sentry::Baggage) + expect(subject.baggage.mutable).to eq(false) + expect(subject.baggage.items).to eq({ + "public_key"=>"49d0f7386ad645858ae85020e393bef3", + "sample_rate"=>"0.01337", + "trace_id"=>"771a43a4192642f0b136d5159a501700", + "user_id"=>"Amélie" + }) + ensure + ENV.delete("SENTRY_TRACE") + ENV.delete("SENTRY_BAGGAGE") + end + end + + it "generates correct new trace_id when ENV values are malformed" do + begin + ENV["SENTRY_TRACE"] = "I am borked lol" + ENV["SENTRY_BAGGAGE"] = "that's not it chief" + + subject = described_class.new(scope) + expect(subject.trace_id.length).to eq(32) + expect(subject.span_id.length).to eq(16) + expect(subject.parent_sampled).to eq(nil) + expect(subject.incoming_trace).to eq(false) + expect(subject.baggage).to eq(nil) + ensure + ENV.delete("SENTRY_TRACE") + ENV.delete("SENTRY_BAGGAGE") + end + end + it "generates correct attributes when incoming sentry-trace only (from older SDKs)" do env = { "sentry-trace" => "771a43a4192642f0b136d5159a501700-7c51afd529da4a2a" diff --git a/sentry-ruby/spec/sentry/span_spec.rb b/sentry-ruby/spec/sentry/span_spec.rb index 96d1f378b..18fac72a2 100644 --- a/sentry-ruby/spec/sentry/span_spec.rb +++ b/sentry-ruby/spec/sentry/span_spec.rb @@ -99,7 +99,7 @@ end subject do - baggage = Sentry::Baggage.from_incoming_header( + baggage = Sentry::Baggage.from_baggage_string( "other-vendor-value-1=foo;bar;baz, "\ "sentry-trace_id=771a43a4192642f0b136d5159a501700, "\ "sentry-public_key=49d0f7386ad645858ae85020e393bef3, "\ diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb index 008e0b540..e89f5b3a2 100644 --- a/sentry-ruby/spec/sentry/transaction_spec.rb +++ b/sentry-ruby/spec/sentry/transaction_spec.rb @@ -566,7 +566,7 @@ context "when incoming baggage with sentry items" do let(:incoming_baggage) do - Sentry::Baggage.from_incoming_header("sentry-trace_id=12345,foo=bar") + Sentry::Baggage.from_baggage_string("sentry-trace_id=12345,foo=bar") end it "does not populate new baggage" do @@ -580,7 +580,7 @@ context "when incoming baggage with no sentry items" do let(:incoming_baggage) do - Sentry::Baggage.from_incoming_header("foo=bar") + Sentry::Baggage.from_baggage_string("foo=bar") end it "populates sentry baggage" do