Skip to content

Commit

Permalink
Move baggage to from_sentry_trace constructor because it is always co…
Browse files Browse the repository at this point in the history
…upled with sentry_trace
  • Loading branch information
sl0thentr0py committed Sep 19, 2022
1 parent 6ae3ad8 commit 8b8af16
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 74 deletions.
6 changes: 4 additions & 2 deletions sentry-rails/lib/sentry/rails/action_cable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ def capture(connection, transaction_name:, extra_context: nil, &block)

def start_transaction(env, transaction_name)
sentry_trace = env["HTTP_SENTRY_TRACE"]
options = { name: transaction_name, op: "rails.action_cable".freeze, baggage: env["HTTP_BAGGAGE"] }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
baggage = env["HTTP_BAGGAGE"]

options = { name: transaction_name, op: "rails.action_cable".freeze }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, **options)
end

Expand Down
6 changes: 4 additions & 2 deletions sentry-rails/lib/sentry/rails/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ def capture_exception(exception, env)

def start_transaction(env, scope)
sentry_trace = env["HTTP_SENTRY_TRACE"]
options = { name: scope.transaction_name, op: transaction_op, baggage: env["HTTP_BAGGAGE"] }
baggage = env["HTTP_BAGGAGE"]

options = { name: scope.transaction_name, op: transaction_op }

if @assets_regex && scope.transaction_name.match?(@assets_regex)
options.merge!(sampled: false)
end

transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options)
end
end
Expand Down
38 changes: 14 additions & 24 deletions sentry-rails/spec/sentry/rails/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@
expect(second_span[:description]).to eq("PostsController#show")
end

context "with sentry-trace header" do
context "with sentry-trace and baggage headers" do
let(:external_transaction) do
Sentry::Transaction.new(
op: "pageload",
Expand All @@ -167,10 +167,20 @@
)
end

let(:baggage) do
"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;"
end

it "inherits trace info from the transaction" do
p = Post.create!

get "/posts/#{p.id}", headers: { "sentry-trace" => external_transaction.to_sentry_trace }
headers = { "sentry-trace" => external_transaction.to_sentry_trace, baggage: baggage }
get "/posts/#{p.id}", headers: headers

transaction = transport.events.last
expect(transaction.type).to eq("transaction")
Expand All @@ -183,29 +193,9 @@
expect(transaction.contexts.dig(:trace, :trace_id)).to eq(external_transaction.trace_id)
expect(transaction.contexts.dig(:trace, :parent_span_id)).to eq(external_transaction.span_id)
expect(transaction.contexts.dig(:trace, :span_id)).not_to eq(external_transaction.span_id)
end
end

context "with baggage header" do
let(:baggage_string) do
"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;"
end

it "has the dynamic_sampling_context on the TransactionEvent" do
expect(Sentry::Transaction).to receive(:new).
with(hash_including(baggage: baggage_string)).
and_call_original

p = Post.create!
get "/posts/#{p.id}", headers: { "baggage" => baggage_string }

