Skip to content
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

Allow SENTRY_TRACE and SENTRY_BAGGAGE env vars to continue traces #2179

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/lib/sentry/baggage.rb
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this method is supposed to be public or not. If it is public for users, we can't simply rename it as it'll break users' apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. Even if it's not public by design, folks might be using it. I could either make a deprecated wrapper with the old name and keep the new name, or revert the rename, either are fine by me.

items = {}
mutable = true

header.split(',').each do |item|
baggage_string.split(',').each do |item|
item = item.strip
key, val = item.split('=')

Expand Down
50 changes: 28 additions & 22 deletions sentry-ruby/lib/sentry/propagation_context.rb
Expand Up @@ -40,31 +40,37 @@
@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"]
Copy link
Collaborator

@st0012 st0012 Nov 25, 2023

Choose a reason for hiding this comment

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

I may be wrong, but I don't think the "environment variables" here means the system environment variables. It likely means request envs too?


# 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)

Check warning on line 54 in sentry-ruby/lib/sentry/propagation_context.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/propagation_context.rb#L54

Added line #L54 was not covered by tests

if sentry_trace_data
@trace_id, @parent_span_id, @parent_sampled = sentry_trace_data

Check warning on line 57 in sentry-ruby/lib/sentry/propagation_context.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/propagation_context.rb#L56-L57

Added lines #L56 - L57 were not covered by tests

@baggage = if baggage_string && !baggage_string.empty?
Baggage.from_baggage_string(baggage_string)

Check warning on line 60 in sentry-ruby/lib/sentry/propagation_context.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/propagation_context.rb#L59-L60

Added lines #L59 - L60 were not covered by tests
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({})

Check warning on line 65 in sentry-ruby/lib/sentry/propagation_context.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/propagation_context.rb#L65

Added line #L65 was not covered by tests
end

@baggage.freeze!
@incoming_trace = true

Check warning on line 69 in sentry-ruby/lib/sentry/propagation_context.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/propagation_context.rb#L68-L69

Added lines #L68 - L69 were not covered by tests
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
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/transaction.rb
Expand Up @@ -111,7 +111,7 @@
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)

Check warning on line 114 in sentry-ruby/lib/sentry/transaction.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/transaction.rb#L114

Added line #L114 was not covered by tests
else
# If there's an incoming sentry-trace but no incoming baggage header,
# for instance in traces coming from older SDKs,
Expand Down
18 changes: 9 additions & 9 deletions sentry-ruby/spec/sentry/baggage_spec.rb
Expand Up @@ -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",
Expand All @@ -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,"\
Expand All @@ -79,22 +79,22 @@
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
end

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)
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/spec/sentry/client_spec.rb
Expand Up @@ -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, "\
Expand Down Expand Up @@ -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;"
Expand Down
47 changes: 47 additions & 0 deletions sentry-ruby/spec/sentry/propagation_context_spec.rb
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/span_spec.rb
Expand Up @@ -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, "\
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/spec/sentry/transaction_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down