Skip to content

Commit

Permalink
Add config.instrumenter to switch between sentry and otel instrumenta…
Browse files Browse the repository at this point in the history
…tion (#1944)
  • Loading branch information
sl0thentr0py authored Nov 29, 2022
1 parent f78a284 commit a62fb2d
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 3 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## Unreleased

### Features

- Add OpenTelemetry support with new `sentry-opentelemetry` gem
- Add `config.instrumenter` to switch between sentry and otel instrumentation [#1944](https://github.com/getsentry/sentry-ruby/pull/1944)

## 5.6.0

### Features
Expand Down
24 changes: 24 additions & 0 deletions sentry-delayed_job/spec/sentry/delayed_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,30 @@ def perform
event = transport.events.last
expect(event.contexts.dig(:trace, :trace_id)).to eq(transaction.contexts.dig(:trace, :trace_id))
end

context "with instrumenter :otel" do
before do
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
config.instrumenter = :otel
config.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::DelayedJobAdapter"
end
end

it "does not record transaction" do
FailedJob.perform_later
enqueued_job = Delayed::Backend::ActiveRecord::Job.last
begin
enqueued_job.invoke_job
rescue ZeroDivisionError
nil
end

expect(transport.events.count).to eq(1)
event = transport.events.last
expect(event).to be_a(Sentry::ErrorEvent)
end
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion sentry-rails/lib/sentry/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions sentry-rails/spec/sentry/rails/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions sentry-resque/spec/sentry/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,22 @@ def self.perform(msg)
expect(tracing_event.dig(:contexts, :trace, :status)).to eq("internal_error")
expect(tracing_event.dig(:contexts, :trace, :op)).to eq("queue.resque")
end

context "with instrumenter :otel" do
before do
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
config.instrumenter = :otel
end
end

it "does not record transaction" do
Resque::Job.create(:default, MessageJob, "report")
worker.work(0)

expect(transport.events.count).to eq(1)
event = transport.events.first.to_hash
expect(event[:message]).to eq("report")
end
end
end
11 changes: 11 additions & 0 deletions sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ class Configuration
# @return [Boolean]
attr_accessor :auto_session_tracking

# The instrumenter to use, :sentry or :otel
# @return [Symbol]
attr_reader :instrumenter

# these are not config options
# @!visibility private
attr_reader :errors, :gem_specs
Expand All @@ -237,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 = []
Expand Down Expand Up @@ -269,6 +275,7 @@ def initialize
self.trusted_proxies = []
self.dsn = ENV['SENTRY_DSN']
self.server_name = server_name_from_env
self.instrumenter = :sentry

self.before_send = nil
self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT
Expand Down Expand Up @@ -332,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 = []

Expand Down
7 changes: 5 additions & 2 deletions sentry-ruby/lib/sentry/hub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand All @@ -92,7 +93,9 @@ def start_transaction(transaction: nil, custom_sampling_context: {}, **options)
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

Expand Down
21 changes: 21 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,25 @@ class SentryConfigurationSample < Sentry::Configuration
expect(subject.auto_session_tracking).to eq(false)
end
end

describe "#instrumenter" do
it "returns :sentry by default" do
expect(subject.instrumenter).to eq(:sentry)
end

it "can be set to :sentry" do
subject.instrumenter = :sentry
expect(subject.instrumenter).to eq(:sentry)
end

it "can be set to :otel" do
subject.instrumenter = :otel
expect(subject.instrumenter).to eq(:otel)
end

it "defaults to :sentry if invalid" do
subject.instrumenter = :foo
expect(subject.instrumenter).to eq(:sentry)
end
end
end
57 changes: 57 additions & 0 deletions sentry-ruby/spec/sentry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,24 @@
expect(described_class.start_transaction(op: "foo")).to eq(nil)
end
end

context "when instrumenter is not :sentry" do
before do
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
config.instrumenter = :otel
end
end

it "noops without explicit instrumenter" do
expect(described_class.start_transaction(op: "foo")).to eq(nil)
end

it "creates transaction with explicit instrumenter" do
transaction = described_class.start_transaction(op: "foo", instrumenter: :otel)
expect(transaction).to be_a(Sentry::Transaction)
end
end
end

describe ".with_child_span" do
Expand Down Expand Up @@ -515,6 +533,45 @@
expect(child_span.parent_span_id).to eq(parent_span.span_id)
expect(child_span.timestamp).to be_a(Float)
end

context "when instrumenter is not :sentry" do
before do
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
config.instrumenter = :otel
end

described_class.get_current_scope.set_span(parent_span)
end

it "yields block with nil without explicit instrumenter" do
span = nil
executed = false

result = described_class.with_child_span do |child_span|
span = child_span
executed = true
"foobar"
end

expect(result).to eq("foobar")
expect(span).to eq(nil)
expect(executed).to eq(true)
end

it "records the child span with explicit instrumenter" do
child_span = nil

result = described_class.with_child_span(instrumenter: :otel, op: "child") do |span|
child_span = span
"foobar"
end

expect(result).to eq("foobar")
expect(child_span.parent_span_id).to eq(parent_span.span_id)
expect(child_span.timestamp).to be_a(Float)
end
end
end
end

Expand Down
16 changes: 16 additions & 0 deletions sentry-sidekiq/spec/sentry/sidekiq_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,22 @@ def retry_last_failed_job
event = transport.events.last
expect(event.contexts.dig(:trace, :trace_id)).to eq(transaction.contexts.dig(:trace, :trace_id))
end

context "with instrumenter :otel" do
before do
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
config.instrumenter = :otel
end
end

it "does not record transaction" do
execute_worker(processor, SadWorker)
expect(transport.events.count).to eq(1)
event = transport.events.first
expect(event).to be_a(Sentry::ErrorEvent)
end
end
end
end

0 comments on commit a62fb2d

Please sign in to comment.