Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Transaction#set_context api #1947

Merged
merged 6 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
## 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)
- Expose `span_id` in `Span` constructor [#1945](https://github.com/getsentry/sentry-ruby/pull/1945)
- Expose `end_timestamp` in `Span#finish` and `Transaction#finish` [#1946](https://github.com/getsentry/sentry-ruby/pull/1946)
- Add `Transaction#set_context` api [#1947](https://github.com/getsentry/sentry-ruby/pull/1947)

## 5.6.0

### Features
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
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
7 changes: 4 additions & 3 deletions sentry-ruby/lib/sentry/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,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
Expand All @@ -89,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

Expand Down
56 changes: 43 additions & 13 deletions sentry-ruby/lib/sentry/transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -60,8 +64,7 @@ def initialize(
)
super(transaction: self, **options)

@name = name
@source = SOURCES.include?(source) ? source.to_sym : :custom
set_name(name, source: source)
@parent_sampled = parent_sampled
@hub = hub
@baggage = baggage
Expand All @@ -74,6 +77,7 @@ def initialize(
@environment = hub.configuration.environment
@dsn = hub.configuration.dsn
@effective_sample_rate = nil
@contexts = {}
init_span_recorder
end

Expand All @@ -91,16 +95,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)
Expand All @@ -123,6 +121,20 @@ def self.from_sentry_trace(sentry_trace, baggage: nil, hub: Sentry.get_current_h
)
end

# Extract the trace_id, parent_span_id and parent_sampled values from a sentry-trace header.
#
# @param sentry_trace [String] the sentry-trace header value from the previous transaction.
# @return [Array, nil]
def self.extract_sentry_trace(sentry_trace)
match = SENTRY_TRACE_REGEXP.match(sentry_trace)
return nil if match.nil?

trace_id, parent_span_id, sampled_flag = match[1..3]
parent_sampled = sampled_flag.nil? ? nil : sampled_flag != "0"

[trace_id, parent_span_id, parent_sampled]
end

# @return [Hash]
def to_hash
hash = super
Expand Down Expand Up @@ -210,7 +222,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
Expand All @@ -222,7 +234,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
Expand All @@ -244,6 +256,24 @@ def get_baggage
@baggage
end

# Set the transaction name directly.
# Considered internal api since it bypasses the usual scope logic.
# @param name [String]
# @param source [Symbol]
# @return [void]
def set_name(name, source: :custom)
sl0thentr0py marked this conversation as resolved.
Show resolved Hide resolved
@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)
Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/lib/sentry/transaction_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions sentry-ruby/spec/sentry/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,12 @@ def sentry_context
"trace_id" => transaction.trace_id
})
end

it "adds explicitly added contexts to event" do
transaction.set_context(:foo, { bar: 42 })
event = subject.event_from_transaction(transaction)
expect(event.contexts).to include({ foo: { bar: 42 } })
end
end

describe "#event_from_exception" do
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
19 changes: 19 additions & 0 deletions sentry-ruby/spec/sentry/hub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,25 @@
end
end

context "when event is a transaction" do
it "scope.set_context merges and takes precedence over transaction.set_context" do
scope.set_context(:foo, { val: 42 })
scope.set_context(:bar, { val: 43 })

transaction = Sentry::Transaction.new(hub: subject, name: 'test')
transaction.set_context(:foo, { val: 44 })
transaction.set_context(:baz, { val: 45 })

transaction_event = subject.current_client.event_from_transaction(transaction)
subject.capture_event(transaction_event)

event = transport.events.last
expect(event.contexts[:foo]). to eq({ val: 44 })
expect(event.contexts[:bar]). to eq({ val: 43 })
expect(event.contexts[:baz]). to eq({ val: 45 })
end
end

it_behaves_like "capture_helper" do
let(:capture_helper) { :capture_event }
let(:capture_subject) { event }
Expand Down
8 changes: 8 additions & 0 deletions sentry-ruby/spec/sentry/span_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,14 @@
expect(span_2.transaction).to eq(subject.transaction)
end

it "initializes a new child Span with explicit span id" do
span_id = SecureRandom.hex(8)
new_span = subject.start_child(op: "foo", span_id: span_id)

expect(new_span.op).to eq("foo")
expect(new_span.span_id).to eq(span_id)
end

context "when the parent span has a span_recorder" do
subject do
# inherits the span recorder from the transaction
Expand Down
25 changes: 25 additions & 0 deletions sentry-ruby/spec/sentry/transaction_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,16 @@
expect(event[:transaction]).to eq("foo")
end

it "finishes the transaction with explicit timestamp" do
timestamp = Sentry.utc_now.to_f
subject.finish(end_timestamp: timestamp)

expect(events.count).to eq(1)
event = events.last.to_hash

expect(event[:timestamp]).to eq(timestamp)
end

it "assigns the transaction's tags" do
Sentry.set_tags(name: "apple")

Expand Down Expand Up @@ -549,4 +559,19 @@
end
end
end

describe "#set_name" do
it "sets name and source directly" do
subject.set_name("bar", source: :url)
expect(subject.name).to eq("bar")
expect(subject.source).to eq(:url)
end
end

describe "#set_context" do
it "sets arbitrary context" do
subject.set_context(:foo, { bar: 42 })
expect(subject.contexts).to eq({ foo: { bar: 42 } })
end
end
end
Loading