From 0b8eeb6d9a376e522d80877a183b96f2133773e2 Mon Sep 17 00:00:00 2001 From: Matt Quinn Date: Fri, 26 Aug 2022 00:10:24 -0400 Subject: [PATCH 01/44] Add basic OpenTelemetry support For users who already use OpenTelemetry instrumentation or would prefer to use it instead of Sentry's own tracing API, provide a means to disable Sentry's automatic trace instrumentation and a span processor that exports OpenTelemetry spans to Sentry. Usage: ```ruby Sentry.init do |config| # ... # Enable trace collection config.traces_sample_rate = 1.0 # or use traces_sampler # Disable automatic span instrumentation config.auto_trace_instrumentation = false end OpenTelemetry::SDK.configure do |c| # ... c.add_span_processor(Sentry::OpenTelemetry::SpanProcessor.new) end ``` Setting `auto_instrument_traces` to `false` in `Sentry.init` stops Sentry from automatically creating any transactions or spans. (Tracing must still be enabled with `traces_sample_rate` or `traces_sampler`.) `Sentry::OpenTelemetry::SpanProcessor` is notified whenever an OTel span is started or stopped. In reaction, it starts or stops a Sentry transaction (if the span has no parent) or span. When the transaction is finished it is picked up and processed by Sentry's worker thread like any other transaction would be. If the child service uses the `Sentry::Rack::CaptureExceptions` middleware (see order caveat below), then its transactions will be linked inside Sentry with the parent service's transaction. Caveats: sentry-rails is not yet supported (haven't looked into it yet). `Sentry.init` must be run before `OpenTelemetry::SDK.configure` (Sentry's `Net::HTTP` monkey patch to add the `sentry-trace` header must run after OTel's monkey-patch that adds an HTTP span) `Sentry::Rack::CaptureExceptions` must be before `OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware` in the middleware stack (the hub and scope must be set up before OTel creates any spans) Since Sentry spans are started and stopped in reaction to OTel spans starting and stopping, the durations reported might be slightly different. (This should be fixable by overriding the timestamps on the Sentry spans we create to match OTel's). In the case where an OTel root span has a child span that starts after the root span has finished, this will be represented in Sentry as two transactions (one for the original root span, and one for the late child). --- sentry-ruby/lib/sentry-ruby.rb | 1 + sentry-ruby/lib/sentry/configuration.rb | 4 ++ sentry-ruby/lib/sentry/net/http.rb | 8 ++- .../sentry/open_telemetry/span_processor.rb | 63 +++++++++++++++++++ .../lib/sentry/rack/capture_exceptions.rb | 8 ++- sentry-ruby/lib/sentry/redis.rb | 4 +- sentry-ruby/lib/sentry/scope.rb | 9 ++- 7 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 sentry-ruby/lib/sentry/open_telemetry/span_processor.rb diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 1bd3c092e..2a6846122 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -19,6 +19,7 @@ require "sentry/hub" require "sentry/background_worker" require "sentry/session_flusher" +require "sentry/open_telemetry/span_processor" [ "sentry/rake", diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 5cbc08355..473086818 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -211,6 +211,9 @@ class Configuration # @return [Boolean] attr_accessor :auto_session_tracking + # Automatically instrument supported libraries with transactions and spans. + attr_accessor :auto_instrument_traces + # these are not config options # @!visibility private attr_reader :errors, :gem_specs @@ -269,6 +272,7 @@ def initialize self.trusted_proxies = [] self.dsn = ENV['SENTRY_DSN'] self.server_name = server_name_from_env + self.auto_instrument_traces = true self.before_send = nil self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 510d817d4..c664dd219 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -28,8 +28,12 @@ module HTTP def request(req, body = nil, &block) return super unless started? - sentry_span = start_sentry_span - set_sentry_trace_header(req, sentry_span) + if Sentry.configuration.auto_instrument_traces + sentry_span = start_sentry_span + set_sentry_trace_header(req, sentry_span) + else + set_sentry_trace_header(req, Sentry.get_current_scope.get_span) + end super.tap do |res| record_sentry_breadcrumb(req, res) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb new file mode 100644 index 000000000..03df7b3cc --- /dev/null +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -0,0 +1,63 @@ +module Sentry + module OpenTelemetry + class SpanProcessor + def initialize + @otel_span_map = {} + end + + def on_start(otel_span, _parent_context) + return if from_sentry_sdk?(otel_span) + + scope = Sentry.get_current_scope + parent_sentry_span = scope.get_span + + sentry_span = if parent_sentry_span + parent_sentry_span.start_child(op: otel_span.name) + else + options = { name: otel_span.name, op: otel_span.name } + transaction = Sentry::Transaction.from_sentry_trace(scope.trace, **options) if scope.trace + scope.set_trace(nil) + Sentry.start_transaction(transaction: transaction, **options) + end + + scope.set_span(sentry_span) + @otel_span_map[otel_span.context.span_id] = [sentry_span, parent_sentry_span] + end + + def on_finish(otel_span) + current_scope = Sentry.get_current_scope + sentry_span, parent_span = @otel_span_map.delete(otel_span.context.span_id) + return unless sentry_span + + sentry_span.set_op(otel_span.name) + current_scope.set_transaction_name(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) + + otel_span.attributes&.each do |key, value| + sentry_span.set_data(key, value) + if key == "db.statement" + sentry_span.set_description(value) + end + end + + sentry_span.finish + current_scope.set_span(parent_span) if parent_span + end + + def force_flush(timeout: nil) + # no-op: we rely on Sentry.close being called for the same reason as + # whatever triggered this shutdown. + end + + def shutdown(timeout: nil) + # no-op: we rely on Sentry.close being called for the same reason as + # whatever triggered this shutdown. + end + + private + + def from_sentry_sdk?(otel_span) + caller.any? { |line| line =~ /lib[\\\/]sentry[\\\/]background_worker.rb/ } + end + end + end +end diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index f4803df5f..802e64d96 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -21,7 +21,10 @@ def call(env) scope.set_transaction_name(env["PATH_INFO"]) if env["PATH_INFO"] scope.set_rack_env(env) - transaction = start_transaction(env, scope) + sentry_trace = env["HTTP_SENTRY_TRACE"] + scope.set_trace(sentry_trace) + + transaction = start_transaction(env, scope, sentry_trace) if Sentry.configuration.auto_instrument_traces scope.set_span(transaction) if transaction begin @@ -61,8 +64,7 @@ def capture_exception(exception, env) end end - def start_transaction(env, scope) - sentry_trace = env["HTTP_SENTRY_TRACE"] + def start_transaction(env, scope, sentry_trace) options = { name: scope.transaction_name, op: transaction_op } transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb index 483563ce7..88e5a0d35 100644 --- a/sentry-ruby/lib/sentry/redis.rb +++ b/sentry-ruby/lib/sentry/redis.rb @@ -83,7 +83,9 @@ def logging(commands, &block) end if defined?(::Redis::Client) - Sentry.register_patch do + Sentry.register_patch do |config| + return unless config.auto_instrument_traces + patch = Sentry::Redis::Client Redis::Client.prepend(patch) unless Redis::Client.ancestors.include?(patch) end diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index 55630a656..34ddb49d3 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -7,7 +7,7 @@ module Sentry class Scope include ArgumentCheckingHelper - ATTRIBUTES = [:transaction_names, :contexts, :extra, :tags, :user, :level, :breadcrumbs, :fingerprint, :event_processors, :rack_env, :span, :session] + ATTRIBUTES = [:transaction_names, :contexts, :extra, :tags, :user, :level, :breadcrumbs, :fingerprint, :event_processors, :rack_env, :span, :session, :trace] attr_reader(*ATTRIBUTES) @@ -241,6 +241,13 @@ def add_event_processor(&block) @event_processors << block end + # Sets the scope's trace (the header value used by Sentry to propagate traces). + def set_trace(trace) + check_argument_type!(trace, String) unless trace.nil? + + @trace = trace + end + protected # for duplicating scopes internally From 07f6b817889c7180acd2db5965f2f1d2c10de7b8 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Oct 2022 14:50:16 +0200 Subject: [PATCH 02/44] Move to config.instrumenter - values :sentry/:otel instead of auto_instrument_traces --- sentry-ruby/lib/sentry/configuration.rb | 13 ++++++++++--- sentry-ruby/lib/sentry/net/http.rb | 2 +- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 2 +- sentry-ruby/lib/sentry/redis.rb | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 473086818..9c6eebc41 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -211,8 +211,9 @@ class Configuration # @return [Boolean] attr_accessor :auto_session_tracking - # Automatically instrument supported libraries with transactions and spans. - attr_accessor :auto_instrument_traces + # The instrumenter to use, :sentry or :otel + # @return [Symbol] + attr_reader :instrumenter # these are not config options # @!visibility private @@ -240,6 +241,8 @@ class Configuration MODULE_SEPARATOR = "::".freeze SKIP_INSPECTION_ATTRIBUTES = [:@linecache, :@stacktrace_builder] + INSTRUMENTERS = [:sentry, :otel] + # Post initialization callbacks are called at the end of initialization process # allowing extending the configuration of sentry-ruby by multiple extensions @@post_initialization_callbacks = [] @@ -272,7 +275,7 @@ def initialize self.trusted_proxies = [] self.dsn = ENV['SENTRY_DSN'] self.server_name = server_name_from_env - self.auto_instrument_traces = true + self.instrumenter = :sentry self.before_send = nil self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT @@ -336,6 +339,10 @@ def environment=(environment) @environment = environment.to_s end + def instrumenter=(instrumenter) + @instrumenter = INSTRUMENTERS.include?(instrumenter) ? instrumenter : :sentry + end + def sending_allowed? @errors = [] diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index c664dd219..f83cee328 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -28,7 +28,7 @@ module HTTP def request(req, body = nil, &block) return super unless started? - if Sentry.configuration.auto_instrument_traces + if Sentry.configuration.instrumenter == :sentry sentry_span = start_sentry_span set_sentry_trace_header(req, sentry_span) else diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index 802e64d96..9c0f59567 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -24,7 +24,7 @@ def call(env) sentry_trace = env["HTTP_SENTRY_TRACE"] scope.set_trace(sentry_trace) - transaction = start_transaction(env, scope, sentry_trace) if Sentry.configuration.auto_instrument_traces + transaction = start_transaction(env, scope, sentry_trace) if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction begin diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb index 88e5a0d35..7bd0427dd 100644 --- a/sentry-ruby/lib/sentry/redis.rb +++ b/sentry-ruby/lib/sentry/redis.rb @@ -84,7 +84,7 @@ def logging(commands, &block) if defined?(::Redis::Client) Sentry.register_patch do |config| - return unless config.auto_instrument_traces + return unless config.instrumenter == :sentry patch = Sentry::Redis::Client Redis::Client.prepend(patch) unless Redis::Client.ancestors.include?(patch) From 6a05daf71e730dae6158e77bdffe46f8d4abe301 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Oct 2022 15:09:03 +0200 Subject: [PATCH 03/44] Revert scope.set_trace and rack changes; use rack_env instead --- .../lib/sentry/open_telemetry/span_processor.rb | 4 ++-- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 8 +++----- sentry-ruby/lib/sentry/scope.rb | 14 ++++++-------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 03df7b3cc..df96cf430 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -15,8 +15,8 @@ def on_start(otel_span, _parent_context) parent_sentry_span.start_child(op: otel_span.name) else options = { name: otel_span.name, op: otel_span.name } - transaction = Sentry::Transaction.from_sentry_trace(scope.trace, **options) if scope.trace - scope.set_trace(nil) + sentry_trace = scope.sentry_trace + transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace Sentry.start_transaction(transaction: transaction, **options) end diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index 9c0f59567..79f929d82 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -21,10 +21,7 @@ def call(env) scope.set_transaction_name(env["PATH_INFO"]) if env["PATH_INFO"] scope.set_rack_env(env) - sentry_trace = env["HTTP_SENTRY_TRACE"] - scope.set_trace(sentry_trace) - - transaction = start_transaction(env, scope, sentry_trace) if Sentry.configuration.instrumenter == :sentry + transaction = start_transaction(env, scope) if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction begin @@ -64,7 +61,8 @@ def capture_exception(exception, env) end end - def start_transaction(env, scope, sentry_trace) + def start_transaction(env, scope) + sentry_trace = scope.sentry_trace options = { name: scope.transaction_name, op: transaction_op } transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index 34ddb49d3..35aed50fd 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -7,7 +7,7 @@ module Sentry class Scope include ArgumentCheckingHelper - ATTRIBUTES = [:transaction_names, :contexts, :extra, :tags, :user, :level, :breadcrumbs, :fingerprint, :event_processors, :rack_env, :span, :session, :trace] + ATTRIBUTES = [:transaction_names, :contexts, :extra, :tags, :user, :level, :breadcrumbs, :fingerprint, :event_processors, :rack_env, :span, :session] attr_reader(*ATTRIBUTES) @@ -126,6 +126,11 @@ def set_rack_env(env) @rack_env = env end + def sentry_trace + return nil unless rack_env + rack_env["HTTP_SENTRY_TRACE"] + end + # Sets the scope's span attribute. # @param span [Span] # @return [Span] @@ -241,13 +246,6 @@ def add_event_processor(&block) @event_processors << block end - # Sets the scope's trace (the header value used by Sentry to propagate traces). - def set_trace(trace) - check_argument_type!(trace, String) unless trace.nil? - - @trace = trace - end - protected # for duplicating scopes internally From ff707e3083098200453706aa6c68d12502d104dc Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Oct 2022 15:17:50 +0200 Subject: [PATCH 04/44] Also add scope.baggage for consistency --- sentry-rails/lib/sentry/rails/action_cable.rb | 4 ++-- sentry-rails/lib/sentry/rails/capture_exceptions.rb | 4 ++-- sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 3 ++- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 2 +- sentry-ruby/lib/sentry/scope.rb | 9 +++++++++ 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index 397a5a755..599cc9aa7 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -30,8 +30,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"] + sentry_trace = scope.sentry_trace + baggage = scope.baggage options = { name: scope.transaction_name, source: scope.transaction_source, op: "rails.action_cable".freeze } transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb index a49eb51e5..c155fa40c 100644 --- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb +++ b/sentry-rails/lib/sentry/rails/capture_exceptions.rb @@ -31,8 +31,8 @@ def capture_exception(exception, env) end def start_transaction(env, scope) - sentry_trace = env["HTTP_SENTRY_TRACE"] - baggage = env["HTTP_BAGGAGE"] + sentry_trace = scope.sentry_trace + baggage = scope.baggage options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op } diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index df96cf430..4b3a99974 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -16,7 +16,8 @@ def on_start(otel_span, _parent_context) else options = { name: otel_span.name, op: otel_span.name } sentry_trace = scope.sentry_trace - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace + baggage = scope.baggage + transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace Sentry.start_transaction(transaction: transaction, **options) end diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index 7778348a4..d4f98c8da 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -63,7 +63,7 @@ def capture_exception(exception, env) def start_transaction(env, scope) sentry_trace = scope.sentry_trace - baggage = env["HTTP_BAGGAGE"] + baggage = scope.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 Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index e13f84047..06169d0e0 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -143,11 +143,20 @@ def set_rack_env(env) @rack_env = env end + # Get the sentry-trace header on the rack env + # @return [String, nil] def sentry_trace return nil unless rack_env rack_env["HTTP_SENTRY_TRACE"] end + # Get the baggage header on the rack env + # @return [String, nil] + def baggage + return nil unless rack_env + rack_env["HTTP_BAGGAGE"] + end + # Sets the scope's span attribute. # @param span [Span] # @return [Span] From 781793b87fcd960933fb51adc1dd2d46708bc0dd Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 10 Oct 2022 15:24:32 +0200 Subject: [PATCH 05/44] Make net/http conditional cleaner --- sentry-ruby/lib/sentry/net/http.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 0f7147ca4..a08f62aed 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -28,12 +28,8 @@ module HTTP def request(req, body = nil, &block) return super unless started? - if Sentry.configuration.instrumenter == :sentry - sentry_span = start_sentry_span - set_sentry_trace_header(req, sentry_span) - else - set_sentry_trace_header(req, Sentry.get_current_scope.get_span) - end + sentry_span = start_sentry_span + set_sentry_trace_header(req, sentry_span) super.tap do |res| record_sentry_breadcrumb(req, res) @@ -44,14 +40,16 @@ def request(req, body = nil, &block) private def set_sentry_trace_header(req, sentry_span) - return unless sentry_span + # TODO-neel maybe some otel span validation + sentry_or_otel_span = sentry_span || Sentry.get_current_scope.get_span + return unless sentry_or_otel_span client = Sentry.get_current_client - trace = client.generate_sentry_trace(sentry_span) + trace = client.generate_sentry_trace(sentry_or_otel_span) req[SENTRY_TRACE_HEADER_NAME] = trace if trace - baggage = client.generate_baggage(sentry_span) + baggage = client.generate_baggage(sentry_or_otel_span) req[BAGGAGE_HEADER_NAME] = baggage if baggage && !baggage.empty? end @@ -83,9 +81,12 @@ def record_sentry_span(req, res, sentry_span) end def start_sentry_span - return unless Sentry.initialized? && span = Sentry.get_current_scope.get_span + return unless Sentry.initialized? + return unless Sentry.configuration.instrumenter == :sentry return if from_sentry_sdk? - return if span.sampled == false + + span = Sentry.get_current_scope.get_span + return unless span&.sampled span.start_child(op: OP_NAME, start_timestamp: Sentry.utc_now.to_f) end From cb6b91f2be54e5c5ab493caf77c53994310a4d3b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 17 Oct 2022 15:52:42 +0200 Subject: [PATCH 06/44] Add instrumenter checks to all other start_transaction's --- sentry-delayed_job/lib/sentry/delayed_job/plugin.rb | 2 +- sentry-rails/lib/sentry/rails/action_cable.rb | 2 +- sentry-rails/lib/sentry/rails/active_job.rb | 2 +- sentry-resque/lib/sentry/resque.rb | 2 +- sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 1 + sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb | 2 +- 6 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb index 9754a3bdb..f08a9e860 100644 --- a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb +++ b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb @@ -19,7 +19,7 @@ class Plugin < ::Delayed::Plugin scope.set_contexts(**contexts) scope.set_tags("delayed_job.queue" => job.queue, "delayed_job.id" => job.id.to_s) - transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "delayed_job") + transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "delayed_job") if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction begin diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index 599cc9aa7..1be4a74ba 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -14,7 +14,7 @@ def capture(connection, transaction_name:, extra_context: nil, &block) scope.set_rack_env(env) scope.set_context("action_cable", extra_context) if extra_context scope.set_transaction_name(transaction_name, source: :view) - transaction = start_transaction(env, scope) + transaction = start_transaction(env, scope) if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction begin diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 9e732183f..781bd53ba 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -22,7 +22,7 @@ def record(job, &block) begin scope.set_transaction_name(job.class.name, source: :task) transaction = - if job.is_a?(::Sentry::SendEventJob) + if job.is_a?(::Sentry::SendEventJob) || Sentry.configuration.instrumenter != :sentry nil else Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "active_job") diff --git a/sentry-resque/lib/sentry/resque.rb b/sentry-resque/lib/sentry/resque.rb index a2c361cdd..83e13935d 100644 --- a/sentry-resque/lib/sentry/resque.rb +++ b/sentry-resque/lib/sentry/resque.rb @@ -25,7 +25,7 @@ def record(queue, worker, payload, &block) name = contexts.dig(:"Active-Job", :job_class) || contexts.dig(:"Resque", :job_class) scope.set_transaction_name(name, source: :task) - transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "resque") + transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "resque") if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction yield diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 4b3a99974..731b3db8f 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -56,6 +56,7 @@ def shutdown(timeout: nil) private + # TODO-neel what to do about this def from_sentry_sdk?(otel_span) caller.any? { |line| line =~ /lib[\\\/]sentry[\\\/]background_worker.rb/ } end diff --git a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb index 0619eb8a5..49cff7fe5 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb @@ -17,7 +17,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["sentry_trace"]) if Sentry.configuration.instrumenter == :sentry scope.set_span(transaction) if transaction begin From e7b28026af41d8bc2593c0d892606cbf74d1f58d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 17 Oct 2022 15:57:29 +0200 Subject: [PATCH 07/44] Missing init guard in net/http --- sentry-ruby/lib/sentry/net/http.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index a08f62aed..b37a6c5f4 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -40,6 +40,8 @@ def request(req, body = nil, &block) private def set_sentry_trace_header(req, sentry_span) + return unless Sentry.initialized? + # TODO-neel maybe some otel span validation sentry_or_otel_span = sentry_span || Sentry.get_current_scope.get_span return unless sentry_or_otel_span From 84e7949914c407d7c60b6865ac3ed488eec528e5 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 18 Oct 2022 14:29:19 +0200 Subject: [PATCH 08/44] DWIP --- sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 731b3db8f..8489f17fc 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -1,23 +1,28 @@ module Sentry module OpenTelemetry class SpanProcessor + include LoggingHelper + def initialize @otel_span_map = {} end def on_start(otel_span, _parent_context) + return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel return if from_sentry_sdk?(otel_span) scope = Sentry.get_current_scope parent_sentry_span = scope.get_span sentry_span = if parent_sentry_span + Sentry.configuration.logger.info("Continuing otel span on parent #{parent_sentry_span.name}") parent_sentry_span.start_child(op: otel_span.name) else options = { name: otel_span.name, op: otel_span.name } sentry_trace = scope.sentry_trace baggage = scope.baggage transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") Sentry.start_transaction(transaction: transaction, **options) end @@ -26,6 +31,8 @@ def on_start(otel_span, _parent_context) end def on_finish(otel_span) + return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel + current_scope = Sentry.get_current_scope sentry_span, parent_span = @otel_span_map.delete(otel_span.context.span_id) return unless sentry_span From 0b21d4d3150ab388456aaa3599ae0a86ba63e634 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 20 Oct 2022 14:05:06 +0200 Subject: [PATCH 09/44] Revert instrumenter checks at call sites --- sentry-delayed_job/lib/sentry/delayed_job/plugin.rb | 2 +- sentry-rails/lib/sentry/rails/action_cable.rb | 2 +- sentry-rails/lib/sentry/rails/active_job.rb | 2 +- sentry-resque/lib/sentry/resque.rb | 2 +- sentry-ruby/lib/sentry/hub.rb | 3 ++- sentry-ruby/lib/sentry/net/http.rb | 1 - sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 2 -- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 2 +- sentry-ruby/lib/sentry/redis.rb | 2 -- sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb | 2 +- 10 files changed, 8 insertions(+), 12 deletions(-) diff --git a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb index f08a9e860..9754a3bdb 100644 --- a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb +++ b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb @@ -19,7 +19,7 @@ class Plugin < ::Delayed::Plugin scope.set_contexts(**contexts) scope.set_tags("delayed_job.queue" => job.queue, "delayed_job.id" => job.id.to_s) - transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "delayed_job") if Sentry.configuration.instrumenter == :sentry + transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "delayed_job") scope.set_span(transaction) if transaction begin diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index 1be4a74ba..599cc9aa7 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -14,7 +14,7 @@ def capture(connection, transaction_name:, extra_context: nil, &block) scope.set_rack_env(env) scope.set_context("action_cable", extra_context) if extra_context scope.set_transaction_name(transaction_name, source: :view) - transaction = start_transaction(env, scope) if Sentry.configuration.instrumenter == :sentry + transaction = start_transaction(env, scope) scope.set_span(transaction) if transaction begin diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 781bd53ba..9e732183f 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -22,7 +22,7 @@ def record(job, &block) begin scope.set_transaction_name(job.class.name, source: :task) transaction = - if job.is_a?(::Sentry::SendEventJob) || Sentry.configuration.instrumenter != :sentry + if job.is_a?(::Sentry::SendEventJob) nil else Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "active_job") diff --git a/sentry-resque/lib/sentry/resque.rb b/sentry-resque/lib/sentry/resque.rb index 83e13935d..a2c361cdd 100644 --- a/sentry-resque/lib/sentry/resque.rb +++ b/sentry-resque/lib/sentry/resque.rb @@ -25,7 +25,7 @@ def record(queue, worker, payload, &block) name = contexts.dig(:"Active-Job", :job_class) || contexts.dig(:"Resque", :job_class) scope.set_transaction_name(name, source: :task) - transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "resque") if Sentry.configuration.instrumenter == :sentry + transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "resque") scope.set_span(transaction) if transaction yield diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index 582855a30..f7d5563f5 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -76,8 +76,9 @@ def pop_scope @stack.pop end - def start_transaction(transaction: nil, custom_sampling_context: {}, **options) + def start_transaction(transaction: nil, custom_sampling_context: {}, instrumenter: :sentry, **options) return unless configuration.tracing_enabled? + return unless instrumenter == configuration.instrumenter transaction ||= Transaction.new(**options.merge(hub: self)) diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index b37a6c5f4..7a526f589 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -84,7 +84,6 @@ def record_sentry_span(req, res, sentry_span) def start_sentry_span return unless Sentry.initialized? - return unless Sentry.configuration.instrumenter == :sentry return if from_sentry_sdk? span = Sentry.get_current_scope.get_span diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 8489f17fc..0f966b039 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -1,8 +1,6 @@ module Sentry module OpenTelemetry class SpanProcessor - include LoggingHelper - def initialize @otel_span_map = {} end diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index d4f98c8da..bb567a800 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -21,7 +21,7 @@ def call(env) scope.set_transaction_name(env["PATH_INFO"], source: :url) if env["PATH_INFO"] scope.set_rack_env(env) - transaction = start_transaction(env, scope) if Sentry.configuration.instrumenter == :sentry + transaction = start_transaction(env, scope) scope.set_span(transaction) if transaction begin diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb index 028dcd8a5..3a6d42217 100644 --- a/sentry-ruby/lib/sentry/redis.rb +++ b/sentry-ruby/lib/sentry/redis.rb @@ -89,8 +89,6 @@ def logging(commands, &block) if defined?(::Redis::Client) Sentry.register_patch do |config| - return unless config.instrumenter == :sentry - patch = Sentry::Redis::Client Redis::Client.prepend(patch) unless Redis::Client.ancestors.include?(patch) end diff --git a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb index 49cff7fe5..0619eb8a5 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb @@ -17,7 +17,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"]) if Sentry.configuration.instrumenter == :sentry + transaction = start_transaction(scope, job["sentry_trace"]) scope.set_span(transaction) if transaction begin From 9f8ce8db1e8db05e86a20cc1d3a3f3efcfae27d3 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 12:24:30 +0200 Subject: [PATCH 10/44] Guard with_child_span with instrumenter check --- sentry-ruby/lib/sentry/hub.rb | 4 +++- sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index f7d5563f5..8e58c60fc 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -93,7 +93,9 @@ def start_transaction(transaction: nil, custom_sampling_context: {}, instrumente transaction end - def with_child_span(**attributes, &block) + def with_child_span(instrumenter: :sentry, **attributes, &block) + return yield(nil) unless instrumenter == configuration.instrumenter + current_span = current_scope.get_span return yield(nil) unless current_span diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 0f966b039..f6908157e 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -21,7 +21,7 @@ def on_start(otel_span, _parent_context) baggage = scope.baggage transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") - Sentry.start_transaction(transaction: transaction, **options) + Sentry.start_transaction(transaction: transaction, instrumenter: :otel, **options) end scope.set_span(sentry_span) From c7ff027ce19b9dc53de2e949ab01bbacd037389a Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 12:30:44 +0200 Subject: [PATCH 11/44] Make with_child_span blocks safe if span is nil --- sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb | 2 +- .../lib/sentry/rails/tracing/action_controller_subscriber.rb | 4 ++-- .../lib/sentry/rails/tracing/active_record_subscriber.rb | 2 +- .../lib/sentry/rails/tracing/active_storage_subscriber.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb index 17438d191..d79ae207b 100644 --- a/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb @@ -43,7 +43,7 @@ def record_on_current_span(duration:, **options) Sentry.with_child_span(**options) do |child_span| # duration in ActiveSupport is computed in millisecond # so we need to covert it as second before calculating the timestamp - child_span.set_timestamp(child_span.start_timestamp + duration / 1000) + child_span&.set_timestamp(child_span.start_timestamp + duration / 1000) yield(child_span) if block_given? end end diff --git a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb index 3b221a381..b9d827bc1 100644 --- a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb @@ -22,8 +22,8 @@ def self.subscribe! ) do |span| payload = payload.dup cleanup_data(payload) - span.set_data(:payload, payload) - span.set_http_status(payload[:status]) + span&.set_data(:payload, payload) + span&.set_http_status(payload[:status]) end end end diff --git a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb index c19e7dad4..e84901773 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb @@ -13,7 +13,7 @@ def self.subscribe! next if EXCLUDED_EVENTS.include? payload[:name] record_on_current_span(op: SPAN_PREFIX + event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:sql], duration: duration) do |span| - span.set_data(:connection_id, payload[:connection_id]) + span&.set_data(:connection_id, payload[:connection_id]) end end end diff --git a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb index a28217b86..d7b005ed0 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb @@ -23,7 +23,7 @@ def self.subscribe! subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload| record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:service], duration: duration) do |span| payload.each do |key, value| - span.set_data(key, value) unless key == START_TIMESTAMP_NAME + span&.set_data(key, value) unless key == START_TIMESTAMP_NAME end end end From e3199ca2ad3cc2d18edb9fe5b5e2522044ef9253 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 14:09:31 +0200 Subject: [PATCH 12/44] WIP --- sentry-ruby/lib/sentry/open_telemetry/span_processor.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index f6908157e..9f317b7a5 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -13,10 +13,10 @@ def on_start(otel_span, _parent_context) parent_sentry_span = scope.get_span sentry_span = if parent_sentry_span - Sentry.configuration.logger.info("Continuing otel span on parent #{parent_sentry_span.name}") - parent_sentry_span.start_child(op: otel_span.name) + Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.name}") + parent_sentry_span.start_child(description: otel_span.name) else - options = { name: otel_span.name, op: otel_span.name } + options = { name: otel_span.name } sentry_trace = scope.sentry_trace baggage = scope.baggage transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace @@ -35,6 +35,7 @@ def on_finish(otel_span) sentry_span, parent_span = @otel_span_map.delete(otel_span.context.span_id) return unless sentry_span + # TODO-neel ops sentry_span.set_op(otel_span.name) current_scope.set_transaction_name(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) From f39713addf0ad7db1ed28ed755e05c1c2ec6ca06 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 15:29:11 +0200 Subject: [PATCH 13/44] Set contexts.otel as per spec on transaction --- .../sentry/open_telemetry/span_processor.rb | 42 ++++++++++++++++--- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 9f317b7a5..f662dc47b 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -37,12 +37,16 @@ def on_finish(otel_span) # TODO-neel ops sentry_span.set_op(otel_span.name) - current_scope.set_transaction_name(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) - otel_span.attributes&.each do |key, value| - sentry_span.set_data(key, value) - if key == "db.statement" - sentry_span.set_description(value) + if sentry_span.is_a?(Sentry::Transaction) + current_scope.set_transaction_name(otel_span.name) + current_scope.set_context(:otel, otel_context_hash(otel_span)) + else + otel_span.attributes&.each do |key, value| + sentry_span.set_data(key, value) + if key == "db.statement" + sentry_span.set_description(value) + end end end @@ -66,6 +70,34 @@ def shutdown(timeout: nil) def from_sentry_sdk?(otel_span) caller.any? { |line| line =~ /lib[\\\/]sentry[\\\/]background_worker.rb/ } end + + def otel_context_hash(otel_span) + otel_context = {} + otel_context[:attributes] = otel_span.attributes unless otel_span.attributes.empty? + + resource_attributes = otel_span.resource.attribute_enumerator.to_h + + service = {} + service[:name] = resource_attributes.delete("service.name") + service[:namespace] = resource_attributes.delete("service.namespace") + service[:instance_id] = resource_attributes.delete("service.instance.id") + service[:version] = resource_attributes.delete("service.version") + service.compact! + + otel_context[:service] = service unless service.empty? + + otel_sdk = {} + otel_sdk[:name] = resource_attributes.delete("telemetry.sdk.name") + otel_sdk[:language] = resource_attributes.delete("telemetry.sdk.language") + otel_sdk[:version] = resource_attributes.delete("telemetry.sdk.version") + otel_sdk[:auto_version] = resource_attributes.delete("telemetry.auto.version") + otel_sdk.compact! + + otel_context[:otel_sdk] = otel_sdk unless otel_sdk.empty? + + # remaining resource_attributes just go to the main hash + otel_context.merge!(resource_attributes) + end end end end From 26127e5244fcbf377152c0054fd56c02dacd2287 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 16:39:47 +0200 Subject: [PATCH 14/44] Replace caller with checking net.peer.name for HTTP spans --- .../sentry/open_telemetry/span_processor.rb | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index f662dc47b..9345fa088 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -1,6 +1,10 @@ module Sentry module OpenTelemetry class SpanProcessor + + ATTRIBUTE_NET_PEER_NAME = "net.peer.name" + ATTRIBUTE_DB_STATEMENT = "db.statement" + def initialize @otel_span_map = {} end @@ -44,12 +48,13 @@ def on_finish(otel_span) else otel_span.attributes&.each do |key, value| sentry_span.set_data(key, value) - if key == "db.statement" + if key == ATTRIBUTE_DB_STATEMENT sentry_span.set_description(value) end end end + Sentry.configuration.logger.info("Finishing sentry_span #{sentry_span.op}") sentry_span.finish current_scope.set_span(parent_span) if parent_span end @@ -66,9 +71,22 @@ def shutdown(timeout: nil) private - # TODO-neel what to do about this def from_sentry_sdk?(otel_span) - caller.any? { |line| line =~ /lib[\\\/]sentry[\\\/]background_worker.rb/ } + dsn = Sentry.configuration.dsn + return false unless dsn + + if otel_span.name.start_with?("HTTP") + # only check client requests, connects are sometimes internal + return false unless %i(client internal).include?(otel_span.kind) + + address = otel_span.attributes[ATTRIBUTE_NET_PEER_NAME] + + # if no address drop it, just noise + return true unless address + return true if dsn.host == address + end + + false end def otel_context_hash(otel_span) From d691b0f2c379df4b5bc7d7c748e7db543f237452 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 21 Oct 2022 16:50:54 +0200 Subject: [PATCH 15/44] Simplify contexts.otel --- .../sentry/open_telemetry/span_processor.rb | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index 9345fa088..fb7fbc596 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -94,27 +94,9 @@ def otel_context_hash(otel_span) otel_context[:attributes] = otel_span.attributes unless otel_span.attributes.empty? resource_attributes = otel_span.resource.attribute_enumerator.to_h + otel_context[:resource] = resource_attributes unless resource_attributes.empty? - service = {} - service[:name] = resource_attributes.delete("service.name") - service[:namespace] = resource_attributes.delete("service.namespace") - service[:instance_id] = resource_attributes.delete("service.instance.id") - service[:version] = resource_attributes.delete("service.version") - service.compact! - - otel_context[:service] = service unless service.empty? - - otel_sdk = {} - otel_sdk[:name] = resource_attributes.delete("telemetry.sdk.name") - otel_sdk[:language] = resource_attributes.delete("telemetry.sdk.language") - otel_sdk[:version] = resource_attributes.delete("telemetry.sdk.version") - otel_sdk[:auto_version] = resource_attributes.delete("telemetry.auto.version") - otel_sdk.compact! - - otel_context[:otel_sdk] = otel_sdk unless otel_sdk.empty? - - # remaining resource_attributes just go to the main hash - otel_context.merge!(resource_attributes) + otel_context end end end From 551809f50e908bc02d062db0ee1afe01bbe3a1e0 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 27 Oct 2022 14:51:28 +0200 Subject: [PATCH 16/44] Use trace_id/span_id/parent_span_id from otel --- .../sentry/open_telemetry/span_processor.rb | 35 ++++++++++++++++--- sentry-ruby/lib/sentry/span.rb | 3 +- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index fb7fbc596..fcf6742d7 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -5,6 +5,13 @@ class SpanProcessor ATTRIBUTE_NET_PEER_NAME = "net.peer.name" ATTRIBUTE_DB_STATEMENT = "db.statement" + # https://github.com/open-telemetry/opentelemetry-ruby/blob/18bfd391f2bda2c958d5d6935886c8cba61414dd/api/lib/opentelemetry/trace.rb#L18-L22 + # An invalid trace identifier, a 16-byte string with all zero bytes. + INVALID_TRACE_ID = ("\0" * 16).b + + # An invalid span identifier, an 8-byte string with all zero bytes. + INVALID_SPAN_ID = ("\0" * 8).b + def initialize @otel_span_map = {} end @@ -13,30 +20,40 @@ def on_start(otel_span, _parent_context) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel return if from_sentry_sdk?(otel_span) + span_id, trace_id, parent_span_id = get_trace_data(otel_span) + scope = Sentry.get_current_scope parent_sentry_span = scope.get_span sentry_span = if parent_sentry_span Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.name}") - parent_sentry_span.start_child(description: otel_span.name) + + parent_sentry_span.start_child(span_id: span_id, description: otel_span.name) else - options = { name: otel_span.name } + options = { + span_id: span_id, + trace_id: trace_id, + parent_span_id: parent_span_id, + name: otel_span.name + } + sentry_trace = scope.sentry_trace baggage = scope.baggage - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") + transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace Sentry.start_transaction(transaction: transaction, instrumenter: :otel, **options) end scope.set_span(sentry_span) - @otel_span_map[otel_span.context.span_id] = [sentry_span, parent_sentry_span] + @otel_span_map[span_id] = [sentry_span, parent_sentry_span] end def on_finish(otel_span) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel current_scope = Sentry.get_current_scope - sentry_span, parent_span = @otel_span_map.delete(otel_span.context.span_id) + sentry_span, parent_span = @otel_span_map.delete(otel_span.context.hex_span_id) return unless sentry_span # TODO-neel ops @@ -89,6 +106,14 @@ def from_sentry_sdk?(otel_span) false end + def get_trace_data(otel_span) + span_id = otel_span.context.hex_span_id + trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == INVALID_TRACE_ID + parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == INVALID_SPAN_ID + + [span_id, trace_id, parent_span_id] + end + def otel_context_hash(otel_span) otel_context = {} otel_context[:attributes] = otel_span.attributes unless otel_span.attributes.empty? diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 94c276b92..a2c3eeded 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -67,13 +67,14 @@ def initialize( op: nil, status: nil, trace_id: nil, + span_id: nil, parent_span_id: nil, sampled: nil, start_timestamp: nil, timestamp: nil ) @trace_id = trace_id || SecureRandom.uuid.delete("-") - @span_id = SecureRandom.hex(8) + @span_id = span_id || SecureRandom.hex(8) @parent_span_id = parent_span_id @sampled = sampled @start_timestamp = start_timestamp || Sentry.utc_now.to_f From 542a019935b43253881257d0b9692138b15f903f Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 27 Oct 2022 14:56:30 +0200 Subject: [PATCH 17/44] CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ed3fcfc1..cc2f2e205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ``` - Use `Sentry.with_child_span` in redis and net/http instead of `span.start_child` [#1920](https://github.com/getsentry/sentry-ruby/pull/1920) - This might change the nesting of some spans and make it more accurate +- Add OpenTelemetry `SpanProcessor` TODO-neel expand [#1876](https://github.com/getsentry/sentry-ruby/pull/1876) ### Bug Fixes From 0cd4b7b89b9b31850b5581ca31e8339ad6620e2c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 27 Oct 2022 15:14:44 +0200 Subject: [PATCH 18/44] Some cleanup --- .../sentry/open_telemetry/span_processor.rb | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index fcf6742d7..f4ae51d6f 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -21,6 +21,7 @@ def on_start(otel_span, _parent_context) return if from_sentry_sdk?(otel_span) span_id, trace_id, parent_span_id = get_trace_data(otel_span) + return unless span_id scope = Sentry.get_current_scope parent_sentry_span = scope.get_span @@ -30,18 +31,22 @@ def on_start(otel_span, _parent_context) parent_sentry_span.start_child(span_id: span_id, description: otel_span.name) else - options = { + continue_options = { span_id: span_id, + name: otel_span.name + } + + options = { trace_id: trace_id, parent_span_id: parent_span_id, - name: otel_span.name + **continue_options } sentry_trace = scope.sentry_trace baggage = scope.baggage Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace + transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **continue_options) if sentry_trace Sentry.start_transaction(transaction: transaction, instrumenter: :otel, **options) end @@ -52,10 +57,14 @@ def on_start(otel_span, _parent_context) def on_finish(otel_span) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel - current_scope = Sentry.get_current_scope - sentry_span, parent_span = @otel_span_map.delete(otel_span.context.hex_span_id) + span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == INVALID_SPAN_ID + return unless span_id + + sentry_span, parent_span = @otel_span_map.delete(span_id) return unless sentry_span + current_scope = Sentry.get_current_scope + # TODO-neel ops sentry_span.set_op(otel_span.name) @@ -107,7 +116,7 @@ def from_sentry_sdk?(otel_span) end def get_trace_data(otel_span) - span_id = otel_span.context.hex_span_id + span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == INVALID_SPAN_ID trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == INVALID_TRACE_ID parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == INVALID_SPAN_ID From c8f690cabaf37c97767a9e6558366570f369514f Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 28 Oct 2022 14:33:57 +0200 Subject: [PATCH 19/44] Use otel timestamps --- .../lib/sentry/open_telemetry/span_processor.rb | 11 ++++++++--- sentry-ruby/lib/sentry/span.rb | 4 ++-- sentry-ruby/lib/sentry/transaction.rb | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index f4ae51d6f..d6abd8a56 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -29,11 +29,16 @@ def on_start(otel_span, _parent_context) sentry_span = if parent_sentry_span Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.name}") - parent_sentry_span.start_child(span_id: span_id, description: otel_span.name) + parent_sentry_span.start_child( + span_id: span_id, + description: otel_span.name, + start_timestamp: otel_span.start_timestamp / 1e9 + ) else continue_options = { span_id: span_id, - name: otel_span.name + name: otel_span.name, + start_timestamp: otel_span.start_timestamp / 1e9 } options = { @@ -81,7 +86,7 @@ def on_finish(otel_span) end Sentry.configuration.logger.info("Finishing sentry_span #{sentry_span.op}") - sentry_span.finish + sentry_span.finish(end_timestamp: otel_span.end_timestamp / 1e9) current_scope.set_span(parent_span) if parent_span end diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 9ca450e16..f7d16cb83 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -90,11 +90,11 @@ def initialize( # Finishes the span by adding a timestamp. # @return [self] - def finish + def finish(end_timestamp: nil) # already finished return if @timestamp - @timestamp = Sentry.utc_now.to_f + @timestamp = end_timestamp || Sentry.utc_now.to_f self end diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index 2a5ab0f0c..e237b5a9b 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -210,7 +210,7 @@ def set_initial_sample_decision(sampling_context:) # Finishes the transaction's recording and send it to Sentry. # @param hub [Hub] the hub that'll send this transaction. (Deprecated) # @return [TransactionEvent] - def finish(hub: nil) + def finish(hub: nil, end_timestamp: nil) if hub log_warn( <<~MSG @@ -222,7 +222,7 @@ def finish(hub: nil) hub ||= @hub - super() # Span#finish doesn't take arguments + super(end_timestamp: end_timestamp) if @name.nil? @name = UNLABELD_NAME From 904b1380b041a3f4119c66a9120ca88f338f5a0d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 31 Oct 2022 15:09:13 +0100 Subject: [PATCH 20/44] update_span_with_otel_data http --- .../sentry/open_telemetry/span_processor.rb | 38 +++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index d6abd8a56..f1abf5384 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -2,6 +2,9 @@ module Sentry module OpenTelemetry class SpanProcessor + ATTRIBUTE_HTTP_METHOD = "http.method" + ATTRIBUTE_HTTP_TARGET = "http.target" + ATTRIBUTE_HTTP_STATUS_CODE = "http.status_code" ATTRIBUTE_NET_PEER_NAME = "net.peer.name" ATTRIBUTE_DB_STATEMENT = "db.statement" @@ -70,19 +73,13 @@ def on_finish(otel_span) current_scope = Sentry.get_current_scope - # TODO-neel ops sentry_span.set_op(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) current_scope.set_transaction_name(otel_span.name) current_scope.set_context(:otel, otel_context_hash(otel_span)) else - otel_span.attributes&.each do |key, value| - sentry_span.set_data(key, value) - if key == ATTRIBUTE_DB_STATEMENT - sentry_span.set_description(value) - end - end + update_span_with_otel_data(sentry_span, otel_span) end Sentry.configuration.logger.info("Finishing sentry_span #{sentry_span.op}") @@ -137,6 +134,33 @@ def otel_context_hash(otel_span) otel_context end + + def update_span_with_otel_data(sentry_span, otel_span) + otel_span.attributes&.each { |k, v| sentry_span.set_data(k, v) } + + op = otel_span.name + description = otel_span.name + + if (http_method = otel_span.attributes[ATTRIBUTE_HTTP_METHOD]) + op = "http.#{otel_span.kind}" + description = http_method + + peer_name = otel_span.attributes[ATTRIBUTE_NET_PEER_NAME] + description += " #{peer_name}" if peer_name + + target = otel_span.attributes[ATTRIBUTE_HTTP_TARGET] + description += target if target + + status_code = otel_span.attributes[ATTRIBUTE_HTTP_STATUS_CODE] + sentry_span.set_http_status(status_code) if status_code + end + + # if key == ATTRIBUTE_DB_STATEMENT + # sentry_span.set_description(value) + # end + sentry_span.set_op(op) + sentry_span.set_description(description) + end end end end From 6f22b695f9f78de8d9502e4a7c9968d14836d61e Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 3 Nov 2022 15:55:01 +0100 Subject: [PATCH 21/44] Add db statement as description --- .../lib/sentry/open_telemetry/span_processor.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb index f1abf5384..8daa1d9f8 100644 --- a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb +++ b/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb @@ -6,6 +6,7 @@ class SpanProcessor ATTRIBUTE_HTTP_TARGET = "http.target" ATTRIBUTE_HTTP_STATUS_CODE = "http.status_code" ATTRIBUTE_NET_PEER_NAME = "net.peer.name" + ATTRIBUTE_DB_SYSTEM = "db.system" ATTRIBUTE_DB_STATEMENT = "db.statement" # https://github.com/open-telemetry/opentelemetry-ruby/blob/18bfd391f2bda2c958d5d6935886c8cba61414dd/api/lib/opentelemetry/trace.rb#L18-L22 @@ -30,7 +31,7 @@ def on_start(otel_span, _parent_context) parent_sentry_span = scope.get_span sentry_span = if parent_sentry_span - Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.name}") + Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.op}") parent_sentry_span.start_child( span_id: span_id, @@ -153,11 +154,13 @@ def update_span_with_otel_data(sentry_span, otel_span) status_code = otel_span.attributes[ATTRIBUTE_HTTP_STATUS_CODE] sentry_span.set_http_status(status_code) if status_code + elsif otel_span.attributes[ATTRIBUTE_DB_SYSTEM] + op = "db" + + statement = otel_span.attributes[ATTRIBUTE_DB_STATEMENT] + description = statement if statement end - # if key == ATTRIBUTE_DB_STATEMENT - # sentry_span.set_description(value) - # end sentry_span.set_op(op) sentry_span.set_description(description) end From fcb843b600689b06eaa378a075a556874772e58b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Nov 2022 14:24:14 +0100 Subject: [PATCH 22/44] Add sentry-opentelemetry gem --- sentry-opentelemetry/.gitignore | 11 +++ sentry-opentelemetry/.rspec | 3 + sentry-opentelemetry/CODE_OF_CONDUCT.md | 74 +++++++++++++++++++ sentry-opentelemetry/Gemfile | 23 ++++++ sentry-opentelemetry/LICENSE.txt | 21 ++++++ sentry-opentelemetry/Makefile | 3 + sentry-opentelemetry/README.md | 68 +++++++++++++++++ sentry-opentelemetry/Rakefile | 8 ++ sentry-opentelemetry/bin/console | 15 ++++ sentry-opentelemetry/bin/setup | 8 ++ .../lib/sentry-opentelemetry.rb | 2 + .../sentry/opentelemetry}/span_processor.rb | 0 .../lib/sentry/opentelemetry/version.rb | 7 ++ .../sentry-opentelemetry.gemspec | 29 ++++++++ sentry-opentelemetry/spec/spec_helper.rb | 34 +++++++++ sentry-ruby/lib/sentry-ruby.rb | 1 - 16 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 sentry-opentelemetry/.gitignore create mode 100644 sentry-opentelemetry/.rspec create mode 100644 sentry-opentelemetry/CODE_OF_CONDUCT.md create mode 100644 sentry-opentelemetry/Gemfile create mode 100644 sentry-opentelemetry/LICENSE.txt create mode 100644 sentry-opentelemetry/Makefile create mode 100644 sentry-opentelemetry/README.md create mode 100644 sentry-opentelemetry/Rakefile create mode 100755 sentry-opentelemetry/bin/console create mode 100755 sentry-opentelemetry/bin/setup create mode 100644 sentry-opentelemetry/lib/sentry-opentelemetry.rb rename {sentry-ruby/lib/sentry/open_telemetry => sentry-opentelemetry/lib/sentry/opentelemetry}/span_processor.rb (100%) create mode 100644 sentry-opentelemetry/lib/sentry/opentelemetry/version.rb create mode 100644 sentry-opentelemetry/sentry-opentelemetry.gemspec create mode 100644 sentry-opentelemetry/spec/spec_helper.rb diff --git a/sentry-opentelemetry/.gitignore b/sentry-opentelemetry/.gitignore new file mode 100644 index 000000000..b04a8c840 --- /dev/null +++ b/sentry-opentelemetry/.gitignore @@ -0,0 +1,11 @@ +/.bundle/ +/.yardoc +/_yardoc/ +/coverage/ +/doc/ +/pkg/ +/spec/reports/ +/tmp/ + +# rspec failure tracking +.rspec_status diff --git a/sentry-opentelemetry/.rspec b/sentry-opentelemetry/.rspec new file mode 100644 index 000000000..34c5164d9 --- /dev/null +++ b/sentry-opentelemetry/.rspec @@ -0,0 +1,3 @@ +--format documentation +--color +--require spec_helper diff --git a/sentry-opentelemetry/CODE_OF_CONDUCT.md b/sentry-opentelemetry/CODE_OF_CONDUCT.md new file mode 100644 index 000000000..3c12de66b --- /dev/null +++ b/sentry-opentelemetry/CODE_OF_CONDUCT.md @@ -0,0 +1,74 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +In the interest of fostering an open and welcoming environment, we as +contributors and maintainers pledge to making participation in our project and +our community a harassment-free experience for everyone, regardless of age, body +size, disability, ethnicity, gender identity and expression, level of experience, +nationality, personal appearance, race, religion, or sexual identity and +orientation. + +## Our Standards + +Examples of behavior that contributes to creating a positive environment +include: + +* Using welcoming and inclusive language +* Being respectful of differing viewpoints and experiences +* Gracefully accepting constructive criticism +* Focusing on what is best for the community +* Showing empathy towards other community members + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery and unwelcome sexual attention or +advances +* Trolling, insulting/derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or electronic + address, without explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Our Responsibilities + +Project maintainers are responsible for clarifying the standards of acceptable +behavior and are expected to take appropriate and fair corrective action in +response to any instances of unacceptable behavior. + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct, or to ban temporarily or +permanently any contributor for other behaviors that they deem inappropriate, +threatening, offensive, or harmful. + +## Scope + +This Code of Conduct applies both within project spaces and in public spaces +when an individual is representing the project or its community. Examples of +representing a project or community include using an official project e-mail +address, posting via an official social media account, or acting as an appointed +representative at an online or offline event. Representation of a project may be +further defined and clarified by project maintainers. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting the project team at stan001212@gmail.com. All +complaints will be reviewed and investigated and will result in a response that +is deemed necessary and appropriate to the circumstances. The project team is +obligated to maintain confidentiality with regard to the reporter of an incident. +Further details of specific enforcement policies may be posted separately. + +Project maintainers who do not follow or enforce the Code of Conduct in good +faith may face temporary or permanent repercussions as determined by other +members of the project's leadership. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, +available at [https://contributor-covenant.org/version/1/4][version] + +[homepage]: https://contributor-covenant.org +[version]: https://contributor-covenant.org/version/1/4/ diff --git a/sentry-opentelemetry/Gemfile b/sentry-opentelemetry/Gemfile new file mode 100644 index 000000000..264539e7f --- /dev/null +++ b/sentry-opentelemetry/Gemfile @@ -0,0 +1,23 @@ +source "https://rubygems.org" +git_source(:github) { |name| "https://github.com/#{name}.git" } + +# Specify your gem's dependencies in sentry-ruby.gemspec +gemspec + +gem "rake", "~> 12.0" +gem "rspec", "~> 3.0" +gem 'simplecov' +gem "simplecov-cobertura", "~> 1.4" +gem "rexml" + +# TODO-neel CI +# opentelemetry_version = ENV["OPENTELEMETRY_VERSION"] +# opentelemetry_version = "1.2.0" if opentelemetry_version.nil? +# gem "opentelemetry-sdk" +# gem "opentelemtry-instrumentation-all" + +gem "sentry-ruby", path: "../sentry-ruby" + +gem "object_tracer" +gem "debug", github: "ruby/debug", platform: :ruby if RUBY_VERSION.to_f >= 2.6 +gem "pry" diff --git a/sentry-opentelemetry/LICENSE.txt b/sentry-opentelemetry/LICENSE.txt new file mode 100644 index 000000000..a53c2869c --- /dev/null +++ b/sentry-opentelemetry/LICENSE.txt @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2020 Sentry + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/sentry-opentelemetry/Makefile b/sentry-opentelemetry/Makefile new file mode 100644 index 000000000..5ce0336fa --- /dev/null +++ b/sentry-opentelemetry/Makefile @@ -0,0 +1,3 @@ +build: + bundle install + gem build sentry-opentelemetry.gemspec diff --git a/sentry-opentelemetry/README.md b/sentry-opentelemetry/README.md new file mode 100644 index 000000000..f4bf46b18 --- /dev/null +++ b/sentry-opentelemetry/README.md @@ -0,0 +1,68 @@ +

+ + + +
+

+ +# sentry-opentelemetry, the OpenTelemetry integration for Sentry's Ruby client + +--- + + +[![Gem Version](https://img.shields.io/gem/v/sentry-opentelemetry.svg)](https://rubygems.org/gems/sentry-opentelemetry) +![Build Status](https://github.com/getsentry/sentry-ruby/workflows/sentry-opentelemetry%20Test/badge.svg) +[![Coverage Status](https://img.shields.io/codecov/c/github/getsentry/sentry-ruby/master?logo=codecov)](https://codecov.io/gh/getsentry/sentry-ruby/branch/master) +[![Gem](https://img.shields.io/gem/dt/sentry-opentelemetry.svg)](https://rubygems.org/gems/sentry-opentelemetry/) +[![SemVer](https://api.dependabot.com/badges/compatibility_score?dependency-name=sentry-opentelemetry&package-manager=bundler&version-scheme=semver)](https://dependabot.com/compatibility-score.html?dependency-name=sentry-opentelemetry&package-manager=bundler&version-scheme=semver) + + +[Documentation](https://docs.sentry.io/platforms/ruby/guides/opentelemetry/) | [Bug Tracker](https://github.com/getsentry/sentry-ruby/issues) | [Forum](https://forum.sentry.io/) | IRC: irc.freenode.net, #sentry + +The official Ruby-language client and integration layer for the [Sentry](https://github.com/getsentry/sentry) error reporting API. + + +## Getting Started + +### Install + +```ruby +gem "sentry-ruby" +gem "sentry-rails" +gem "sentry-opentelemetry" + +gem "opentelemetry-sdk" +gem "opentelemetry-instrumentation-all" +``` + +### Configuration + +First, make sure to initialize Sentry before OpenTelemetry by prefixing your initializers and set the `instrumenter` to `:otel`. +```ruby +# config/initializers/01_sentry.rb + +Sentry.init do |config| + config.dsn = "MY_DSN" + config.traces_sample_rate = 1.0 + config.instrumenter = :otel +end +``` + +This will disable all Sentry instrumentation and rely on the chosen OpenTelemetry tracers for creating spans. + +Next, configure OpenTelemetry as per your needs and hook in the Sentry span processor and propagator(?? TODO-neel). Note that you need to re-order the middlewares (?? TODO-neel) for Sentry to pick spans up correctly. + +```ruby +# config/initializers/02_otel.rb + +OpenTelemetry::SDK.configure do |c| + c.use_all + c.add_span_processor(Sentry::OpenTelemetry::SpanProcessor.new) +end + +Rails.application.config.middleware.move_after( + Sentry::Rails::CaptureExceptions, + OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware +) +``` + diff --git a/sentry-opentelemetry/Rakefile b/sentry-opentelemetry/Rakefile new file mode 100644 index 000000000..1417dbd2d --- /dev/null +++ b/sentry-opentelemetry/Rakefile @@ -0,0 +1,8 @@ +require "bundler/gem_tasks" +require "rspec/core/rake_task" + +RSpec::Core::RakeTask.new(:spec).tap do |task| + task.rspec_opts = "--order rand" +end + +task :default => :spec diff --git a/sentry-opentelemetry/bin/console b/sentry-opentelemetry/bin/console new file mode 100755 index 000000000..c58bfc8b0 --- /dev/null +++ b/sentry-opentelemetry/bin/console @@ -0,0 +1,15 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require "bundler/setup" +require "sentry/opentelemetry" + +# You can add fixtures and/or initialization code here to make experimenting +# with your gem easier. You can also use a different console, if you like. + +# (If you use this, don't forget to add pry to your Gemfile!) +# require "pry" +# Pry.start + +require "irb" +IRB.start(__FILE__) diff --git a/sentry-opentelemetry/bin/setup b/sentry-opentelemetry/bin/setup new file mode 100755 index 000000000..dce67d860 --- /dev/null +++ b/sentry-opentelemetry/bin/setup @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -euo pipefail +IFS=$'\n\t' +set -vx + +bundle install + +# Do any other automated setup that you need to do here diff --git a/sentry-opentelemetry/lib/sentry-opentelemetry.rb b/sentry-opentelemetry/lib/sentry-opentelemetry.rb new file mode 100644 index 000000000..4218101ad --- /dev/null +++ b/sentry-opentelemetry/lib/sentry-opentelemetry.rb @@ -0,0 +1,2 @@ +require "sentry/opentelemetry/version" +require "sentry/opentelemetry/span_processor" diff --git a/sentry-ruby/lib/sentry/open_telemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb similarity index 100% rename from sentry-ruby/lib/sentry/open_telemetry/span_processor.rb rename to sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb new file mode 100644 index 000000000..6fd58f94a --- /dev/null +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Sentry + module OpenTelemetry + VERSION = "5.5.0" + end +end diff --git a/sentry-opentelemetry/sentry-opentelemetry.gemspec b/sentry-opentelemetry/sentry-opentelemetry.gemspec new file mode 100644 index 000000000..cc6327263 --- /dev/null +++ b/sentry-opentelemetry/sentry-opentelemetry.gemspec @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require_relative "lib/sentry/opentelemetry/version" + +Gem::Specification.new do |spec| + spec.name = "sentry-opentelemetry" + spec.version = Sentry::OpenTelemetry::VERSION + spec.authors = ["Sentry Team"] + spec.description = spec.summary = "A gem that provides OpenTelemetry integration for the Sentry error logger" + spec.email = "accounts@sentry.io" + spec.license = 'MIT' + spec.homepage = "https://github.com/getsentry/sentry-ruby" + + spec.platform = Gem::Platform::RUBY + spec.required_ruby_version = '>= 2.4' + spec.extra_rdoc_files = ["README.md", "LICENSE.txt"] + spec.files = `git ls-files | grep -Ev '^(spec|benchmarks|examples)'`.split("\n") + + spec.metadata["homepage_uri"] = spec.homepage + spec.metadata["source_code_uri"] = spec.homepage + spec.metadata["changelog_uri"] = "#{spec.homepage}/blob/master/CHANGELOG.md" + + spec.bindir = "exe" + spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } + spec.require_paths = ["lib"] + + spec.add_dependency "sentry-ruby", "~> 5.5.0" + spec.add_dependency "opentelemetry-sdk", ">= 1.0" +end diff --git a/sentry-opentelemetry/spec/spec_helper.rb b/sentry-opentelemetry/spec/spec_helper.rb new file mode 100644 index 000000000..f875739e2 --- /dev/null +++ b/sentry-opentelemetry/spec/spec_helper.rb @@ -0,0 +1,34 @@ +require "bundler/setup" +require "pry" +require "debug" if RUBY_VERSION.to_f >= 2.6 + +require "sentry-ruby" + +require 'simplecov' + +SimpleCov.start do + project_name "sentry-opentelemetry" + root File.join(__FILE__, "../../../") + coverage_dir File.join(__FILE__, "../../coverage") +end + +if ENV["CI"] + require 'simplecov-cobertura' + SimpleCov.formatter = SimpleCov::Formatter::CoberturaFormatter +end + +require "sentry-opentelemetry" + +DUMMY_DSN = 'http://12345:67890@sentry.localdomain/sentry/42' + +RSpec.configure do |config| + # Enable flags like --only-failures and --next-failure + config.example_status_persistence_file_path = ".rspec_status" + + # Disable RSpec exposing methods globally on `Module` and `main` + config.disable_monkey_patching! + + config.expect_with :rspec do |c| + c.syntax = :expect + end +end diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 449cc283a..fc6293c78 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -20,7 +20,6 @@ require "sentry/hub" require "sentry/background_worker" require "sentry/session_flusher" -require "sentry/open_telemetry/span_processor" [ "sentry/rake", From 8508d2ff626cce6d4682db9d388c35176e124aed Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 7 Nov 2022 17:41:49 +0100 Subject: [PATCH 23/44] Use otel classes and constants --- .../lib/sentry-opentelemetry.rb | 1 + .../sentry/opentelemetry/span_processor.rb | 43 ++++++++----------- .../sentry-opentelemetry.gemspec | 2 + 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry-opentelemetry.rb b/sentry-opentelemetry/lib/sentry-opentelemetry.rb index 4218101ad..4d4e85c71 100644 --- a/sentry-opentelemetry/lib/sentry-opentelemetry.rb +++ b/sentry-opentelemetry/lib/sentry-opentelemetry.rb @@ -1,2 +1,3 @@ require "sentry/opentelemetry/version" require "sentry/opentelemetry/span_processor" +require "sentry/opentelemetry/propagator" diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 8daa1d9f8..7a61e6c92 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -1,20 +1,11 @@ +require "opentelemetry" +require "opentelemetry-sdk" +require "opentelemetry-semantic_conventions" + module Sentry module OpenTelemetry - class SpanProcessor - - ATTRIBUTE_HTTP_METHOD = "http.method" - ATTRIBUTE_HTTP_TARGET = "http.target" - ATTRIBUTE_HTTP_STATUS_CODE = "http.status_code" - ATTRIBUTE_NET_PEER_NAME = "net.peer.name" - ATTRIBUTE_DB_SYSTEM = "db.system" - ATTRIBUTE_DB_STATEMENT = "db.statement" - - # https://github.com/open-telemetry/opentelemetry-ruby/blob/18bfd391f2bda2c958d5d6935886c8cba61414dd/api/lib/opentelemetry/trace.rb#L18-L22 - # An invalid trace identifier, a 16-byte string with all zero bytes. - INVALID_TRACE_ID = ("\0" * 16).b - - # An invalid span identifier, an 8-byte string with all zero bytes. - INVALID_SPAN_ID = ("\0" * 8).b + class SpanProcessor < ::OpenTelemetry::SDK::Trace::SpanProcessor + SEMANTIC_CONVENTIONS = ::OpenTelemetry::SemanticConventions::Trace def initialize @otel_span_map = {} @@ -66,7 +57,7 @@ def on_start(otel_span, _parent_context) def on_finish(otel_span) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel - span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == INVALID_SPAN_ID + span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID return unless span_id sentry_span, parent_span = @otel_span_map.delete(span_id) @@ -108,7 +99,7 @@ def from_sentry_sdk?(otel_span) # only check client requests, connects are sometimes internal return false unless %i(client internal).include?(otel_span.kind) - address = otel_span.attributes[ATTRIBUTE_NET_PEER_NAME] + address = otel_span.attributes[SEMANTIC_CONVENTIONS::NET_PEER_NAME] # if no address drop it, just noise return true unless address @@ -119,9 +110,9 @@ def from_sentry_sdk?(otel_span) end def get_trace_data(otel_span) - span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == INVALID_SPAN_ID - trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == INVALID_TRACE_ID - parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == INVALID_SPAN_ID + span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID + trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == ::OpenTelemetry::Trace::INVALID_TRACE_ID + parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID [span_id, trace_id, parent_span_id] end @@ -142,22 +133,22 @@ def update_span_with_otel_data(sentry_span, otel_span) op = otel_span.name description = otel_span.name - if (http_method = otel_span.attributes[ATTRIBUTE_HTTP_METHOD]) + if (http_method = otel_span.attributes[SEMANTIC_CONVENTIONS::HTTP_METHOD]) op = "http.#{otel_span.kind}" description = http_method - peer_name = otel_span.attributes[ATTRIBUTE_NET_PEER_NAME] + peer_name = otel_span.attributes[SEMANTIC_CONVENTIONS::NET_PEER_NAME] description += " #{peer_name}" if peer_name - target = otel_span.attributes[ATTRIBUTE_HTTP_TARGET] + target = otel_span.attributes[SEMANTIC_CONVENTIONS::HTTP_TARGET] description += target if target - status_code = otel_span.attributes[ATTRIBUTE_HTTP_STATUS_CODE] + status_code = otel_span.attributes[SEMANTIC_CONVENTIONS::HTTP_STATUS_CODE] sentry_span.set_http_status(status_code) if status_code - elsif otel_span.attributes[ATTRIBUTE_DB_SYSTEM] + elsif otel_span.attributes[SEMANTIC_CONVENTIONS::DB_SYSTEM] op = "db" - statement = otel_span.attributes[ATTRIBUTE_DB_STATEMENT] + statement = otel_span.attributes[SEMANTIC_CONVENTIONS::DB_STATEMENT] description = statement if statement end diff --git a/sentry-opentelemetry/sentry-opentelemetry.gemspec b/sentry-opentelemetry/sentry-opentelemetry.gemspec index cc6327263..d6526d650 100644 --- a/sentry-opentelemetry/sentry-opentelemetry.gemspec +++ b/sentry-opentelemetry/sentry-opentelemetry.gemspec @@ -26,4 +26,6 @@ Gem::Specification.new do |spec| spec.add_dependency "sentry-ruby", "~> 5.5.0" spec.add_dependency "opentelemetry-sdk", ">= 1.0" + spec.add_dependency "opentelemetry-api", ">= 1.0" + spec.add_dependency "opentelemetry-semantic_conventions", ">= 1.0" end From a31a7283e1386a1c23875cce9faeff293f568bff Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 7 Nov 2022 18:28:24 +0100 Subject: [PATCH 24/44] sentry_trace logic extract --- sentry-ruby/lib/sentry/transaction.rb | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index e237b5a9b..165227b65 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -91,16 +91,10 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h return unless hub.configuration.tracing_enabled? return unless sentry_trace - match = SENTRY_TRACE_REGEXP.match(sentry_trace) - return if match.nil? - trace_id, parent_span_id, sampled_flag = match[1..3] + sentry_trace_data = extract_sentry_trace(sentry_trace) + return unless sentry_trace_data - parent_sampled = - if sampled_flag.nil? - nil - else - sampled_flag != "0" - end + trace_id, parent_span_id, parent_sampled = sentry_trace_data baggage = if baggage && !baggage.empty? Baggage.from_incoming_header(baggage) @@ -123,6 +117,16 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h ) end + 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 + # @return [Hash] def to_hash hash = super From 7f15f964568610a6a267d7ddf2f78e04ea50e32b Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 7 Nov 2022 19:04:59 +0100 Subject: [PATCH 25/44] sentry_trace propagator WIP --- .../lib/sentry-opentelemetry.rb | 3 + .../lib/sentry/opentelemetry/propagator.rb | 62 +++++++++++++++++++ .../sentry/opentelemetry/span_processor.rb | 15 ++--- .../sentry-opentelemetry.gemspec | 2 - sentry-ruby/lib/sentry/net/http.rb | 1 + 5 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb diff --git a/sentry-opentelemetry/lib/sentry-opentelemetry.rb b/sentry-opentelemetry/lib/sentry-opentelemetry.rb index 4d4e85c71..7a2bb70eb 100644 --- a/sentry-opentelemetry/lib/sentry-opentelemetry.rb +++ b/sentry-opentelemetry/lib/sentry-opentelemetry.rb @@ -1,3 +1,6 @@ +require "sentry-ruby" +require "opentelemetry-sdk" + require "sentry/opentelemetry/version" require "sentry/opentelemetry/span_processor" require "sentry/opentelemetry/propagator" diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb new file mode 100644 index 000000000..b00399880 --- /dev/null +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -0,0 +1,62 @@ +module Sentry + module OpenTelemetry + class Propagator + + FIELDS = [SENTRY_TRACE_HEADER_NAME, BAGGAGE_HEADER_NAME].freeze + + SENTRY_TRACE_PARENT_KEY = ::OpenTelemetry::Context.create_key('sentry-trace-parent') + SENTRY_DSC_KEY = ::OpenTelemetry::Context.create_key('sentry-dsc') + + def inject(carrier, + context: ::OpenTelemetry::Context.current, + setter: ::OpenTelemetry::Context::Propagation.text_map_setter) + + span_context = ::OpenTelemetry::Trace.current_span(context).context + return unless span_context.valid? + + sampled_flag = span_context.trace_flags.sampled? ? 1 : 0 unless span_context.trace_flags.nil? + sentry_trace = "#{span_context.hex_trace_id}-#{span_context.hex_span_id}-#{sampled_flag}" + + setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_trace) + end + + def extract(carrier, + context: ::OpenTelemetry::Context.current, + getter: ::OpenTelemetry::Context::Propagation.text_map_getter) + + sentry_trace = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) + return context unless sentry_trace + + sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace) + return unless sentry_trace_data + + context = context.set_value(SENTRY_TRACE_PARENT_KEY, sentry_trace_data) + trace_id, parent_span_id, parent_sampled = sentry_trace_data + + trace_flags = if parent_sampled.nil? + nil + elsif parent_sampled + ::OpenTelemetry::Trace::TraceFlags::SAMPLED + else + ::OpenTelemetry::Trace::TraceFlags::DEFAULT + end + + span_context = ::OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, + span_id: parent_span_id, + trace_flags: trace_flags, + remote: true) + + span = ::OpenTelemetry::Trace.non_recording_span(span_context) + ::OpenTelemetry::Trace.context_with_span(span, parent_context: context) + + # TODO-neel baggage + # baggage = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) + context + end + + def fields + FIELDS + end + end + end +end diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 7a61e6c92..8a51f9f62 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -1,14 +1,12 @@ -require "opentelemetry" -require "opentelemetry-sdk" -require "opentelemetry-semantic_conventions" - module Sentry module OpenTelemetry class SpanProcessor < ::OpenTelemetry::SDK::Trace::SpanProcessor SEMANTIC_CONVENTIONS = ::OpenTelemetry::SemanticConventions::Trace + attr_reader :span_map + def initialize - @otel_span_map = {} + @span_map = {} end def on_start(otel_span, _parent_context) @@ -18,6 +16,7 @@ def on_start(otel_span, _parent_context) span_id, trace_id, parent_span_id = get_trace_data(otel_span) return unless span_id + # TODO-neel remove scope scope = Sentry.get_current_scope parent_sentry_span = scope.get_span @@ -46,12 +45,13 @@ def on_start(otel_span, _parent_context) baggage = scope.baggage Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") + # TODO-neel remove this, pass in everything from context to start_transaction transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **continue_options) if sentry_trace Sentry.start_transaction(transaction: transaction, instrumenter: :otel, **options) end scope.set_span(sentry_span) - @otel_span_map[span_id] = [sentry_span, parent_sentry_span] + @span_map[span_id] = [sentry_span, parent_sentry_span] end def on_finish(otel_span) @@ -60,7 +60,7 @@ def on_finish(otel_span) span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID return unless span_id - sentry_span, parent_span = @otel_span_map.delete(span_id) + sentry_span, parent_span = @span_map.delete(span_id) return unless sentry_span current_scope = Sentry.get_current_scope @@ -110,6 +110,7 @@ def from_sentry_sdk?(otel_span) end def get_trace_data(otel_span) + # TODO-neel get sentry-trace from context span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == ::OpenTelemetry::Trace::INVALID_TRACE_ID parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID diff --git a/sentry-opentelemetry/sentry-opentelemetry.gemspec b/sentry-opentelemetry/sentry-opentelemetry.gemspec index d6526d650..cc6327263 100644 --- a/sentry-opentelemetry/sentry-opentelemetry.gemspec +++ b/sentry-opentelemetry/sentry-opentelemetry.gemspec @@ -26,6 +26,4 @@ Gem::Specification.new do |spec| spec.add_dependency "sentry-ruby", "~> 5.5.0" spec.add_dependency "opentelemetry-sdk", ">= 1.0" - spec.add_dependency "opentelemetry-api", ">= 1.0" - spec.add_dependency "opentelemetry-semantic_conventions", ">= 1.0" end diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index 8d751823b..e6025126a 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -102,6 +102,7 @@ def extract_request_info(req) end Sentry.register_patch do + # TODO-neel don't patch if otel patch = Sentry::Net::HTTP Net::HTTP.send(:prepend, patch) unless Net::HTTP.ancestors.include?(patch) end From f15b9a814da4ef4000c9469d4e7c576b632cd8a6 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 8 Nov 2022 15:57:03 +0100 Subject: [PATCH 26/44] Revert scope / net http changes --- .../lib/sentry/opentelemetry/span_processor.rb | 1 + sentry-rails/lib/sentry/rails/action_cable.rb | 4 ++-- .../lib/sentry/rails/capture_exceptions.rb | 4 ++-- sentry-ruby/lib/sentry/net/http.rb | 11 +++-------- sentry-ruby/lib/sentry/rack/capture_exceptions.rb | 5 +++-- sentry-ruby/lib/sentry/scope.rb | 14 -------------- 6 files changed, 11 insertions(+), 28 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 8a51f9f62..4bf025233 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -41,6 +41,7 @@ def on_start(otel_span, _parent_context) **continue_options } + # TODO-neel cleanup sentry_trace = scope.sentry_trace baggage = scope.baggage diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb index eeb7a1e70..6ccbb5549 100644 --- a/sentry-rails/lib/sentry/rails/action_cable.rb +++ b/sentry-rails/lib/sentry/rails/action_cable.rb @@ -32,8 +32,8 @@ def capture(connection, transaction_name:, extra_context: nil, &block) end def start_transaction(env, scope) - sentry_trace = scope.sentry_trace - baggage = scope.baggage + 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 diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb index 27843dab9..815d8dda0 100644 --- a/sentry-rails/lib/sentry/rails/capture_exceptions.rb +++ b/sentry-rails/lib/sentry/rails/capture_exceptions.rb @@ -31,8 +31,8 @@ def capture_exception(exception, env) end def start_transaction(env, scope) - sentry_trace = scope.sentry_trace - baggage = scope.baggage + sentry_trace = env["HTTP_SENTRY_TRACE"] + baggage = env["HTTP_BAGGAGE"] options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op } diff --git a/sentry-ruby/lib/sentry/net/http.rb b/sentry-ruby/lib/sentry/net/http.rb index e6025126a..ba1e7b564 100644 --- a/sentry-ruby/lib/sentry/net/http.rb +++ b/sentry-ruby/lib/sentry/net/http.rb @@ -47,18 +47,14 @@ def request(req, body = nil, &block) private def set_sentry_trace_header(req, sentry_span) - return unless Sentry.initialized? - - # TODO-neel maybe some otel span validation - sentry_or_otel_span = sentry_span || Sentry.get_current_scope.get_span - return unless sentry_or_otel_span + return unless sentry_span client = Sentry.get_current_client - trace = client.generate_sentry_trace(sentry_or_otel_span) + trace = client.generate_sentry_trace(sentry_span) req[SENTRY_TRACE_HEADER_NAME] = trace if trace - baggage = client.generate_baggage(sentry_or_otel_span) + baggage = client.generate_baggage(sentry_span) req[BAGGAGE_HEADER_NAME] = baggage if baggage && !baggage.empty? end @@ -102,7 +98,6 @@ def extract_request_info(req) end Sentry.register_patch do - # TODO-neel don't patch if otel patch = Sentry::Net::HTTP Net::HTTP.send(:prepend, patch) unless Net::HTTP.ancestors.include?(patch) end diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb index a7dd3dfee..36c8b1848 100644 --- a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb +++ b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb @@ -62,8 +62,9 @@ def capture_exception(exception, env) end def start_transaction(env, scope) - sentry_trace = scope.sentry_trace - baggage = scope.baggage + 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 Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options) diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index 06169d0e0..09c4b7cde 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -143,20 +143,6 @@ def set_rack_env(env) @rack_env = env end - # Get the sentry-trace header on the rack env - # @return [String, nil] - def sentry_trace - return nil unless rack_env - rack_env["HTTP_SENTRY_TRACE"] - end - - # Get the baggage header on the rack env - # @return [String, nil] - def baggage - return nil unless rack_env - rack_env["HTTP_BAGGAGE"] - end - # Sets the scope's span attribute. # @param span [Span] # @return [Span] From 4bf33d7b00326fe4faec654bbb3c60d1aa748696 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 8 Nov 2022 16:40:41 +0100 Subject: [PATCH 27/44] Remove scope from on_start, use span_map entirely --- .../lib/sentry/opentelemetry/propagator.rb | 4 +- .../sentry/opentelemetry/span_processor.rb | 61 +++++++------------ 2 files changed, 24 insertions(+), 41 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index b00399880..e310abe95 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -31,7 +31,7 @@ def extract(carrier, return unless sentry_trace_data context = context.set_value(SENTRY_TRACE_PARENT_KEY, sentry_trace_data) - trace_id, parent_span_id, parent_sampled = sentry_trace_data + trace_id, span_id, parent_sampled = sentry_trace_data trace_flags = if parent_sampled.nil? nil @@ -42,7 +42,7 @@ def extract(carrier, end span_context = ::OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, - span_id: parent_span_id, + span_id: span_id, trace_flags: trace_flags, remote: true) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 4bf025233..2f4d4948b 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -16,43 +16,32 @@ def on_start(otel_span, _parent_context) span_id, trace_id, parent_span_id = get_trace_data(otel_span) return unless span_id - # TODO-neel remove scope - scope = Sentry.get_current_scope - parent_sentry_span = scope.get_span + sentry_parent_span = @span_map[parent_span_id] if parent_span_id - sentry_span = if parent_sentry_span - Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{parent_sentry_span.op}") + sentry_span = if sentry_parent_span + Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{sentry_parent_span.op}") - parent_sentry_span.start_child( + sentry_parent_span.start_child( span_id: span_id, description: otel_span.name, start_timestamp: otel_span.start_timestamp / 1e9 ) else - continue_options = { - span_id: span_id, - name: otel_span.name, - start_timestamp: otel_span.start_timestamp / 1e9 - } + Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") options = { + instrumenter: :otel, + name: otel_span.name, + span_id: span_id, trace_id: trace_id, parent_span_id: parent_span_id, - **continue_options + start_timestamp: otel_span.start_timestamp / 1e9 } - # TODO-neel cleanup - sentry_trace = scope.sentry_trace - baggage = scope.baggage - - Sentry.configuration.logger.info("Starting otel transaction #{otel_span.name}") - # TODO-neel remove this, pass in everything from context to start_transaction - transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **continue_options) if sentry_trace - Sentry.start_transaction(transaction: transaction, instrumenter: :otel, **options) + Sentry.start_transaction(**options) end - scope.set_span(sentry_span) - @span_map[span_id] = [sentry_span, parent_sentry_span] + @span_map[span_id] = sentry_span end def on_finish(otel_span) @@ -61,14 +50,13 @@ def on_finish(otel_span) span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID return unless span_id - sentry_span, parent_span = @span_map.delete(span_id) + sentry_span = @span_map.delete(span_id) return unless sentry_span - current_scope = Sentry.get_current_scope - sentry_span.set_op(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) + current_scope = Sentry.get_current_scope current_scope.set_transaction_name(otel_span.name) current_scope.set_context(:otel, otel_context_hash(otel_span)) else @@ -77,17 +65,6 @@ def on_finish(otel_span) Sentry.configuration.logger.info("Finishing sentry_span #{sentry_span.op}") sentry_span.finish(end_timestamp: otel_span.end_timestamp / 1e9) - current_scope.set_span(parent_span) if parent_span - end - - def force_flush(timeout: nil) - # no-op: we rely on Sentry.close being called for the same reason as - # whatever triggered this shutdown. - end - - def shutdown(timeout: nil) - # no-op: we rely on Sentry.close being called for the same reason as - # whatever triggered this shutdown. end private @@ -110,10 +87,16 @@ def from_sentry_sdk?(otel_span) false end + # TODO-neel get sentry-trace/baggage from context def get_trace_data(otel_span) - # TODO-neel get sentry-trace from context - span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID - trace_id = otel_span.context.hex_trace_id unless otel_span.context.trace_id == ::OpenTelemetry::Trace::INVALID_TRACE_ID + if otel_span.context.valid? + span_id = otel_span.context.hex_span_id + trace_id = otel_span.context.hex_trace_id + else + span_id = nil + trace_id = nil + end + parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID [span_id, trace_id, parent_span_id] From 6529a5c7ab6fe5075841f9b8ac05b9de2b72db6d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 8 Nov 2022 17:18:25 +0100 Subject: [PATCH 28/44] Expose contexts on transaction --- sentry-opentelemetry/README.md | 2 ++ .../sentry/opentelemetry/span_processor.rb | 5 ++--- sentry-ruby/lib/sentry/transaction.rb | 22 +++++++++++++++++++ sentry-ruby/lib/sentry/transaction_event.rb | 1 + 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/sentry-opentelemetry/README.md b/sentry-opentelemetry/README.md index f4bf46b18..af4268aee 100644 --- a/sentry-opentelemetry/README.md +++ b/sentry-opentelemetry/README.md @@ -60,6 +60,8 @@ OpenTelemetry::SDK.configure do |c| c.add_span_processor(Sentry::OpenTelemetry::SpanProcessor.new) end +OpenTelemetry.propagation = Sentry::OpenTelemetry::Propagator.new + Rails.application.config.middleware.move_after( Sentry::Rails::CaptureExceptions, OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 2f4d4948b..a7123136c 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -56,9 +56,8 @@ def on_finish(otel_span) sentry_span.set_op(otel_span.name) if sentry_span.is_a?(Sentry::Transaction) - current_scope = Sentry.get_current_scope - current_scope.set_transaction_name(otel_span.name) - current_scope.set_context(:otel, otel_context_hash(otel_span)) + sentry_span.set_name(otel_span.name) + sentry_span.set_context(:otel, otel_context_hash(otel_span)) else update_span_with_otel_data(sentry_span, otel_span) end diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index 165227b65..58236507d 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -50,6 +50,10 @@ class Transaction < Span # @return [Float, nil] attr_reader :effective_sample_rate + # Additional contexts stored directly on the transaction object. + # @return [Hash] + attr_reader :contexts + def initialize( hub:, name: nil, @@ -74,6 +78,7 @@ def initialize( @environment = hub.configuration.environment @dsn = hub.configuration.dsn @effective_sample_rate = nil + @contexts = {} init_span_recorder end @@ -248,6 +253,23 @@ def get_baggage @baggage end + # Set the transaction name directly. + # @param name [String] + # @param source [Symbol] + # @return [void] + def set_name(name, source: :custom) + @name = name + @source = SOURCES.include?(source) ? source.to_sym : :custom + end + + # Set contexts directly on the transaction. + # @param key [String, Symbol] + # @param value [Object] + # @return [void] + def set_context(key, value) + @contexts[key] = value + end + protected def init_span_recorder(limit = 1000) diff --git a/sentry-ruby/lib/sentry/transaction_event.rb b/sentry-ruby/lib/sentry/transaction_event.rb index 06f89488f..389ca45b0 100644 --- a/sentry-ruby/lib/sentry/transaction_event.rb +++ b/sentry-ruby/lib/sentry/transaction_event.rb @@ -19,6 +19,7 @@ def initialize(transaction:, **options) self.transaction = transaction.name self.transaction_info = { source: transaction.source } + self.contexts.merge!(transaction.contexts) self.contexts.merge!(trace: transaction.get_trace_context) self.timestamp = transaction.timestamp self.start_timestamp = transaction.start_timestamp From 5db849aaebb116895df5d095e7cb59bd1f234b6f Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 8 Nov 2022 17:20:32 +0100 Subject: [PATCH 29/44] Update readme --- sentry-opentelemetry/README.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/sentry-opentelemetry/README.md b/sentry-opentelemetry/README.md index af4268aee..5d5f87424 100644 --- a/sentry-opentelemetry/README.md +++ b/sentry-opentelemetry/README.md @@ -50,7 +50,7 @@ end This will disable all Sentry instrumentation and rely on the chosen OpenTelemetry tracers for creating spans. -Next, configure OpenTelemetry as per your needs and hook in the Sentry span processor and propagator(?? TODO-neel). Note that you need to re-order the middlewares (?? TODO-neel) for Sentry to pick spans up correctly. +Next, configure OpenTelemetry as per your needs and hook in the Sentry span processor and propagator. ```ruby # config/initializers/02_otel.rb @@ -61,10 +61,5 @@ OpenTelemetry::SDK.configure do |c| end OpenTelemetry.propagation = Sentry::OpenTelemetry::Propagator.new - -Rails.application.config.middleware.move_after( - Sentry::Rails::CaptureExceptions, - OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware -) ``` From aba4e19c67b7a5a84776376a6f6ac36280253cdb Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 9 Nov 2022 17:36:12 +0100 Subject: [PATCH 30/44] Make SpanProcessor singleton --- .../lib/sentry/opentelemetry/span_processor.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index a7123136c..c7fe4ed34 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -1,6 +1,10 @@ +require 'singleton' + module Sentry module OpenTelemetry class SpanProcessor < ::OpenTelemetry::SDK::Trace::SpanProcessor + include Singleton + SEMANTIC_CONVENTIONS = ::OpenTelemetry::SemanticConventions::Trace attr_reader :span_map From abc0e5c4a34f366e1e8dc3f5c0439724660092ce Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 9 Nov 2022 17:48:33 +0100 Subject: [PATCH 31/44] Finished sentry-trace propagator --- .../lib/sentry/opentelemetry/propagator.rb | 55 +++++++++---------- .../sentry/opentelemetry/span_processor.rb | 51 +++++++++-------- 2 files changed, 54 insertions(+), 52 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index e310abe95..2a695c1ad 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -4,54 +4,51 @@ class Propagator FIELDS = [SENTRY_TRACE_HEADER_NAME, BAGGAGE_HEADER_NAME].freeze - SENTRY_TRACE_PARENT_KEY = ::OpenTelemetry::Context.create_key('sentry-trace-parent') + SENTRY_TRACE_KEY = ::OpenTelemetry::Context.create_key('sentry-trace') SENTRY_DSC_KEY = ::OpenTelemetry::Context.create_key('sentry-dsc') - def inject(carrier, - context: ::OpenTelemetry::Context.current, - setter: ::OpenTelemetry::Context::Propagation.text_map_setter) - + def inject( + carrier, + context: ::OpenTelemetry::Context.current, + setter: ::OpenTelemetry::Context::Propagation.text_map_setter + ) span_context = ::OpenTelemetry::Trace.current_span(context).context return unless span_context.valid? - sampled_flag = span_context.trace_flags.sampled? ? 1 : 0 unless span_context.trace_flags.nil? - sentry_trace = "#{span_context.hex_trace_id}-#{span_context.hex_span_id}-#{sampled_flag}" + span_map = SpanProcessor.instance.span_map + sentry_span = span_map[span_context.hex_span_id] + return unless sentry_span - setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_trace) + setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_sentry_trace) end - def extract(carrier, - context: ::OpenTelemetry::Context.current, - getter: ::OpenTelemetry::Context::Propagation.text_map_getter) - + def extract( + carrier, + context: ::OpenTelemetry::Context.current, + getter: ::OpenTelemetry::Context::Propagation.text_map_getter + ) sentry_trace = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) return context unless sentry_trace sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace) return unless sentry_trace_data - context = context.set_value(SENTRY_TRACE_PARENT_KEY, sentry_trace_data) - trace_id, span_id, parent_sampled = sentry_trace_data + context = context.set_value(SENTRY_TRACE_KEY, sentry_trace_data) + trace_id, span_id, _parent_sampled = sentry_trace_data - trace_flags = if parent_sampled.nil? - nil - elsif parent_sampled - ::OpenTelemetry::Trace::TraceFlags::SAMPLED - else - ::OpenTelemetry::Trace::TraceFlags::DEFAULT - end + span_context = ::OpenTelemetry::Trace::SpanContext.new( + trace_id: trace_id, + span_id: span_id, + # we simulate a sampled trace on the otel side and leave the sampling to sentry + trace_flags: ::OpenTelemetry::Trace::TraceFlags::SAMPLED, + remote: true + ) - span_context = ::OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, - span_id: span_id, - trace_flags: trace_flags, - remote: true) + # TODO-neel baggage + # baggage = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) span = ::OpenTelemetry::Trace.non_recording_span(span_context) ::OpenTelemetry::Trace.context_with_span(span, parent_context: context) - - # TODO-neel baggage - # baggage = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) - context end def fields diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index c7fe4ed34..58594fad9 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -2,31 +2,35 @@ module Sentry module OpenTelemetry + TraceData = Struct.new(:trace_id, :span_id, :parent_span_id, :parent_sampled) + class SpanProcessor < ::OpenTelemetry::SDK::Trace::SpanProcessor include Singleton SEMANTIC_CONVENTIONS = ::OpenTelemetry::SemanticConventions::Trace + # The mapping from otel span ids to sentry spans + # @return [Hash] attr_reader :span_map def initialize @span_map = {} end - def on_start(otel_span, _parent_context) + def on_start(otel_span, parent_context) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel return if from_sentry_sdk?(otel_span) - span_id, trace_id, parent_span_id = get_trace_data(otel_span) - return unless span_id + trace_data = get_trace_data(otel_span, parent_context) + return unless trace_data.span_id - sentry_parent_span = @span_map[parent_span_id] if parent_span_id + sentry_parent_span = @span_map[trace_data.parent_span_id] if trace_data.parent_span_id sentry_span = if sentry_parent_span Sentry.configuration.logger.info("Continuing otel span #{otel_span.name} on parent #{sentry_parent_span.op}") sentry_parent_span.start_child( - span_id: span_id, + span_id: trace_data.span_id, description: otel_span.name, start_timestamp: otel_span.start_timestamp / 1e9 ) @@ -36,25 +40,23 @@ def on_start(otel_span, _parent_context) options = { instrumenter: :otel, name: otel_span.name, - span_id: span_id, - trace_id: trace_id, - parent_span_id: parent_span_id, + span_id: trace_data.span_id, + trace_id: trace_data.trace_id, + parent_span_id: trace_data.parent_span_id, start_timestamp: otel_span.start_timestamp / 1e9 } Sentry.start_transaction(**options) end - @span_map[span_id] = sentry_span + @span_map[trace_data.span_id] = sentry_span end def on_finish(otel_span) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel + return unless otel_span.context.valid? - span_id = otel_span.context.hex_span_id unless otel_span.context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID - return unless span_id - - sentry_span = @span_map.delete(span_id) + sentry_span = @span_map.delete(otel_span.context.hex_span_id) return unless sentry_span sentry_span.set_op(otel_span.name) @@ -90,19 +92,22 @@ def from_sentry_sdk?(otel_span) false end - # TODO-neel get sentry-trace/baggage from context - def get_trace_data(otel_span) - if otel_span.context.valid? - span_id = otel_span.context.hex_span_id - trace_id = otel_span.context.hex_trace_id - else - span_id = nil - trace_id = nil + # TODO-neel get baggage from context + def get_trace_data(otel_span, parent_context) + trace_data = TraceData.new + return trace_data unless otel_span.context.valid? + + trace_data.span_id = otel_span.context.hex_span_id + trace_data.trace_id = otel_span.context.hex_trace_id + + unless otel_span.parent_span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID + trace_data.parent_span_id = otel_span.parent_span_id.unpack1("H*") end - parent_span_id = otel_span.parent_span_id.unpack1("H*") unless otel_span.parent_span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID + sentry_trace_data = parent_context[Propagator::SENTRY_TRACE_KEY] + trace_data.parent_sampled = sentry_trace_data[2] if sentry_trace_data - [span_id, trace_id, parent_span_id] + trace_data end def otel_context_hash(otel_span) From f75dab2526c5876cf680c07a7d0b1b8337e84863 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 9 Nov 2022 18:34:07 +0100 Subject: [PATCH 32/44] Add baggage propagation --- .../lib/sentry/opentelemetry/propagator.rb | 19 +++++++++++++++++-- .../sentry/opentelemetry/span_processor.rb | 10 +++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index 2a695c1ad..84d3b5b9b 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -5,7 +5,7 @@ class Propagator FIELDS = [SENTRY_TRACE_HEADER_NAME, BAGGAGE_HEADER_NAME].freeze SENTRY_TRACE_KEY = ::OpenTelemetry::Context.create_key('sentry-trace') - SENTRY_DSC_KEY = ::OpenTelemetry::Context.create_key('sentry-dsc') + SENTRY_BAGGAGE_KEY = ::OpenTelemetry::Context.create_key('sentry-baggage') def inject( carrier, @@ -20,6 +20,9 @@ def inject( return unless sentry_span setter.set(carrier, SENTRY_TRACE_HEADER_NAME, sentry_span.to_sentry_trace) + + baggage = sentry_span.to_baggage + setter.set(carrier, BAGGAGE_HEADER_NAME, baggage) if baggage && !baggage.empty? end def extract( @@ -45,7 +48,19 @@ def extract( ) # TODO-neel baggage - # baggage = getter.get(carrier, SENTRY_TRACE_HEADER_NAME) + baggage_header = getter.get(carrier, BAGGAGE_HEADER_NAME) + + 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! + context = context.set_value(SENTRY_BAGGAGE_KEY, baggage) span = ::OpenTelemetry::Trace.non_recording_span(span_context) ::OpenTelemetry::Trace.context_with_span(span, parent_context: context) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index 58594fad9..aba967e79 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -2,7 +2,7 @@ module Sentry module OpenTelemetry - TraceData = Struct.new(:trace_id, :span_id, :parent_span_id, :parent_sampled) + TraceData = Struct.new(:trace_id, :span_id, :parent_span_id, :parent_sampled, :baggage) class SpanProcessor < ::OpenTelemetry::SDK::Trace::SpanProcessor include Singleton @@ -20,9 +20,9 @@ def initialize def on_start(otel_span, parent_context) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel return if from_sentry_sdk?(otel_span) + return unless otel_span.context.valid? trace_data = get_trace_data(otel_span, parent_context) - return unless trace_data.span_id sentry_parent_span = @span_map[trace_data.parent_span_id] if trace_data.parent_span_id @@ -43,6 +43,7 @@ def on_start(otel_span, parent_context) span_id: trace_data.span_id, trace_id: trace_data.trace_id, parent_span_id: trace_data.parent_span_id, + baggage: trace_data.baggage, start_timestamp: otel_span.start_timestamp / 1e9 } @@ -92,11 +93,8 @@ def from_sentry_sdk?(otel_span) false end - # TODO-neel get baggage from context def get_trace_data(otel_span, parent_context) trace_data = TraceData.new - return trace_data unless otel_span.context.valid? - trace_data.span_id = otel_span.context.hex_span_id trace_data.trace_id = otel_span.context.hex_trace_id @@ -107,6 +105,8 @@ def get_trace_data(otel_span, parent_context) sentry_trace_data = parent_context[Propagator::SENTRY_TRACE_KEY] trace_data.parent_sampled = sentry_trace_data[2] if sentry_trace_data + trace_data.baggage = parent_context[Propagator::SENTRY_BAGGAGE_KEY] + trace_data end From 7be30716f682e04ec5789975d5ccde06bcae88bb Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 10 Nov 2022 15:01:28 +0100 Subject: [PATCH 33/44] Add otel.kind to data, fix gemspec after merge --- sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb | 1 + sentry-opentelemetry/lib/sentry/opentelemetry/version.rb | 2 +- sentry-opentelemetry/sentry-opentelemetry.gemspec | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index aba967e79..b789c6779 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -121,6 +121,7 @@ def otel_context_hash(otel_span) end def update_span_with_otel_data(sentry_span, otel_span) + sentry_span.set_data('otel.kind', otel_span.kind) otel_span.attributes&.each { |k, v| sentry_span.set_data(k, v) } op = otel_span.name diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb index 6fd58f94a..549b0732a 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/version.rb @@ -2,6 +2,6 @@ module Sentry module OpenTelemetry - VERSION = "5.5.0" + VERSION = "5.6.0" end end diff --git a/sentry-opentelemetry/sentry-opentelemetry.gemspec b/sentry-opentelemetry/sentry-opentelemetry.gemspec index cc6327263..383c127eb 100644 --- a/sentry-opentelemetry/sentry-opentelemetry.gemspec +++ b/sentry-opentelemetry/sentry-opentelemetry.gemspec @@ -24,6 +24,6 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ["lib"] - spec.add_dependency "sentry-ruby", "~> 5.5.0" + spec.add_dependency "sentry-ruby", "~> 5.6.0" spec.add_dependency "opentelemetry-sdk", ">= 1.0" end From 0ba42c2747bca384758f0e84d92405d010ad5b35 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Nov 2022 14:02:03 +0100 Subject: [PATCH 34/44] Add to ci --- .../workflows/sentry_opentelemetry_test.yml | 51 +++++++++++++++++++ sentry-opentelemetry/Gemfile | 10 ++-- .../lib/sentry/opentelemetry/propagator.rb | 1 - 3 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/sentry_opentelemetry_test.yml diff --git a/.github/workflows/sentry_opentelemetry_test.yml b/.github/workflows/sentry_opentelemetry_test.yml new file mode 100644 index 000000000..1d95f3629 --- /dev/null +++ b/.github/workflows/sentry_opentelemetry_test.yml @@ -0,0 +1,51 @@ +name: sentry-opentelemetry Test + +on: + workflow_dispatch: + push: + branches: + - master + - \d+-\d+ + pull_request: +# Cancel in progress workflows on pull_requests. +# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true +jobs: + test: + defaults: + run: + working-directory: sentry-opentelemetry + name: Ruby ${{ matrix.ruby_version }} & OpenTelemetry ${{ matrix.opentelemetry_version }}, options - ${{ toJson(matrix.options) }} + runs-on: ${{ matrix.os }} + strategy: + matrix: + ruby_version: [2.4, 2.5, 2.6, 2.7, '3.0', head, jruby] + opentelemetry_version: [1.0.0, 1.1.0, 1.2.0] + os: [ubuntu-latest] + include: + - { os: ubuntu-latest, ruby_version: 3.1, options: { rubyopt: "--enable-frozen-string-literal --debug=frozen-string-literal" } } + - { os: ubuntu-latest, ruby_version: 3.1, options: { codecov: 1 } } + steps: + - uses: actions/checkout@v1 + + - name: Set up Ruby ${{ matrix.ruby_version }} + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby_version }} + + - name: Run specs + env: + RUBYOPT: ${{ matrix.options.rubyopt }} + OPENTELEMETRY_VERSION: ${{ matrix.opentelemetry_version }} + run: | + bundle install --jobs 4 --retry 3 + bundle exec rake + + - name: Upload Coverage + if: ${{ matrix.options.codecov }} + run: | + curl -Os https://uploader.codecov.io/latest/linux/codecov + chmod +x codecov + ./codecov -t ${CODECOV_TOKEN} -R `pwd` -f coverage/coverage.xml diff --git a/sentry-opentelemetry/Gemfile b/sentry-opentelemetry/Gemfile index 264539e7f..10bcb2d04 100644 --- a/sentry-opentelemetry/Gemfile +++ b/sentry-opentelemetry/Gemfile @@ -10,11 +10,11 @@ gem 'simplecov' gem "simplecov-cobertura", "~> 1.4" gem "rexml" -# TODO-neel CI -# opentelemetry_version = ENV["OPENTELEMETRY_VERSION"] -# opentelemetry_version = "1.2.0" if opentelemetry_version.nil? -# gem "opentelemetry-sdk" -# gem "opentelemtry-instrumentation-all" +opentelemetry_version = ENV["OPENTELEMETRY_VERSION"] +opentelemetry_version = "1.2.0" if opentelemetry_version.nil? + +gem "opentelemetry-sdk", "~> #{opentelemetry_version}" +gem "opentelemetry-instrumentation-all" gem "sentry-ruby", path: "../sentry-ruby" diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index 84d3b5b9b..b08466f93 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -47,7 +47,6 @@ def extract( remote: true ) - # TODO-neel baggage baggage_header = getter.get(carrier, BAGGAGE_HEADER_NAME) baggage = if baggage_header && !baggage_header.empty? From 7ec474805aea75c52e2af1a7882d10cfe8ddc028 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Nov 2022 14:07:43 +0100 Subject: [PATCH 35/44] fuck old versions --- .github/workflows/sentry_opentelemetry_test.yml | 4 ++-- sentry-opentelemetry/Gemfile | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/sentry_opentelemetry_test.yml b/.github/workflows/sentry_opentelemetry_test.yml index 1d95f3629..51cecb73c 100644 --- a/.github/workflows/sentry_opentelemetry_test.yml +++ b/.github/workflows/sentry_opentelemetry_test.yml @@ -21,8 +21,8 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - ruby_version: [2.4, 2.5, 2.6, 2.7, '3.0', head, jruby] - opentelemetry_version: [1.0.0, 1.1.0, 1.2.0] + ruby_version: [2.6, 2.7, '3.0', head, jruby] + # opentelemetry_version: [1.2.0] os: [ubuntu-latest] include: - { os: ubuntu-latest, ruby_version: 3.1, options: { rubyopt: "--enable-frozen-string-literal --debug=frozen-string-literal" } } diff --git a/sentry-opentelemetry/Gemfile b/sentry-opentelemetry/Gemfile index 10bcb2d04..5bc7e3685 100644 --- a/sentry-opentelemetry/Gemfile +++ b/sentry-opentelemetry/Gemfile @@ -10,10 +10,11 @@ gem 'simplecov' gem "simplecov-cobertura", "~> 1.4" gem "rexml" -opentelemetry_version = ENV["OPENTELEMETRY_VERSION"] -opentelemetry_version = "1.2.0" if opentelemetry_version.nil? +# opentelemetry_version = ENV["OPENTELEMETRY_VERSION"] +# opentelemetry_version = "1.2.0" if opentelemetry_version.nil? +# gem "opentelemetry-sdk", "~> #{opentelemetry_version}" -gem "opentelemetry-sdk", "~> #{opentelemetry_version}" +gem "opentelemetry-sdk" gem "opentelemetry-instrumentation-all" gem "sentry-ruby", path: "../sentry-ruby" From ab57961dd0a4d548ef380066baf84b17e32be4e0 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 11 Nov 2022 15:04:35 +0100 Subject: [PATCH 36/44] Make danger happy --- .github/workflows/sentry_opentelemetry_test.yml | 2 +- sentry-opentelemetry/Gemfile | 2 +- .../lib/sentry/opentelemetry/span_processor.rb | 4 ++++ .../sentry/opentelemetry/span_processor_spec.rb | 13 +++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb diff --git a/.github/workflows/sentry_opentelemetry_test.yml b/.github/workflows/sentry_opentelemetry_test.yml index 51cecb73c..91b800259 100644 --- a/.github/workflows/sentry_opentelemetry_test.yml +++ b/.github/workflows/sentry_opentelemetry_test.yml @@ -31,7 +31,7 @@ jobs: - uses: actions/checkout@v1 - name: Set up Ruby ${{ matrix.ruby_version }} - uses: ruby/setup-ruby@v1 + uses: ruby/setup-ruby@8ddb7b3348b3951590db24c346e94ebafdabc926 with: ruby-version: ${{ matrix.ruby_version }} diff --git a/sentry-opentelemetry/Gemfile b/sentry-opentelemetry/Gemfile index 5bc7e3685..3d2667b51 100644 --- a/sentry-opentelemetry/Gemfile +++ b/sentry-opentelemetry/Gemfile @@ -15,7 +15,7 @@ gem "rexml" # gem "opentelemetry-sdk", "~> #{opentelemetry_version}" gem "opentelemetry-sdk" -gem "opentelemetry-instrumentation-all" +gem "opentelemetry-instrumentation-rails" gem "sentry-ruby", path: "../sentry-ruby" diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index b789c6779..f4b62529f 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -73,6 +73,10 @@ def on_finish(otel_span) sentry_span.finish(end_timestamp: otel_span.end_timestamp / 1e9) end + def clear + @span_map = {} + end + private def from_sentry_sdk?(otel_span) diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb new file mode 100644 index 000000000..6919c1845 --- /dev/null +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +RSpec.describe Sentry::OpenTelemetry::SpanProcessor do + let(:subject) { described_class.instance } + + before { subject.clear } + + describe "singleton instance" do + it "has empty span_map" do + expect(subject.span_map).to eq({}) + end + end +end From b84d78b05da0799c0f1492f2a12ebf1bb00dd761 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 15 Nov 2022 17:16:24 +0100 Subject: [PATCH 37/44] on_start specs --- sentry-opentelemetry/README.md | 2 +- .../sentry/opentelemetry/span_processor.rb | 3 +- .../opentelemetry/span_processor_spec.rb | 81 ++++++++++++++++++- sentry-opentelemetry/spec/spec_helper.rb | 25 +++++- 4 files changed, 104 insertions(+), 7 deletions(-) diff --git a/sentry-opentelemetry/README.md b/sentry-opentelemetry/README.md index 5d5f87424..1c4e645d8 100644 --- a/sentry-opentelemetry/README.md +++ b/sentry-opentelemetry/README.md @@ -57,7 +57,7 @@ Next, configure OpenTelemetry as per your needs and hook in the Sentry span proc OpenTelemetry::SDK.configure do |c| c.use_all - c.add_span_processor(Sentry::OpenTelemetry::SpanProcessor.new) + c.add_span_processor(Sentry::OpenTelemetry::SpanProcessor.instance) end OpenTelemetry.propagation = Sentry::OpenTelemetry::Propagator.new diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb index f4b62529f..7ec818989 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/span_processor.rb @@ -19,8 +19,8 @@ def initialize def on_start(otel_span, parent_context) return unless Sentry.initialized? && Sentry.configuration.instrumenter == :otel - return if from_sentry_sdk?(otel_span) return unless otel_span.context.valid? + return if from_sentry_sdk?(otel_span) trace_data = get_trace_data(otel_span, parent_context) @@ -43,6 +43,7 @@ def on_start(otel_span, parent_context) span_id: trace_data.span_id, trace_id: trace_data.trace_id, parent_span_id: trace_data.parent_span_id, + parent_sampled: trace_data.parent_sampled, baggage: trace_data.baggage, start_timestamp: otel_span.start_timestamp / 1e9 } diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb index 6919c1845..f31f76e80 100644 --- a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb @@ -2,12 +2,87 @@ RSpec.describe Sentry::OpenTelemetry::SpanProcessor do let(:subject) { described_class.instance } + let(:tracer) { ::OpenTelemetry.tracer_provider.tracer('sentry', '1.0') } - before { subject.clear } + before do + perform_basic_setup + perform_otel_setup + subject.clear + end - describe "singleton instance" do - it "has empty span_map" do + describe 'singleton instance' do + it 'has empty span_map' do expect(subject.span_map).to eq({}) end + + it 'raises error on instantiation' do + expect { described_class.new }.to raise_error(NoMethodError) + end + end + + describe '#on_start' do + context 'when root span' do + let(:parent_context) { ::OpenTelemetry::Context.empty } + + let(:root_span) do + attributes = { + 'http.method' => 'GET', + 'http.host' => 'sentry.io', + 'http.scheme' => 'https' + } + + tracer.start_root_span('HTTP GET', attributes: attributes, kind: :server) + end + + let(:invalid_span) { ::OpenTelemetry::SDK::Trace::Span::INVALID } + + it 'noops when not initialized' do + expect(Sentry).to receive(:initialized?).and_return(false) + subject.on_start(root_span, parent_context) + expect(subject.span_map).to eq({}) + end + + it 'noops when instrumenter is not otel' do + perform_basic_setup do |c| + c.instrumenter = :sentry + end + + subject.on_start(root_span, parent_context) + expect(subject.span_map).to eq({}) + end + + it 'noops when invalid span' do + subject.on_start(invalid_span, parent_context) + expect(subject.span_map).to eq({}) + end + + it 'starts a sentry transaction' do + expect(Sentry).to receive(:start_transaction).and_call_original + subject.on_start(root_span, parent_context) + + span_id = root_span.context.hex_span_id + trace_id = root_span.context.hex_trace_id + + expect(subject.span_map.size).to eq(1) + expect(subject.span_map.keys.first).to eq(span_id) + + transaction = subject.span_map.values.first + expect(transaction).to be_a(Sentry::Transaction) + expect(transaction.name).to eq(root_span.name) + expect(transaction.span_id).to eq(span_id) + expect(transaction.trace_id).to eq(trace_id) + expect(transaction.start_timestamp).to eq(root_span.start_timestamp / 1e9) + + expect(transaction.parent_span_id).to eq(nil) + expect(transaction.parent_sampled).to eq(nil) + expect(transaction.baggage).to eq(nil) + end + end + + context 'when child span' do + # TODO + it 'noops on internal sentry sdk requests' do + end + end end end diff --git a/sentry-opentelemetry/spec/spec_helper.rb b/sentry-opentelemetry/spec/spec_helper.rb index f875739e2..c2582bb8f 100644 --- a/sentry-opentelemetry/spec/spec_helper.rb +++ b/sentry-opentelemetry/spec/spec_helper.rb @@ -2,8 +2,6 @@ require "pry" require "debug" if RUBY_VERSION.to_f >= 2.6 -require "sentry-ruby" - require 'simplecov' SimpleCov.start do @@ -17,7 +15,10 @@ SimpleCov.formatter = SimpleCov::Formatter::CoberturaFormatter end +require "sentry-ruby" +require "sentry/test_helper" require "sentry-opentelemetry" +require "opentelemetry/sdk" DUMMY_DSN = 'http://12345:67890@sentry.localdomain/sentry/42' @@ -32,3 +33,23 @@ c.syntax = :expect end end + +def perform_basic_setup + Sentry.init do |config| + config.logger = Logger.new(nil) + config.dsn = Sentry::TestHelper::DUMMY_DSN + config.transport.transport_class = Sentry::DummyTransport + # so the events will be sent synchronously for testing + config.background_worker_threads = 0 + config.instrumenter = :otel + config.traces_sample_rate = 1.0 + yield(config) if block_given? + end +end + +def perform_otel_setup + ::OpenTelemetry::SDK.configure do |c| + # suppress otlp warnings + c.logger = Logger.new(File::NULL) + end +end From bbbe258f4a7f151657d96a444e9b297cee114235 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 16 Nov 2022 16:54:15 +0100 Subject: [PATCH 38/44] Finished on_start specs --- .../opentelemetry/span_processor_spec.rb | 155 ++++++++++++------ sentry-opentelemetry/spec/spec_helper.rb | 2 - 2 files changed, 106 insertions(+), 51 deletions(-) diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb index f31f76e80..2e0ddeea1 100644 --- a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb @@ -10,6 +10,58 @@ subject.clear end + let(:empty_context) { ::OpenTelemetry::Context.empty } + let(:invalid_span) { ::OpenTelemetry::SDK::Trace::Span::INVALID } + + let(:root_span) do + attributes = { + 'http.method' => 'GET', + 'http.host' => 'sentry.io', + 'http.scheme' => 'https' + } + + tracer.start_root_span('HTTP GET', attributes: attributes, kind: :server) + end + + let(:root_parent_context) do + ::OpenTelemetry::Trace.context_with_span(root_span, parent_context: empty_context) + end + + let(:child_db_span) do + attributes = { + 'db.system' => 'postgresql', + 'db.user' => 'foo', + 'db.name' => 'foo', + 'net.peer.name' => 'localhost', + 'net.transport' => 'IP.TCP', + 'net.peer.ip' => '::1,127.0.0.1', + 'net.peer.port' => '5432,5432', + 'db.operation' => 'SELECT', + 'db.statement' => 'SELECT * FROM foo' + } + + tracer.start_span('SELECT table', with_parent: root_parent_context, attributes: attributes, kind: :client) + end + + let(:child_internal_span) do + attributes = { + 'http.method' => 'POST', + 'http.scheme' => 'https', + 'http.target' => '/api/5434472/envelope/', + 'net.peer.name' => 'sentry.localdomain', + 'net.peer.port' => 443 + } + + tracer.start_span('HTTP POST', with_parent: root_parent_context, attributes: attributes, kind: :client) + end + + def setup_root + subject.on_start(root_span, empty_context) + transaction = subject.span_map.values.first + transaction + end + + describe 'singleton instance' do it 'has empty span_map' do expect(subject.span_map).to eq({}) @@ -21,68 +73,73 @@ end describe '#on_start' do - context 'when root span' do - let(:parent_context) { ::OpenTelemetry::Context.empty } - - let(:root_span) do - attributes = { - 'http.method' => 'GET', - 'http.host' => 'sentry.io', - 'http.scheme' => 'https' - } + it 'noops when not initialized' do + expect(Sentry).to receive(:initialized?).and_return(false) + subject.on_start(root_span, empty_context) + expect(subject.span_map).to eq({}) + end - tracer.start_root_span('HTTP GET', attributes: attributes, kind: :server) + it 'noops when instrumenter is not otel' do + perform_basic_setup do |c| + c.instrumenter = :sentry end - let(:invalid_span) { ::OpenTelemetry::SDK::Trace::Span::INVALID } + subject.on_start(root_span, empty_context) + expect(subject.span_map).to eq({}) + end - it 'noops when not initialized' do - expect(Sentry).to receive(:initialized?).and_return(false) - subject.on_start(root_span, parent_context) - expect(subject.span_map).to eq({}) - end + it 'noops when invalid span' do + subject.on_start(invalid_span, empty_context) + expect(subject.span_map).to eq({}) + end - it 'noops when instrumenter is not otel' do - perform_basic_setup do |c| - c.instrumenter = :sentry - end + it 'noops on internal sentry sdk requests' do + transaction = setup_root + expect(transaction).not_to receive(:start_child) + subject.on_start(child_internal_span, root_parent_context) + end - subject.on_start(root_span, parent_context) - expect(subject.span_map).to eq({}) - end + it 'starts a sentry transaction on otel root span' do + expect(Sentry).to receive(:start_transaction).and_call_original + subject.on_start(root_span, empty_context) - it 'noops when invalid span' do - subject.on_start(invalid_span, parent_context) - expect(subject.span_map).to eq({}) - end + span_id = root_span.context.hex_span_id + trace_id = root_span.context.hex_trace_id - it 'starts a sentry transaction' do - expect(Sentry).to receive(:start_transaction).and_call_original - subject.on_start(root_span, parent_context) + expect(subject.span_map.size).to eq(1) + expect(subject.span_map.keys.first).to eq(span_id) - span_id = root_span.context.hex_span_id - trace_id = root_span.context.hex_trace_id + transaction = subject.span_map.values.first + expect(transaction).to be_a(Sentry::Transaction) + expect(transaction.name).to eq(root_span.name) + expect(transaction.span_id).to eq(span_id) + expect(transaction.trace_id).to eq(trace_id) + expect(transaction.start_timestamp).to eq(root_span.start_timestamp / 1e9) - expect(subject.span_map.size).to eq(1) - expect(subject.span_map.keys.first).to eq(span_id) + expect(transaction.parent_span_id).to eq(nil) + expect(transaction.parent_sampled).to eq(nil) + expect(transaction.baggage).to eq(nil) + end - transaction = subject.span_map.values.first - expect(transaction).to be_a(Sentry::Transaction) - expect(transaction.name).to eq(root_span.name) - expect(transaction.span_id).to eq(span_id) - expect(transaction.trace_id).to eq(trace_id) - expect(transaction.start_timestamp).to eq(root_span.start_timestamp / 1e9) + it 'starts a sentry child span on otel child span' do + transaction = setup_root - expect(transaction.parent_span_id).to eq(nil) - expect(transaction.parent_sampled).to eq(nil) - expect(transaction.baggage).to eq(nil) - end - end + expect(transaction).to receive(:start_child).and_call_original + subject.on_start(child_db_span, root_parent_context) - context 'when child span' do - # TODO - it 'noops on internal sentry sdk requests' do - end + span_id = child_db_span.context.hex_span_id + trace_id = child_db_span.context.hex_trace_id + + expect(subject.span_map.size).to eq(2) + expect(subject.span_map.keys.last).to eq(span_id) + + sentry_span = subject.span_map[span_id] + expect(sentry_span).to be_a(Sentry::Span) + expect(sentry_span.transaction).to eq(transaction) + expect(sentry_span.span_id).to eq(span_id) + expect(sentry_span.trace_id).to eq(trace_id) + expect(sentry_span.description).to eq(child_db_span.name) + expect(sentry_span.start_timestamp).to eq(child_db_span.start_timestamp / 1e9) end end end diff --git a/sentry-opentelemetry/spec/spec_helper.rb b/sentry-opentelemetry/spec/spec_helper.rb index c2582bb8f..de6215585 100644 --- a/sentry-opentelemetry/spec/spec_helper.rb +++ b/sentry-opentelemetry/spec/spec_helper.rb @@ -20,8 +20,6 @@ require "sentry-opentelemetry" require "opentelemetry/sdk" -DUMMY_DSN = 'http://12345:67890@sentry.localdomain/sentry/42' - RSpec.configure do |config| # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = ".rspec_status" From 1e5b0fe38dbb400194d4db6da58e571e56f292c2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 16 Nov 2022 18:41:04 +0100 Subject: [PATCH 39/44] Finished span_processor spec --- .../opentelemetry/span_processor_spec.rb | 164 ++++++++++++++---- 1 file changed, 135 insertions(+), 29 deletions(-) diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb index 2e0ddeea1..86d56bdb7 100644 --- a/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/span_processor_spec.rb @@ -2,14 +2,8 @@ RSpec.describe Sentry::OpenTelemetry::SpanProcessor do let(:subject) { described_class.instance } - let(:tracer) { ::OpenTelemetry.tracer_provider.tracer('sentry', '1.0') } - - before do - perform_basic_setup - perform_otel_setup - subject.clear - end + let(:tracer) { ::OpenTelemetry.tracer_provider.tracer('sentry', '1.0') } let(:empty_context) { ::OpenTelemetry::Context.empty } let(:invalid_span) { ::OpenTelemetry::SDK::Trace::Span::INVALID } @@ -43,6 +37,19 @@ tracer.start_span('SELECT table', with_parent: root_parent_context, attributes: attributes, kind: :client) end + let(:child_http_span) do + attributes = { + 'http.method' => 'GET', + 'http.scheme' => 'https', + 'http.target' => '/search', + 'net.peer.name' => 'www.google.com', + 'net.peer.port' => 443, + 'http.status_code' => 200 + } + + tracer.start_span('HTTP GET', with_parent: root_parent_context, attributes: attributes, kind: :client) + end + let(:child_internal_span) do attributes = { 'http.method' => 'POST', @@ -55,13 +62,12 @@ tracer.start_span('HTTP POST', with_parent: root_parent_context, attributes: attributes, kind: :client) end - def setup_root - subject.on_start(root_span, empty_context) - transaction = subject.span_map.values.first - transaction + before do + perform_basic_setup + perform_otel_setup + subject.clear end - describe 'singleton instance' do it 'has empty span_map' do expect(subject.span_map).to eq({}) @@ -93,12 +99,6 @@ def setup_root expect(subject.span_map).to eq({}) end - it 'noops on internal sentry sdk requests' do - transaction = setup_root - expect(transaction).not_to receive(:start_child) - subject.on_start(child_internal_span, root_parent_context) - end - it 'starts a sentry transaction on otel root span' do expect(Sentry).to receive(:start_transaction).and_call_original subject.on_start(root_span, empty_context) @@ -121,25 +121,131 @@ def setup_root expect(transaction.baggage).to eq(nil) end - it 'starts a sentry child span on otel child span' do - transaction = setup_root + context 'with started transaction' do + let(:transaction) do + subject.on_start(root_span, empty_context) + subject.span_map.values.first + end + + it 'noops on internal sentry sdk requests' do + expect(transaction).not_to receive(:start_child) + subject.on_start(child_internal_span, root_parent_context) + end + + it 'starts a sentry child span on otel child span' do + expect(transaction).to receive(:start_child).and_call_original + subject.on_start(child_db_span, root_parent_context) + + span_id = child_db_span.context.hex_span_id + trace_id = child_db_span.context.hex_trace_id - expect(transaction).to receive(:start_child).and_call_original + expect(subject.span_map.size).to eq(2) + expect(subject.span_map.keys.last).to eq(span_id) + + sentry_span = subject.span_map[span_id] + expect(sentry_span).to be_a(Sentry::Span) + expect(sentry_span.transaction).to eq(transaction) + expect(sentry_span.span_id).to eq(span_id) + expect(sentry_span.trace_id).to eq(trace_id) + expect(sentry_span.description).to eq(child_db_span.name) + expect(sentry_span.start_timestamp).to eq(child_db_span.start_timestamp / 1e9) + end + end + end + + describe '#on_finish' do + before do + subject.on_start(root_span, empty_context) subject.on_start(child_db_span, root_parent_context) + subject.on_start(child_http_span, root_parent_context) + end - span_id = child_db_span.context.hex_span_id - trace_id = child_db_span.context.hex_trace_id + let(:finished_db_span) { child_db_span.finish } + let(:finished_http_span) { child_http_span.finish } + let(:finished_root_span) { root_span.finish } + let(:finished_invalid_span) { invalid_span.finish } + + it 'noops when not initialized' do + expect(Sentry).to receive(:initialized?).and_return(false) + expect(subject.span_map).not_to receive(:delete) + subject.on_finish(finished_root_span) + end + + it 'noops when instrumenter is not otel' do + perform_basic_setup do |c| + c.instrumenter = :sentry + end + + expect(subject.span_map).not_to receive(:delete) + subject.on_finish(finished_root_span) + end + + it 'noops when invalid span' do + expect(subject.span_map).not_to receive(:delete) + subject.on_finish(finished_invalid_span) + end + + it 'finishes sentry child span on otel child db span finish' do + expect(subject.span_map).to receive(:delete).and_call_original + + span_id = finished_db_span.context.hex_span_id + sentry_span = subject.span_map[span_id] + expect(sentry_span).to be_a(Sentry::Span) + + expect(sentry_span).to receive(:finish).and_call_original + subject.on_finish(finished_db_span) + + expect(sentry_span.op).to eq('db') + expect(sentry_span.description).to eq(finished_db_span.attributes['db.statement']) + expect(sentry_span.data).to include(finished_db_span.attributes) + expect(sentry_span.data).to include({ 'otel.kind' => finished_db_span.kind }) + expect(sentry_span.timestamp).to eq(finished_db_span.end_timestamp / 1e9) expect(subject.span_map.size).to eq(2) - expect(subject.span_map.keys.last).to eq(span_id) + expect(subject.span_map.keys).not_to include(span_id) + end + it 'finishes sentry child span on otel child http span finish' do + expect(subject.span_map).to receive(:delete).and_call_original + + span_id = finished_http_span.context.hex_span_id sentry_span = subject.span_map[span_id] expect(sentry_span).to be_a(Sentry::Span) - expect(sentry_span.transaction).to eq(transaction) - expect(sentry_span.span_id).to eq(span_id) - expect(sentry_span.trace_id).to eq(trace_id) - expect(sentry_span.description).to eq(child_db_span.name) - expect(sentry_span.start_timestamp).to eq(child_db_span.start_timestamp / 1e9) + + expect(sentry_span).to receive(:finish).and_call_original + subject.on_finish(finished_http_span) + + expect(sentry_span.op).to eq('http.client') + expect(sentry_span.description).to eq('GET www.google.com/search') + expect(sentry_span.data).to include(finished_http_span.attributes) + expect(sentry_span.data).to include({ 'otel.kind' => finished_http_span.kind }) + expect(sentry_span.timestamp).to eq(finished_http_span.end_timestamp / 1e9) + + expect(subject.span_map.size).to eq(2) + expect(subject.span_map.keys).not_to include(span_id) + end + + it 'finishes sentry transaction on otel root span finish' do + subject.on_finish(finished_db_span) + subject.on_finish(finished_http_span) + + expect(subject.span_map).to receive(:delete).and_call_original + + span_id = finished_root_span.context.hex_span_id + transaction = subject.span_map[span_id] + expect(transaction).to be_a(Sentry::Transaction) + + expect(transaction).to receive(:finish).and_call_original + subject.on_finish(finished_root_span) + + expect(transaction.op).to eq(finished_root_span.name) + expect(transaction.name).to eq(finished_root_span.name) + expect(transaction.contexts[:otel]).to eq({ + attributes: finished_root_span.attributes, + resource: finished_root_span.resource.attribute_enumerator.to_h + }) + + expect(subject.span_map).to eq({}) end end end From 54fe113e9ebcd809e8486c4f17fab46cda346cbc Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 17 Nov 2022 15:59:40 +0100 Subject: [PATCH 40/44] Start propagator spec --- .../sentry/opentelemetry/propagator_spec.rb | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb new file mode 100644 index 000000000..0096bc1b1 --- /dev/null +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +RSpec.describe Sentry::OpenTelemetry::Propagator do + let(:tracer) { ::OpenTelemetry.tracer_provider.tracer('sentry', '1.0') } + let(:span_processor) { Sentry::OpenTelemetry::SpanProcessor.instance } + let(:span_map) { span_processor.span_map } + + before do + perform_basic_setup + perform_otel_setup + span_map.clear + end + + describe '#inject' do + let(:carrier) { {} } + + it 'noops with invalid span_context' do + subject.inject(carrier) + expect(carrier).to eq({}) + end + + it 'noops if span not found in span_map' do + span = tracer.start_root_span('test') + ctx = ::OpenTelemetry::Trace.context_with_span(span) + + expect(span_map).to receive(:[]).and_call_original + subject.inject(carrier, context: ctx) + expect(carrier).to eq({}) + end + + context 'with running trace' do + let(:ctx) do + # setup root span, child span and return current context + empty_context = ::OpenTelemetry::Context.empty + root_span = tracer.start_root_span('test') + span_processor.on_start(root_span, empty_context) + root_context = ::OpenTelemetry::Trace.context_with_span(root_span, parent_context: empty_context) + child_span = tracer.start_span('child test', with_parent: root_context) + span_processor.on_start(child_span, root_context) + expect(span_map.size).to eq(2) + + ::OpenTelemetry::Trace.context_with_span(child_span, parent_context: root_context) + end + + it 'sets sentry-trace and baggage headers on carrier' do + subject.inject(carrier, context: ctx) + + span_id = ::OpenTelemetry::Trace.current_span(ctx).context.hex_span_id + span = span_map[span_id] + + expect(carrier['sentry-trace']).to eq(span.to_sentry_trace) + expect(carrier['baggage']).to eq(span.to_baggage) + end + end + end + + describe '#extract' do + + end + + describe '#fields' do + it 'returns header names' do + expect(subject.fields).to eq(['sentry-trace', 'baggage']) + end + end +end From e10476dc19a42d81bd424e499abe3a09f47c72c8 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 18 Nov 2022 15:33:09 +0100 Subject: [PATCH 41/44] Fix hex pack in extract; finish propagator specs --- .../lib/sentry/opentelemetry/propagator.rb | 6 +- .../sentry/opentelemetry/propagator_spec.rb | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb index b08466f93..c2b8a6ab4 100644 --- a/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb +++ b/sentry-opentelemetry/lib/sentry/opentelemetry/propagator.rb @@ -34,14 +34,14 @@ def extract( return context unless sentry_trace sentry_trace_data = Transaction.extract_sentry_trace(sentry_trace) - return unless sentry_trace_data + return context unless sentry_trace_data context = context.set_value(SENTRY_TRACE_KEY, sentry_trace_data) trace_id, span_id, _parent_sampled = sentry_trace_data span_context = ::OpenTelemetry::Trace::SpanContext.new( - trace_id: trace_id, - span_id: span_id, + trace_id: [trace_id].pack('H*'), + span_id: [span_id].pack('H*'), # we simulate a sampled trace on the otel side and leave the sampling to sentry trace_flags: ::OpenTelemetry::Trace::TraceFlags::SAMPLED, remote: true diff --git a/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb b/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb index 0096bc1b1..41503297a 100644 --- a/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb +++ b/sentry-opentelemetry/spec/sentry/opentelemetry/propagator_spec.rb @@ -55,7 +55,85 @@ end describe '#extract' do + let(:ctx) { ::OpenTelemetry::Context.empty } + it 'returns unchanged context without sentry-trace' do + carrier = {} + updated_ctx = subject.extract(carrier, context: ctx) + expect(updated_ctx).to eq(ctx) + end + + it 'returns unchanged context with invalid sentry-trace' do + carrier = { 'sentry-trace' => '000-000-0' } + updated_ctx = subject.extract(carrier, context: ctx) + expect(updated_ctx).to eq(ctx) + end + + context 'with only sentry-trace header' do + let(:carrier) do + { 'sentry-trace' => 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1' } + end + + it 'returns context with sentry-trace data' do + updated_ctx = subject.extract(carrier, context: ctx) + + sentry_trace_data = updated_ctx[described_class::SENTRY_TRACE_KEY] + expect(sentry_trace_data).not_to be_nil + + trace_id, parent_span_id, parent_sampled = sentry_trace_data + expect(trace_id).to eq('d4cda95b652f4a1592b449d5929fda1b') + expect(parent_span_id).to eq('6e0c63257de34c92') + expect(parent_sampled).to eq(true) + end + + it 'returns context with empty frozen baggage' do + updated_ctx = subject.extract(carrier, context: ctx) + + baggage = updated_ctx[described_class::SENTRY_BAGGAGE_KEY] + expect(baggage).to be_a(Sentry::Baggage) + expect(baggage.items).to eq({}) + expect(baggage.mutable).to eq(false) + end + + it 'returns context with correct span_context' do + updated_ctx = subject.extract(carrier, context: ctx) + + span_context = ::OpenTelemetry::Trace.current_span(updated_ctx).context + expect(span_context.valid?).to eq(true) + expect(span_context.hex_trace_id).to eq('d4cda95b652f4a1592b449d5929fda1b') + expect(span_context.hex_span_id).to eq('6e0c63257de34c92') + expect(span_context.trace_flags.sampled?).to eq(true) + expect(span_context.remote?).to eq(true) + end + end + + context 'with sentry-trace and baggage headers' do + let(:carrier) do + { + 'sentry-trace' => 'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1', + 'baggage' => 'other-vendor-value-1=foo;bar;baz, '\ + 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b, '\ + 'sentry-public_key=49d0f7386ad645858ae85020e393bef3, '\ + 'sentry-sample_rate=0.01337, '\ + 'sentry-user_id=Am%C3%A9lie, '\ + 'other-vendor-value-2=foo;bar;' + } + end + + it 'returns context with baggage' do + updated_ctx = subject.extract(carrier, context: ctx) + + baggage = updated_ctx[described_class::SENTRY_BAGGAGE_KEY] + expect(baggage).to be_a(Sentry::Baggage) + expect(baggage.mutable).to eq(false) + expect(baggage.items).to eq({ + 'sample_rate' => '0.01337', + 'public_key' => '49d0f7386ad645858ae85020e393bef3', + 'trace_id' => 'd4cda95b652f4a1592b449d5929fda1b', + 'user_id' => 'Amélie' + }) + end + end end describe '#fields' do From 944f2ed00d51b1ed3d911c89c9177ea5a48edc38 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 18 Nov 2022 15:37:33 +0100 Subject: [PATCH 42/44] Fix gem build warning --- sentry-opentelemetry/sentry-opentelemetry.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-opentelemetry/sentry-opentelemetry.gemspec b/sentry-opentelemetry/sentry-opentelemetry.gemspec index 383c127eb..e9cd2f156 100644 --- a/sentry-opentelemetry/sentry-opentelemetry.gemspec +++ b/sentry-opentelemetry/sentry-opentelemetry.gemspec @@ -25,5 +25,5 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "sentry-ruby", "~> 5.6.0" - spec.add_dependency "opentelemetry-sdk", ">= 1.0" + spec.add_dependency "opentelemetry-sdk", "~> 1.0" end From dc7b0c6d0de1f9c470a33df85c96e3dd54209a02 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 18 Nov 2022 15:40:29 +0100 Subject: [PATCH 43/44] Add sentry-opentelemetry to craft/release flow --- .craft.yml | 1 + .scripts/batch_build.rb | 2 +- .scripts/batch_release.rb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.craft.yml b/.craft.yml index a4972d4c9..0786e6366 100644 --- a/.craft.yml +++ b/.craft.yml @@ -10,4 +10,5 @@ targets: 'gem:sentry-rails': 'gem:sentry-sidekiq': 'gem:sentry-delayed_job': + 'gem:sentry-opentelemetry': - name: github diff --git a/.scripts/batch_build.rb b/.scripts/batch_build.rb index 8f7c078e0..cfe961077 100755 --- a/.scripts/batch_build.rb +++ b/.scripts/batch_build.rb @@ -1,4 +1,4 @@ -INTEGRATIONS = %w(sentry-rails sentry-sidekiq sentry-delayed_job sentry-resque) +INTEGRATIONS = %w(sentry-rails sentry-sidekiq sentry-delayed_job sentry-resque sentry-opentelemetry) GEMS = %w(sentry-ruby) + INTEGRATIONS GEMS.each do |gem_name| diff --git a/.scripts/batch_release.rb b/.scripts/batch_release.rb index 51273914a..ab368fd9d 100755 --- a/.scripts/batch_release.rb +++ b/.scripts/batch_release.rb @@ -1,4 +1,4 @@ -INTEGRATIONS = %w(sentry-rails sentry-sidekiq sentry-delayed_job sentry-resque) +INTEGRATIONS = %w(sentry-rails sentry-sidekiq sentry-delayed_job sentry-resque sentry-opentelemetry) GEMS = %w(sentry-ruby) + INTEGRATIONS def get_version_file_name(gem_name) From 2c431c1d0792127d08ab74adcdc09fa0001cc9b8 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 22 Nov 2022 13:55:17 +0100 Subject: [PATCH 44/44] Just disable subscribers for rails and remove safe span access --- sentry-rails/lib/sentry/rails/railtie.rb | 2 +- .../sentry/rails/tracing/abstract_subscriber.rb | 2 +- .../tracing/action_controller_subscriber.rb | 4 ++-- .../rails/tracing/active_record_subscriber.rb | 2 +- .../rails/tracing/active_storage_subscriber.rb | 2 +- sentry-rails/spec/sentry/rails/tracing_spec.rb | 17 +++++++++++++++++ 6 files changed, 23 insertions(+), 6 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb index 8e21931ca..63f929b1c 100644 --- a/sentry-rails/lib/sentry/rails/railtie.rb +++ b/sentry-rails/lib/sentry/rails/railtie.rb @@ -115,7 +115,7 @@ def override_streaming_reporter end def activate_tracing - if Sentry.configuration.tracing_enabled? + if Sentry.configuration.tracing_enabled? && Sentry.configuration.instrumenter == :sentry subscribers = Sentry.configuration.rails.tracing_subscribers Sentry::Rails::Tracing.register_subscribers(subscribers) Sentry::Rails::Tracing.subscribe_tracing_events diff --git a/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb index d79ae207b..17438d191 100644 --- a/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/abstract_subscriber.rb @@ -43,7 +43,7 @@ def record_on_current_span(duration:, **options) Sentry.with_child_span(**options) do |child_span| # duration in ActiveSupport is computed in millisecond # so we need to covert it as second before calculating the timestamp - child_span&.set_timestamp(child_span.start_timestamp + duration / 1000) + child_span.set_timestamp(child_span.start_timestamp + duration / 1000) yield(child_span) if block_given? end end diff --git a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb index a7faa2ed1..4a7970b0a 100644 --- a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb @@ -23,8 +23,8 @@ def self.subscribe! ) do |span| payload = payload.dup cleanup_data(payload) - span&.set_data(:payload, payload) - span&.set_http_status(payload[:status]) + span.set_data(:payload, payload) + span.set_http_status(payload[:status]) end end end diff --git a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb index e84901773..c19e7dad4 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb @@ -13,7 +13,7 @@ def self.subscribe! next if EXCLUDED_EVENTS.include? payload[:name] record_on_current_span(op: SPAN_PREFIX + event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:sql], duration: duration) do |span| - span&.set_data(:connection_id, payload[:connection_id]) + span.set_data(:connection_id, payload[:connection_id]) end end end diff --git a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb index ece6b3cc2..2fd523e9a 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb @@ -23,7 +23,7 @@ def self.subscribe! subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload| record_on_current_span(op: "file.#{event_name}".freeze, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:service], duration: duration) do |span| payload.each do |key, value| - span&.set_data(key, value) unless key == START_TIMESTAMP_NAME + span.set_data(key, value) unless key == START_TIMESTAMP_NAME end end end diff --git a/sentry-rails/spec/sentry/rails/tracing_spec.rb b/sentry-rails/spec/sentry/rails/tracing_spec.rb index 8d8b9da54..346471f9c 100644 --- a/sentry-rails/spec/sentry/rails/tracing_spec.rb +++ b/sentry-rails/spec/sentry/rails/tracing_spec.rb @@ -91,6 +91,23 @@ end end + context "with instrumenter :otel" do + before do + make_basic_app do |config| + config.traces_sample_rate = 1.0 + config.instrumenter = :otel + end + end + + it "doesn't do any tracing" do + p = Post.create! + get "/posts/#{p.id}" + + expect(response).to have_http_status(:ok) + expect(transport.events.count).to eq(0) + end + end + context "with sprockets-rails" do let(:string_io) { StringIO.new } let(:logger) do