From 48717de0c8cf4c24f69516ec01a299b5d1ac18f3 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 31 Aug 2023 13:26:35 +0200 Subject: [PATCH] Tracing without performance: new continue_trace api (#2089) --- CHANGELOG.md | 1 + sentry-rails/lib/sentry/rails/action_cable.rb | 5 +- .../lib/sentry/rails/capture_exceptions.rb | 5 +- sentry-ruby/lib/sentry-ruby.rb | 9 +++ sentry-ruby/lib/sentry/hub.rb | 18 +++++ sentry-ruby/lib/sentry/propagation_context.rb | 71 ++++++++++++++++-- .../lib/sentry/rack/capture_exceptions.rb | 5 +- sentry-ruby/lib/sentry/scope.rb | 11 ++- sentry-ruby/lib/sentry/transaction.rb | 25 ++----- sentry-ruby/spec/sentry/net/http_spec.rb | 4 +- .../spec/sentry/propagation_context_spec.rb | 75 ++++++++++++++++++- .../sentry/rack/capture_exceptions_spec.rb | 26 +++++++ sentry-ruby/spec/sentry/scope_spec.rb | 13 ++++ sentry-ruby/spec/sentry/span_spec.rb | 4 +- sentry-ruby/spec/sentry_spec.rb | 73 ++++++++++++++++-- .../sidekiq/sentry_context_middleware.rb | 9 +-- .../sidekiq/sentry_context_middleware_spec.rb | 14 ++-- 17 files changed, 308 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fd1a7f6a..40b8fe6e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ``` - Tracing without Performance - Implement `PropagationContext` on `Scope` and add `Sentry.get_trace_propagation_headers` API [#2084](https://github.com/getsentry/sentry-ruby/pull/2084) + - Implement `Sentry.continue_trace` API [#2089](https://github.com/getsentry/sentry-ruby/pull/2089) The SDK now supports connecting arbitrary events (Errors / Transactions / Replays) across distributed services and not just Transactions. To continue an incoming trace starting with this version of the SDK, use `Sentry.continue_trace` as follows. diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index 5bf01f5da..155377466 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -33,11 +33,8 @@ def capture(connection, transaction_name:, extra_context: nil, &block) end def start_transaction(env, scope) - sentry_trace = env["HTTP_SENTRY_TRACE"] - baggage = env["HTTP_BAGGAGE"] - options = { name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME } - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + transaction = Sentry.continue_trace(env, **options) Sentry.start_transaction(transaction: transaction, **options) end diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb index a145d53dd..8d93784ed 100644 --- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb +++ b/sentry-rails/lib/sentry/rails/capture_exceptions.rb @@ -32,16 +32,13 @@ def capture_exception(exception, env) end def start_transaction(env, scope) - sentry_trace = env["HTTP_SENTRY_TRACE"] - baggage = env["HTTP_BAGGAGE"] - options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op } if @assets_regexp && scope.transaction_name.match?(@assets_regexp) options.merge!(sampled: false) end - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + transaction = Sentry.continue_trace(env, **options) Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) end diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 03edc0a08..daed11803 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -516,6 +516,15 @@ def get_trace_propagation_headers get_current_hub.get_trace_propagation_headers end + # Continue an incoming trace from a rack env like hash. + # + # @param env [Hash] + # @return [Transaction, nil] + def continue_trace(env, **options) + return nil unless initialized? + get_current_hub.continue_trace(env, **options) + end + ##### Helpers ##### # @!visibility private diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index c9f95eefe..7d9dca4c7 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -255,6 +255,24 @@ def get_trace_propagation_headers headers end + def continue_trace(env, **options) + configure_scope { |s| s.generate_propagation_context(env) } + + return nil unless configuration.tracing_enabled? + + propagation_context = current_scope.propagation_context + return nil unless propagation_context.incoming_trace + + Transaction.new( + hub: self, + trace_id: propagation_context.trace_id, + parent_span_id: propagation_context.parent_span_id, + parent_sampled: propagation_context.parent_sampled, + baggage: propagation_context.baggage, + **options + ) + end + private def current_layer diff --git a/sentry-ruby/lib/sentry/propagation_context.rb b/sentry-ruby/lib/sentry/propagation_context.rb index 9bf163b72..c76e73e90 100644 --- a/sentry-ruby/lib/sentry/propagation_context.rb +++ b/sentry-ruby/lib/sentry/propagation_context.rb @@ -5,6 +5,13 @@ module Sentry class PropagationContext + SENTRY_TRACE_REGEXP = Regexp.new( + "^[ \t]*" + # whitespace + "([0-9a-f]{32})?" + # trace_id + "-?([0-9a-f]{16})?" + # span_id + "-?([01])?" + # sampled + "[ \t]*$" # whitespace + ) # An uuid that can be used to identify a trace. # @return [String] @@ -13,15 +20,67 @@ class PropagationContext # @return [String] attr_reader :span_id # Span parent's span_id. - # @return [String] + # @return [String, nil] attr_reader :parent_span_id + # The sampling decision of the parent transaction. + # @return [Boolean, nil] + attr_reader :parent_sampled + # Is there an incoming trace or not? + # @return [Boolean] + attr_reader :incoming_trace + # This is only for accessing the current baggage variable. + # Please use the #get_baggage method for interfacing outside this class. + # @return [Baggage, nil] + attr_reader :baggage - def initialize(scope) + def initialize(scope, env = nil) @scope = scope - @trace_id = SecureRandom.uuid.delete("-") - @span_id = SecureRandom.uuid.delete("-").slice(0, 16) @parent_span_id = nil + @parent_sampled = 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 + end + end + + @trace_id ||= SecureRandom.uuid.delete("-") + @span_id = SecureRandom.uuid.delete("-").slice(0, 16) + end + + # Extract the trace_id, parent_span_id and parent_sampled values from a sentry-trace header. + # + # @param sentry_trace [String] the sentry-trace header value from the previous transaction. + # @return [Array, nil] + def self.extract_sentry_trace(sentry_trace) + match = SENTRY_TRACE_REGEXP.match(sentry_trace) + return nil if match.nil? + + trace_id, parent_span_id, sampled_flag = match[1..3] + parent_sampled = sampled_flag.nil? ? nil : sampled_flag != "0" + + [trace_id, parent_span_id, parent_sampled] end # Returns the trace context that can be used to embed in an Event. @@ -40,8 +99,8 @@ def get_traceparent "#{trace_id}-#{span_id}" end - # Returns the W3C baggage header from the propagation context. - # @return [String, nil] + # Returns the Baggage from the propagation context or populates as head SDK if empty. + # @return [Baggage, nil] def get_baggage populate_head_baggage if @baggage.nil? || @baggage.mutable @baggage diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index 36c8b1848..2248799a3 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -62,11 +62,8 @@ def capture_exception(exception, env) end def start_transaction(env, scope) - sentry_trace = env["HTTP_SENTRY_TRACE"] - baggage = env["HTTP_BAGGAGE"] - options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op } - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + transaction = Sentry.continue_trace(env, **options) Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) end diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index f0ea77982..afbc03ea0 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -279,6 +279,13 @@ def add_event_processor(&block) @event_processors << block end + # Generate a new propagation context either from the incoming env headers or from scratch. + # @param env [Hash, nil] + # @return [void] + def generate_propagation_context(env = nil) + @propagation_context = PropagationContext.new(self, env) + end + protected # for duplicating scopes internally @@ -307,10 +314,6 @@ def set_new_breadcrumb_buffer @breadcrumbs = BreadcrumbBuffer.new(@max_breadcrumbs) end - def generate_propagation_context - @propagation_context = PropagationContext.new(self) - end - class << self # @return [Hash] def os_context diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index 0dce5dcce..b154f71fb 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -2,16 +2,13 @@ require "sentry/baggage" require "sentry/profiler" +require "sentry/propagation_context" module Sentry class Transaction < Span - SENTRY_TRACE_REGEXP = Regexp.new( - "^[ \t]*" + # whitespace - "([0-9a-f]{32})?" + # trace_id - "-?([0-9a-f]{16})?" + # span_id - "-?([01])?" + # sampled - "[ \t]*$" # whitespace - ) + # @deprecated Use Sentry::PropagationContext::SENTRY_TRACE_REGEXP instead. + SENTRY_TRACE_REGEXP = PropagationContext::SENTRY_TRACE_REGEXP + UNLABELD_NAME = "".freeze MESSAGE_PREFIX = "[Tracing]" @@ -92,6 +89,8 @@ def initialize( init_span_recorder end + # @deprecated use Sentry.continue_trace instead. + # # Initalizes a Transaction instance with a Sentry trace string from another transaction (usually from an external request). # # The original transaction will become the parent of the new Transaction instance. And they will share the same `trace_id`. @@ -132,18 +131,10 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h ) end - # Extract the trace_id, parent_span_id and parent_sampled values from a sentry-trace header. - # - # @param sentry_trace [String] the sentry-trace header value from the previous transaction. + # @deprecated Use Sentry::PropagationContext.extract_sentry_trace instead. # @return [Array, nil] def self.extract_sentry_trace(sentry_trace) - match = SENTRY_TRACE_REGEXP.match(sentry_trace) - return nil if match.nil? - - trace_id, parent_span_id, sampled_flag = match[1..3] - parent_sampled = sampled_flag.nil? ? nil : sampled_flag != "0" - - [trace_id, parent_span_id, parent_sampled] + PropagationContext.extract_sentry_trace(sentry_trace) end # @return [Hash] diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb index 1e96f7feb..49c628a22 100644 --- a/sentry-ruby/spec/sentry/net/http_spec.rb +++ b/sentry-ruby/spec/sentry/net/http_spec.rb @@ -138,7 +138,7 @@ "sentry-user_id=Am%C3%A9lie, "\ "other-vendor-value-2=foo;bar;" - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage) + transaction = Sentry.continue_trace({ "sentry-trace" => sentry_trace, "baggage" => baggage }) Sentry.get_current_scope.set_span(transaction) response = http.request(request) @@ -190,7 +190,7 @@ "sentry-user_id=Am%C3%A9lie, "\ "other-vendor-value-2=foo;bar;" - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage) + transaction = Sentry.continue_trace({ "sentry-trace" => sentry_trace, "baggage" => baggage }) Sentry.get_current_scope.set_span(transaction) response = http.request(request) diff --git a/sentry-ruby/spec/sentry/propagation_context_spec.rb b/sentry-ruby/spec/sentry/propagation_context_spec.rb index ce2c337b2..60728bc11 100644 --- a/sentry-ruby/spec/sentry/propagation_context_spec.rb +++ b/sentry-ruby/spec/sentry/propagation_context_spec.rb @@ -9,10 +9,83 @@ let(:subject) { described_class.new(scope) } describe "#initialize" do - it "generates correct attributes" do + it "generates correct attributes without env" do expect(subject.trace_id.length).to eq(32) expect(subject.span_id.length).to eq(16) expect(subject.parent_span_id).to be_nil + expect(subject.parent_sampled).to be_nil + expect(subject.baggage).to be_nil + expect(subject.incoming_trace).to eq(false) + end + + it "generates correct attributes when incoming sentry-trace and baggage" do + env = { + "sentry-trace" => "771a43a4192642f0b136d5159a501700-7c51afd529da4a2a", + "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, env) + 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" + }) + end + + it "generates correct attributes when incoming HTTP_SENTRY_TRACE and HTTP_BAGGAGE" do + env = { + "HTTP_SENTRY_TRACE" => "771a43a4192642f0b136d5159a501700-7c51afd529da4a2a", + "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;" + } + + subject = described_class.new(scope, env) + 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" + }) + end + + it "generates correct attributes when incoming sentry-trace only (from older SDKs)" do + env = { + "sentry-trace" => "771a43a4192642f0b136d5159a501700-7c51afd529da4a2a" + } + + subject = described_class.new(scope, env) + 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({}) end end diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb index 7429b2d0d..22766342c 100644 --- a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb +++ b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb @@ -565,6 +565,32 @@ def will_be_sampled_by_sdk end end + describe "tracing without performance" do + let(:incoming_prop_context) { Sentry::PropagationContext.new(Sentry::Scope.new) } + let(:env) do + { + "HTTP_SENTRY_TRACE" => incoming_prop_context.get_traceparent, + "HTTP_BAGGAGE" => incoming_prop_context.get_baggage.serialize + } + end + + let(:stack) do + app = ->(_e) { raise exception } + described_class.new(app) + end + + it "captures exception with correct DSC and trace context" do + expect { stack.call(env) }.to raise_error(ZeroDivisionError) + + trace_context = last_sentry_event.contexts[:trace] + expect(trace_context[:trace_id]).to eq(incoming_prop_context.trace_id) + expect(trace_context[:parent_span_id]).to eq(incoming_prop_context.span_id) + expect(trace_context[:span_id].length).to eq(16) + + expect(last_sentry_event.dynamic_sampling_context).to eq(incoming_prop_context.get_dynamic_sampling_context) + end + end + describe "session capturing" do context "when auto_session_tracking is false" do before do diff --git a/sentry-ruby/spec/sentry/scope_spec.rb b/sentry-ruby/spec/sentry/scope_spec.rb index 3573e3a6e..95f1a396a 100644 --- a/sentry-ruby/spec/sentry/scope_spec.rb +++ b/sentry-ruby/spec/sentry/scope_spec.rb @@ -311,4 +311,17 @@ end end end + + describe "#generate_propagation_context" do + it "initializes new propagation context without env" do + expect(Sentry::PropagationContext).to receive(:new).with(subject, nil) + subject.generate_propagation_context + end + + it "initializes new propagation context without env" do + env = { foo: 42 } + expect(Sentry::PropagationContext).to receive(:new).with(subject, env) + subject.generate_propagation_context(env) + end + end end diff --git a/sentry-ruby/spec/sentry/span_spec.rb b/sentry-ruby/spec/sentry/span_spec.rb index 5518f23c5..1d50896c3 100644 --- a/sentry-ruby/spec/sentry/span_spec.rb +++ b/sentry-ruby/spec/sentry/span_spec.rb @@ -77,7 +77,7 @@ sentry_trace = subject.to_sentry_trace expect(sentry_trace).to eq("#{subject.trace_id}-#{subject.span_id}-1") - expect(sentry_trace).to match(Sentry::Transaction::SENTRY_TRACE_REGEXP) + expect(sentry_trace).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) end context "without sampled value" do @@ -87,7 +87,7 @@ sentry_trace = subject.to_sentry_trace expect(sentry_trace).to eq("#{subject.trace_id}-#{subject.span_id}-") - expect(sentry_trace).to match(Sentry::Transaction::SENTRY_TRACE_REGEXP) + expect(sentry_trace).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) end end end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index beb931131..80c74180f 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -331,18 +331,18 @@ unsampled_trace = "d298e6b033f84659928a2267c3879aaa-2a35b8e9a1b974f4-0" not_sampled_trace = "d298e6b033f84659928a2267c3879aaa-2a35b8e9a1b974f4-" - transaction = Sentry::Transaction.from_sentry_trace(sampled_trace, op: "rack.request", name: "/payment") + transaction = Sentry.continue_trace({ "sentry-trace" => sampled_trace }, op: "rack.request", name: "/payment") described_class.start_transaction(transaction: transaction) expect(transaction.sampled).to eq(true) - transaction = Sentry::Transaction.from_sentry_trace(unsampled_trace, op: "rack.request", name: "/payment") + transaction = Sentry.continue_trace({ "sentry-trace" => unsampled_trace }, op: "rack.request", name: "/payment") described_class.start_transaction(transaction: transaction) expect(transaction.sampled).to eq(false) allow(Random).to receive(:rand).and_return(0.4) - transaction = Sentry::Transaction.from_sentry_trace(not_sampled_trace, op: "rack.request", name: "/payment") + transaction = Sentry.continue_trace({ "sentry-trace" => not_sampled_trace }, op: "rack.request", name: "/payment") described_class.start_transaction(transaction: transaction) expect(transaction.sampled).to eq(true) @@ -672,7 +672,7 @@ traceparent = described_class.get_traceparent propagation_context = described_class.get_current_scope.propagation_context - expect(traceparent).to match(Sentry::Transaction::SENTRY_TRACE_REGEXP) + expect(traceparent).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) expect(traceparent).to eq("#{propagation_context.trace_id}-#{propagation_context.span_id}") end @@ -683,7 +683,7 @@ traceparent = described_class.get_traceparent - expect(traceparent).to match(Sentry::Transaction::SENTRY_TRACE_REGEXP) + expect(traceparent).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) expect(traceparent).to eq("#{span.trace_id}-#{span.span_id}-1") end end @@ -716,6 +716,69 @@ end end + describe ".continue_trace" do + + context "without incoming sentry trace" do + let(:env) { { "HTTP_FOO" => "bar" } } + + it "returns nil with tracing disabled" do + expect(described_class.continue_trace(env)).to eq(nil) + end + + it "returns nil with tracing enabled" do + Sentry.configuration.traces_sample_rate = 1.0 + expect(described_class.continue_trace(env)).to eq(nil) + end + + it "sets new propagation context on scope" do + expect(Sentry.get_current_scope).to receive(:generate_propagation_context).and_call_original + described_class.continue_trace(env) + + propagation_context = Sentry.get_current_scope.propagation_context + expect(propagation_context.incoming_trace).to eq(false) + end + end + + context "with incoming sentry trace" do + let(:incoming_prop_context) { Sentry::PropagationContext.new(Sentry::Scope.new) } + let(:env) do + { + "HTTP_SENTRY_TRACE" => incoming_prop_context.get_traceparent, + "HTTP_BAGGAGE" => incoming_prop_context.get_baggage.serialize + } + end + + it "returns nil with tracing disabled" do + expect(described_class.continue_trace(env)).to eq(nil) + end + + it "sets new propagation context from env on scope" do + expect(Sentry.get_current_scope).to receive(:generate_propagation_context).and_call_original + described_class.continue_trace(env) + + propagation_context = Sentry.get_current_scope.propagation_context + expect(propagation_context.incoming_trace).to eq(true) + expect(propagation_context.trace_id).to eq(incoming_prop_context.trace_id) + expect(propagation_context.parent_span_id).to eq(incoming_prop_context.span_id) + expect(propagation_context.parent_sampled).to eq(nil) + expect(propagation_context.baggage.items).to eq(incoming_prop_context.get_baggage.items) + expect(propagation_context.baggage.mutable).to eq(false) + end + + it "returns new Transaction with tracing enabled" do + Sentry.configuration.traces_sample_rate = 1.0 + + transaction = described_class.continue_trace(env, name: "foobar") + expect(transaction).to be_a(Sentry::Transaction) + expect(transaction.name).to eq("foobar") + expect(transaction.trace_id).to eq(incoming_prop_context.trace_id) + expect(transaction.parent_span_id).to eq(incoming_prop_context.span_id) + expect(transaction.baggage.items).to eq(incoming_prop_context.get_baggage.items) + expect(transaction.baggage.mutable).to eq(false) + end + end + end + describe 'release detection' do let(:fake_root) { "/tmp/sentry/" } diff --git a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb index 646c40e60..fcb6e0ada 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb @@ -19,7 +19,7 @@ def call(_worker, job, queue) scope.set_tags(build_tags(job["tags"])) scope.set_contexts(sidekiq: job.merge("queue" => queue)) scope.set_transaction_name(context_filter.transaction_name, source: :task) - transaction = start_transaction(scope, job["sentry_trace"]) + transaction = start_transaction(scope, job["trace_propagation_headers"]) scope.set_span(transaction) if transaction begin @@ -39,9 +39,9 @@ def build_tags(tags) Array(tags).each_with_object({}) { |name, tags_hash| tags_hash[:"sidekiq.#{name}"] = true } end - def start_transaction(scope, sentry_trace) + def start_transaction(scope, env) options = { name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME } - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace + transaction = Sentry.continue_trace(env, **options) Sentry.start_transaction(transaction: transaction, **options) end @@ -58,9 +58,8 @@ def call(_worker_class, job, _queue, _redis_pool) return yield unless Sentry.initialized? user = Sentry.get_current_scope.user - transaction = Sentry.get_current_scope.get_transaction job["sentry_user"] = user unless user.empty? - job["sentry_trace"] = transaction.to_sentry_trace if transaction + job["trace_propagation_headers"] = Sentry.get_trace_propagation_headers yield end end diff --git a/sentry-sidekiq/spec/sentry/sidekiq/sentry_context_middleware_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq/sentry_context_middleware_spec.rb index c07f18050..3a2b48de0 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq/sentry_context_middleware_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq/sentry_context_middleware_spec.rb @@ -57,11 +57,12 @@ expect(event.tags.keys).to include(:"sidekiq.marvel", :"sidekiq.dc") end - context "with sentry_trace" do + context "with trace_propagation_headers" do let(:parent_transaction) { Sentry.start_transaction(op: "sidekiq") } it "starts the transaction from it" do - execute_worker(processor, HappyWorker, sentry_trace: parent_transaction.to_sentry_trace) + trace_propagation_headers = { "sentry-trace" => parent_transaction.to_sentry_trace } + execute_worker(processor, HappyWorker, trace_propagation_headers: trace_propagation_headers) expect(transport.events.count).to eq(1) transaction = transport.events.first @@ -93,12 +94,11 @@ end end - it "does not add user or sentry_trace to the job if they're absence in the current scope" do + it "does not add user to the job if they're absent in the current scope" do client.push('queue' => 'default', 'class' => HappyWorker, 'args' => []) expect(queue.size).to be(1) expect(queue.first["sentry_user"]).to be_nil - expect(queue.first["sentry_trace"]).to be_nil end describe "with user" do @@ -121,11 +121,13 @@ Sentry.get_current_scope.set_span(transaction) end - it "sets user of the current scope to the job" do + it "sets the correct trace_propagation_headers linked to the transaction" do client.push('queue' => 'default', 'class' => HappyWorker, 'args' => []) expect(queue.size).to be(1) - expect(queue.first["sentry_trace"]).to eq(transaction.to_sentry_trace) + headers = queue.first["trace_propagation_headers"] + expect(headers["sentry-trace"]).to eq(transaction.to_sentry_trace) + expect(headers["baggage"]).to eq(transaction.to_baggage) end end end