transaction_event = transport.events.last
expect(transaction_event.dynamic_sampling_context).to eq({
# should have baggage converted to DSC
expect(transaction.dynamic_sampling_context).to eq({
"sample_rate" => "0.01337",
"public_key" => "49d0f7386ad645858ae85020e393bef3",
"trace_id" => "771a43a4192642f0b136d5159a501700",
Expand Down
6 changes: 4 additions & 2 deletions sentry-ruby/lib/sentry/rack/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ def capture_exception(exception, env)

def start_transaction(env, scope)
sentry_trace = env["HTTP_SENTRY_TRACE"]
options = { name: scope.transaction_name, op: transaction_op, baggage: env["HTTP_BAGGAGE"] }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
baggage = env["HTTP_BAGGAGE"]

options = { name: scope.transaction_name, op: transaction_op }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options)
end

Expand Down
16 changes: 13 additions & 3 deletions sentry-ruby/lib/sentry/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def initialize(name: nil, parent_sampled: nil, baggage: nil, hub:, **options)
@parent_sampled = parent_sampled
@transaction = self
@hub = hub
@baggage = Baggage.from_incoming_header(baggage) if baggage
@baggage = baggage
@configuration = hub.configuration # to be removed
@tracing_enabled = hub.configuration.tracing_enabled?
@traces_sampler = hub.configuration.traces_sampler
Expand All @@ -59,10 +59,11 @@ def initialize(name: nil, parent_sampled: nil, baggage: nil, hub:, **options)
#
# The child transaction will also store the parent's sampling decision in its `parent_sampled` attribute.
# @param sentry_trace [String] the trace string from the previous transaction.
# @param baggage [String, nil] the incoming baggage header string.
# @param hub [Hub] the hub that'll be responsible for sending this transaction when it's finished.
# @param options [Hash] the options you want to use to initialize a Transaction instance.
# @return [Transaction, nil]
def self.from_sentry_trace(sentry_trace, hub: Sentry.get_current_hub, **options)
def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_hub, **options)
return unless hub.configuration.tracing_enabled?
return unless sentry_trace

Expand All @@ -77,7 +78,16 @@ def self.from_sentry_trace(sentry_trace, hub: Sentry.get_current_hub, **options)
sampled_flag != "0"
end

new(trace_id: trace_id, parent_span_id: parent_span_id, parent_sampled: parent_sampled, hub: hub, **options)
baggage = Baggage.from_incoming_header(baggage) if baggage

new(
trace_id: trace_id,
parent_span_id: parent_span_id,
parent_sampled: parent_sampled,
hub: hub,
baggage: baggage,
**options
)
end

# @return [Hash]
Expand Down
12 changes: 8 additions & 4 deletions sentry-ruby/spec/sentry/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,14 @@ def sentry_context

context "when baggage present" do
let(:transaction) do
baggage = "other-vendor-value-1=foo;bar;baz, "\
baggage = Sentry::Baggage.from_incoming_header(
"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;"
)


Sentry::Transaction.new(name: "test transaction", hub: hub, baggage: baggage, sampled: true)
Expand Down Expand Up @@ -421,9 +423,11 @@ module ExcTag; end
let(:string_io) { StringIO.new }
let(:logger) { ::Logger.new(string_io) }
let(:baggage) do
"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;"
Sentry::Baggage.from_incoming_header(
"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;"
)
end

let(:span) do
Expand Down
61 changes: 25 additions & 36 deletions sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,9 @@ def inspect
end

describe "performance monitoring" do
let(:traces_sample_rate) { 0.5 }

before do
perform_basic_setup do |config|
config.traces_sample_rate = traces_sample_rate
config.traces_sample_rate = 0.5
end
end

Expand Down Expand Up @@ -384,44 +382,35 @@ def will_be_sampled_by_sdk
expect(sampling_context_env).to eq(env)
end
end
end

context "when the baggage header is sent" do
let(:traces_sample_rate) { 1.0 }
let(:baggage_string) do
"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;"
end

let(:additional_headers) { { "HTTP_BAGGAGE" => baggage_string } }

let(:stack) do
described_class.new(
->(_) do
[200, {}, ["ok"]]
end
)
end
context "when the baggage header is sent" do
let(:trace) do
"#{external_transaction.trace_id}-#{external_transaction.span_id}-1"
end

let(:transaction_event) { last_sentry_event }
before do
env["HTTP_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;"
end

it "has the dynamic_sampling_context on the TransactionEvent" do
expect(Sentry::Transaction).to receive(:new).
with(hash_including(baggage: baggage_string)).
and_call_original
it "has the dynamic_sampling_context on the TransactionEvent" do
expect(Sentry::Transaction).to receive(:new).
with(hash_including(:baggage)).
and_call_original

stack.call(env)
stack.call(env)

expect(transaction_event.dynamic_sampling_context).to eq({
"sample_rate" => "0.01337",
"public_key" => "49d0f7386ad645858ae85020e393bef3",
"trace_id" => "771a43a4192642f0b136d5159a501700",
"user_id" => "Amélie"
})
expect(transaction.dynamic_sampling_context).to eq({
"sample_rate" => "0.01337",
"public_key" => "49d0f7386ad645858ae85020e393bef3",
"trace_id" => "771a43a4192642f0b136d5159a501700",
"user_id" => "Amélie"
})
end
end
end

Expand Down
4 changes: 3 additions & 1 deletion sentry-ruby/spec/sentry/span_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,14 @@
end

subject do
baggage = "other-vendor-value-1=foo;bar;baz, "\
baggage = Sentry::Baggage.from_incoming_header(
"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;"
)

Sentry::Transaction.new(hub: Sentry.get_current_hub, baggage: baggage).start_child
end
Expand Down

0 comments on commit 8b8af16

Please sign in to comment.