From 9641957b70a5e1342c24463567a1de67640a2815 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 1 Sep 2023 13:44:16 +0200 Subject: [PATCH] Tracing without performance (#2084) This PR is 1/2 to enable **Tracing without Performance**, i.e. make sure all our events are connected even if they are not Transactions. This enables use cases such as Errors / Transactions / Replays etc all being connected across services and not just Transactions. ### Summary of changes * new `PropagationContext` class that generates trace/span ids and baggage irrespective of whether there are transactions/spans active or not * this lives on the `Scope` * three new top level methods that first check the span and fallback on the scope's propagation context * `Sentry.get_traceparent` * `Sentry.get_baggage` * `Sentry.get_trace_propagation_headers` * move `dynamic_sampling_context` to `Event` from `TransactionEvent` since all events will now have this info * use the new top level helpers in `net/http` patch to set propagation headers closes #2056 also see #2089 --- This PR is 2/2 to enable Tracing without Performance, i.e. make sure all our events are connected even if they are not Transactions. ### Summary of changes * Implement new top level `Sentry.continue_trace(env, **options)` API that standardizes continuing an incoming trace from a rack env like hash. * Use this new API in rack/rails/sidekiq part of #2056 linked to #2084 --- CHANGELOG.md | 16 +++ sentry-rails/lib/sentry/rails/action_cable.rb | 5 +- .../lib/sentry/rails/capture_exceptions.rb | 5 +- sentry-ruby/lib/sentry-ruby.rb | 36 +++++ sentry-ruby/lib/sentry/client.rb | 6 +- sentry-ruby/lib/sentry/event.rb | 6 + sentry-ruby/lib/sentry/hub.rb | 44 ++++++ sentry-ruby/lib/sentry/net/http.rb | 24 ++-- sentry-ruby/lib/sentry/propagation_context.rb | 134 ++++++++++++++++++ .../lib/sentry/rack/capture_exceptions.rb | 5 +- sentry-ruby/lib/sentry/scope.rb | 19 ++- sentry-ruby/lib/sentry/span.rb | 2 +- sentry-ruby/lib/sentry/transaction.rb | 25 ++-- sentry-ruby/lib/sentry/transaction_event.rb | 3 - sentry-ruby/spec/sentry/event_spec.rb | 1 + sentry-ruby/spec/sentry/net/http_spec.rb | 38 +++-- .../spec/sentry/propagation_context_spec.rb | 134 ++++++++++++++++++ .../sentry/rack/capture_exceptions_spec.rb | 28 ++++ sentry-ruby/spec/sentry/scope_spec.rb | 22 ++- sentry-ruby/spec/sentry/span_spec.rb | 4 +- sentry-ruby/spec/sentry_spec.rb | 118 ++++++++++++++- .../sidekiq/sentry_context_middleware.rb | 9 +- .../sidekiq/sentry_context_middleware_spec.rb | 14 +- 23 files changed, 610 insertions(+), 88 deletions(-) create mode 100644 sentry-ruby/lib/sentry/propagation_context.rb create mode 100644 sentry-ruby/spec/sentry/propagation_context_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index f3e4cce68..40b8fe6e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,22 @@ config.trace_propagation_targets = [/.*/] # default is to all targets config.trace_propagation_targets = [/example.com/, 'foobar.org/api/v2'] ``` +- 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. + + ```rb + # rack application + def call(env) + transaction = Sentry.continue_trace(env, name: 'transaction', op: 'op') + Sentry.start_transaction(transaction: transaction) + end + ``` + + To inject headers into outgoing requests, use `Sentry.get_trace_propagation_headers` to get a hash of headers to add to your request. ### Bug Fixes 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 9efb6e6ec..daed11803 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -489,6 +489,42 @@ def add_global_event_processor(&block) Scope.add_global_event_processor(&block) end + # Returns the traceparent (sentry-trace) header for distributed tracing. + # Can be either from the currently active span or the propagation context. + # + # @return [String, nil] + def get_traceparent + return nil unless initialized? + get_current_hub.get_traceparent + end + + # Returns the baggage header for distributed tracing. + # Can be either from the currently active span or the propagation context. + # + # @return [String, nil] + def get_baggage + return nil unless initialized? + get_current_hub.get_baggage + end + + # Returns the a Hash containing sentry-trace and baggage. + # Can be either from the currently active span or the propagation context. + # + # @return [Hash, nil] + def get_trace_propagation_headers + return nil unless initialized? + 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/client.rb b/sentry-ruby/lib/sentry/client.rb index 3d8c0dba6..80f1165c8 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -148,6 +148,8 @@ def send_event(event, hint = nil) raise end + # @deprecated use Sentry.get_traceparent instead. + # # Generates a Sentry trace for distribted tracing from the given Span. # Returns `nil` if `config.propagate_traces` is `false`. # @param span [Span] the span to generate trace from. @@ -160,7 +162,9 @@ def generate_sentry_trace(span) trace end - # Generates a W3C Baggage header for distribted tracing from the given Span. + # @deprecated Use Sentry.get_baggage instead. + # + # Generates a W3C Baggage header for distributed tracing from the given Span. # Returns `nil` if `config.propagate_traces` is `false`. # @param span [Span] the span to generate trace from. # @return [String, nil] diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index 0a76f3f4a..b9f8a58e6 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -37,6 +37,11 @@ class Event # @return [RequestInterface] attr_reader :request + # Dynamic Sampling Context (DSC) that gets attached + # as the trace envelope header in the transport. + # @return [Hash, nil] + attr_accessor :dynamic_sampling_context + # @param configuration [Configuration] # @param integration_meta [Hash, nil] # @param message [String, nil] @@ -54,6 +59,7 @@ def initialize(configuration:, integration_meta: nil, message: nil) @tags = {} @fingerprint = [] + @dynamic_sampling_context = nil # configuration data that's directly used by events @server_name = configuration.server_name diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index 2b87b2694..7d9dca4c7 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -229,6 +229,50 @@ def with_session_tracking(&block) end_session end + def get_traceparent + return nil unless current_scope + + current_scope.get_span&.to_sentry_trace || + current_scope.propagation_context.get_traceparent + end + + def get_baggage + return nil unless current_scope + + current_scope.get_span&.to_baggage || + current_scope.propagation_context.get_baggage&.serialize + end + + def get_trace_propagation_headers + headers = {} + + traceparent = get_traceparent + headers[SENTRY_TRACE_HEADER_NAME] = traceparent if traceparent + + baggage = get_baggage + headers[BAGGAGE_HEADER_NAME] = baggage if baggage && !baggage.empty? + + 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/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 989d8f393..4b0345f52 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -31,7 +31,10 @@ def request(req, body = nil, &block) Sentry.with_child_span(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f) do |sentry_span| request_info = extract_request_info(req) - set_sentry_trace_header(req, sentry_span, request_info) + + if propagate_trace?(request_info[:url], Sentry.configuration) + set_propagation_headers(req) + end super.tap do |res| record_sentry_breadcrumb(request_info, res) @@ -49,17 +52,8 @@ def request(req, body = nil, &block) private - def set_sentry_trace_header(req, sentry_span, request_info) - return unless sentry_span - - client = Sentry.get_current_client - return unless propagate_trace?(request_info[:url], client.configuration.trace_propagation_targets) - - trace = client.generate_sentry_trace(sentry_span) - req[SENTRY_TRACE_HEADER_NAME] = trace if trace - - baggage = client.generate_baggage(sentry_span) - req[BAGGAGE_HEADER_NAME] = baggage if baggage && !baggage.empty? + def set_propagation_headers(req) + Sentry.get_trace_propagation_headers&.each { |k, v| req[k] = v } end def record_sentry_breadcrumb(request_info, res) @@ -96,8 +90,10 @@ def extract_request_info(req) result end - def propagate_trace?(url, trace_propagation_targets) - url && trace_propagation_targets.any? { |target| url.match?(target) } + def propagate_trace?(url, configuration) + url && + configuration.propagate_traces && + configuration.trace_propagation_targets.any? { |target| url.match?(target) } end end end diff --git a/sentry-ruby/lib/sentry/propagation_context.rb b/sentry-ruby/lib/sentry/propagation_context.rb new file mode 100644 index 000000000..1fe5bc1ea --- /dev/null +++ b/sentry-ruby/lib/sentry/propagation_context.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require "securerandom" +require "sentry/baggage" + +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] + attr_reader :trace_id + # An uuid that can be used to identify the span. + # @return [String] + attr_reader :span_id + # Span parent's span_id. + # @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, env = nil) + @scope = scope + @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. + # @return [Hash] + def get_trace_context + { + trace_id: trace_id, + span_id: span_id, + parent_span_id: parent_span_id + } + end + + # Returns the sentry-trace header from the propagation context. + # @return [String] + def get_traceparent + "#{trace_id}-#{span_id}" + end + + # 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 + end + + # Returns the Dynamic Sampling Context from the baggage. + # @return [String, nil] + def get_dynamic_sampling_context + get_baggage&.dynamic_sampling_context + end + + private + + def populate_head_baggage + return unless Sentry.initialized? + + configuration = Sentry.configuration + + items = { + "trace_id" => trace_id, + "environment" => configuration.environment, + "release" => configuration.release, + "public_key" => configuration.dsn&.public_key, + "user_segment" => @scope.user && @scope.user["segment"] + } + + items.compact! + @baggage = Baggage.new(items, mutable: false) + end + end +end 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 7f98cd079..afbc03ea0 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "sentry/breadcrumb_buffer" +require "sentry/propagation_context" require "etc" module Sentry @@ -20,7 +21,8 @@ class Scope :event_processors, :rack_env, :span, - :session + :session, + :propagation_context ] attr_reader(*ATTRIBUTES) @@ -50,7 +52,10 @@ def apply_to_event(event, hint = nil) event.transaction_info = { source: transaction_source } if transaction_source if span - event.contexts[:trace] = span.get_trace_context + event.contexts[:trace] ||= span.get_trace_context + else + event.contexts[:trace] ||= propagation_context.get_trace_context + event.dynamic_sampling_context ||= propagation_context.get_dynamic_sampling_context end event.fingerprint = fingerprint @@ -95,6 +100,7 @@ def dup copy.fingerprint = fingerprint.deep_dup copy.span = span.deep_dup copy.session = session.deep_dup + copy.propagation_context = propagation_context.deep_dup copy end @@ -111,6 +117,7 @@ def update_from_scope(scope) self.transaction_sources = scope.transaction_sources self.fingerprint = scope.fingerprint self.span = scope.span + self.propagation_context = scope.propagation_context end # Updates the scope's data from the given options. @@ -272,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 @@ -292,6 +306,7 @@ def set_default_value @rack_env = {} @span = nil @session = nil + generate_propagation_context set_new_breadcrumb_buffer end diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 8fda79fd8..95dd62efd 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -75,7 +75,7 @@ def initialize( timestamp: nil ) @trace_id = trace_id || SecureRandom.uuid.delete("-") - @span_id = span_id || SecureRandom.hex(8) + @span_id = span_id || SecureRandom.uuid.delete("-").slice(0, 16) @parent_span_id = parent_span_id @sampled = sampled @start_timestamp = start_timestamp || Sentry.utc_now.to_f 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/lib/sentry/transaction_event.rb b/sentry-ruby/lib/sentry/transaction_event.rb index 9b3f6909f..20d25445e 100644 --- a/sentry-ruby/lib/sentry/transaction_event.rb +++ b/sentry-ruby/lib/sentry/transaction_event.rb @@ -8,9 +8,6 @@ class TransactionEvent < Event # @return [] attr_accessor :spans - # @return [Hash, nil] - attr_accessor :dynamic_sampling_context - # @return [Hash] attr_accessor :measurements diff --git a/sentry-ruby/spec/sentry/event_spec.rb b/sentry-ruby/spec/sentry/event_spec.rb index cfa19b259..10c4b448c 100644 --- a/sentry-ruby/spec/sentry/event_spec.rb +++ b/sentry-ruby/spec/sentry/event_spec.rb @@ -30,6 +30,7 @@ expect(event.environment).to eq("test") expect(event.release).to eq("721e41770371db95eee98ca2707686226b993eda") expect(event.sdk).to eq("name" => "sentry.ruby", "version" => Sentry::VERSION) + expect(event.dynamic_sampling_context).to eq(nil) end end diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb index eaa79e955..49c628a22 100644 --- a/sentry-ruby/spec/sentry/net/http_spec.rb +++ b/sentry-ruby/spec/sentry/net/http_spec.rb @@ -97,9 +97,6 @@ response = http.request(request) expect(response.code).to eq("200") - expect(string_io.string).to match( - /\[Tracing\] Adding sentry-trace header to outgoing request:/ - ) end it "adds baggage header to the request header as head SDK when no incoming trace" do @@ -115,10 +112,6 @@ response = http.request(request) expect(response.code).to eq("200") - expect(string_io.string).to match( - /\[Tracing\] Adding baggage header to outgoing request:/ - ) - request_span = transaction.span_recorder.spans.last expect(request["baggage"]).to eq(request_span.to_baggage) expect(request["baggage"]).to eq( @@ -145,16 +138,12 @@ "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) expect(response.code).to eq("200") - expect(string_io.string).to match( - /\[Tracing\] Adding baggage header to outgoing request:/ - ) - request_span = transaction.span_recorder.spans.last expect(request["baggage"]).to eq(request_span.to_baggage) expect(request["baggage"]).to eq( @@ -165,7 +154,7 @@ ) end - context "with config.propagate_trace = false" do + context "with config.propagate_traces = false" do before do Sentry.configuration.propagate_traces = false end @@ -183,9 +172,6 @@ response = http.request(request) expect(response.code).to eq("200") - expect(string_io.string).not_to match( - /Adding sentry-trace header to outgoing request:/ - ) expect(request.key?("sentry-trace")).to eq(false) end @@ -204,15 +190,12 @@ "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) expect(response.code).to eq("200") - expect(string_io.string).not_to match( - /Adding baggage header to outgoing request:/ - ) expect(request.key?("baggage")).to eq(false) end end @@ -390,7 +373,20 @@ def verify_spans(transaction) perform_basic_setup end - it "doesn't affect the HTTP lib anything" do + it "adds sentry-trace and baggage headers for tracing without performance" do + stub_normal_response + + uri = URI("http://example.com/path") + http = Net::HTTP.new(uri.host, uri.port) + request = Net::HTTP::Get.new(uri.request_uri) + response = http.request(request) + + expect(request["sentry-trace"]).to eq(Sentry.get_traceparent) + expect(request["baggage"]).to eq(Sentry.get_baggage) + expect(response.code).to eq("200") + end + + it "doesn't create transaction or breadcrumbs" do stub_normal_response response = Net::HTTP.get_response(URI("http://example.com/path")) diff --git a/sentry-ruby/spec/sentry/propagation_context_spec.rb b/sentry-ruby/spec/sentry/propagation_context_spec.rb new file mode 100644 index 000000000..644fc9530 --- /dev/null +++ b/sentry-ruby/spec/sentry/propagation_context_spec.rb @@ -0,0 +1,134 @@ +require "spec_helper" + +RSpec.describe Sentry::PropagationContext do + before do + perform_basic_setup + end + + let(:scope) { Sentry.get_current_scope } + let(:subject) { described_class.new(scope) } + + describe "#initialize" 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 + + describe "#get_trace_context" do + it "generates correct trace context" do + expect(subject.get_trace_context).to eq({ + trace_id: subject.trace_id, + span_id: subject.span_id, + parent_span_id: subject.parent_span_id + }) + end + end + + describe "#get_traceparent" do + it "generates correct traceparent" do + expect(subject.get_traceparent).to eq("#{subject.trace_id}-#{subject.span_id}") + end + end + + describe "#get_baggage" do + before do + perform_basic_setup do |config| + config.environment = "test" + config.release = "foobar" + end + end + + it "populates head baggage" do + baggage = subject.get_baggage + + expect(baggage.mutable).to eq(false) + expect(baggage.items).to eq({ + "trace_id" => subject.trace_id, + "environment" => "test", + "release" => "foobar", + "public_key" => Sentry.configuration.dsn.public_key + }) + end + end + + describe "#get_dynamic_sampling_context" do + it "generates DSC from baggage" do + expect(subject.get_dynamic_sampling_context).to eq(subject.get_baggage.dynamic_sampling_context) + end + 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..24a90c87b 100644 --- a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb +++ b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb @@ -565,6 +565,34 @@ 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 + + before { perform_basic_setup } + + 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 1e6d92426..95f1a396a 100644 --- a/sentry-ruby/spec/sentry/scope_spec.rb +++ b/sentry-ruby/spec/sentry/scope_spec.rb @@ -24,6 +24,7 @@ expect(subject.fingerprint).to eq([]) expect(subject.transaction_names).to eq([]) expect(subject.transaction_sources).to eq([]) + expect(subject.propagation_context).to be_a(Sentry::PropagationContext) end it "allows setting breadcrumb buffer's size limit" do @@ -276,7 +277,7 @@ end end - it "sets trace context if there's a span" do + it "sets trace context from span if there's a span" do transaction = Sentry::Transaction.new(op: "foo", hub: hub) subject.set_span(transaction) @@ -286,6 +287,12 @@ expect(event.contexts.dig(:trace, :op)).to eq("foo") end + it "sets trace context and dynamic_sampling_context from propagation context if there's no span" do + subject.apply_to_event(event) + expect(event.contexts[:trace]).to eq(subject.propagation_context.get_trace_context) + expect(event.dynamic_sampling_context).to eq(subject.propagation_context.get_dynamic_sampling_context) + end + context "with Rack", rack: true do let(:env) do Rack::MockRequest.env_for("/test", {}) @@ -304,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 72fb18970..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) @@ -667,6 +667,118 @@ end end + describe ".get_traceparent" do + it "returns a valid traceparent header from scope propagation context" do + traceparent = described_class.get_traceparent + propagation_context = described_class.get_current_scope.propagation_context + + expect(traceparent).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) + expect(traceparent).to eq("#{propagation_context.trace_id}-#{propagation_context.span_id}") + end + + it "returns a valid traceparent header from scope current span" do + transaction = Sentry::Transaction.new(op: "foo", hub: Sentry.get_current_hub, sampled: true) + span = transaction.start_child(op: "parent") + described_class.get_current_scope.set_span(span) + + traceparent = described_class.get_traceparent + + expect(traceparent).to match(Sentry::PropagationContext::SENTRY_TRACE_REGEXP) + expect(traceparent).to eq("#{span.trace_id}-#{span.span_id}-1") + end + end + + describe ".get_baggage" do + it "returns a valid baggage header from scope propagation context" do + baggage = described_class.get_baggage + propagation_context = described_class.get_current_scope.propagation_context + + expect(baggage).to eq("sentry-trace_id=#{propagation_context.trace_id},sentry-environment=development,sentry-public_key=12345") + end + + it "returns a valid baggage header from scope current span" do + transaction = Sentry::Transaction.new(op: "foo", hub: Sentry.get_current_hub, sampled: true) + span = transaction.start_child(op: "parent") + described_class.get_current_scope.set_span(span) + + baggage = described_class.get_baggage + + expect(baggage).to eq("sentry-trace_id=#{span.trace_id},sentry-sampled=true,sentry-environment=development,sentry-public_key=12345") + end + end + + describe ".get_trace_propagation_headers" do + it "returns a Hash of sentry-trace and baggage" do + expect(described_class.get_trace_propagation_headers).to eq({ + "sentry-trace" => described_class.get_traceparent, + "baggage" => described_class.get_baggage + }) + 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