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 before_send_transaction hook #1989

Merged
merged 1 commit into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,20 @@

- Add global event processor in OpenTelemetry `SpanProcessor` to link errors with transactions [#1983](https://github.com/getsentry/sentry-ruby/pull/1983)
- Fix some inconsistencies in setting name/op/status in OpenTelemetry `SpanProcessor` [#1987](https://github.com/getsentry/sentry-ruby/pull/1987)
- Add `config.before_send_transaction` hook [#1989](https://github.com/getsentry/sentry-ruby/pull/1989)

Users can now configure a `before_send_transaction` callback that runs similar to `before_send` but for transaction events.

```rb
config.before_send_transaction = lambda do |event, hint|
# skip unimportant transactions or strip sensitive data
if event.transaction == "/healthcheck/route"
nil
else
event
end
end
```

### Bug Fixes

Expand Down
10 changes: 10 additions & 0 deletions sentry-ruby/lib/sentry/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ def send_event(event, hint = nil)
end
end

if event_type == TransactionEvent::TYPE && configuration.before_send_transaction
event = configuration.before_send_transaction.call(event, hint)

if event.nil?
log_info("Discarded event because before_send_transaction returned nil")
transport.record_lost_event(:before_send, 'transaction')
return
end
end

transport.send_event(event)

event
Expand Down
20 changes: 20 additions & 0 deletions sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,19 @@ class Configuration
# @return [Proc]
attr_reader :before_send

# Optional Proc, called before sending an event to the server
# @example
# config.before_send_transaction = lambda do |event, hint|
# # skip unimportant transactions or strip sensitive data
# if event.transaction == "/healthcheck/route"
# nil
# else
# event
# end
# end
# @return [Proc]
attr_reader :before_send_transaction

# An array of breadcrumbs loggers to be used. Available options are:
# - :sentry_logger
# - :http_logger
Expand Down Expand Up @@ -287,6 +300,7 @@ def initialize
self.instrumenter = :sentry

self.before_send = nil
self.before_send_transaction = nil
self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT
self.traces_sample_rate = nil
self.traces_sampler = nil
Expand Down Expand Up @@ -338,6 +352,12 @@ def before_send=(value)
@before_send = value
end

def before_send_transaction=(value)
check_callable!("before_send_transaction", value)

@before_send_transaction = value
end

def before_breadcrumb=(value)
check_callable!("before_breadcrumb", value)

Expand Down
43 changes: 43 additions & 0 deletions sentry-ruby/spec/sentry/client/event_sending_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@
expect(event["tags"]["called"]).to eq(true)
end
end

it "doesn't apply before_send_transaction to Event" do
dbl = double("before_send_transaction")
allow(dbl).to receive(:call)
configuration.before_send_transaction = dbl

expect(dbl).not_to receive(:call)
subject.send_event(event)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid using double/mocking when it's not too hard to test it end-to-end? In my experience taking over the old SDK it makes things hard to maintain in the long-term.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because of the new codecov enforcing (which was turned on org-wide on github), it was complaining about a test line not being run. I will generally not use doubles, but in this case it makes sense.

end
end

it_behaves_like "Event in send_event" do
Expand All @@ -246,6 +255,26 @@

subject.send_event(event)
end

it "applies before_send_transaction callback before sending the event" do
configuration.before_send_transaction = lambda do |event, _hint|
if event.is_a?(Sentry::TransactionEvent)
event.tags[:called] = true
else
event["tags"]["called"] = true
end

event
end

subject.send_event(event)

if event.is_a?(Sentry::Event)
expect(event.tags[:called]).to eq(true)
else
expect(event["tags"]["called"]).to eq(true)
end
end
end

it_behaves_like "TransactionEvent in send_event" do
Expand Down Expand Up @@ -451,6 +480,20 @@
expect(subject.transport).to have_recorded_lost_event(:before_send, 'event')
end
end

context "before_send_transaction returns nil" do
before do
configuration.before_send_transaction = lambda do |_event, _hint|
nil
end
end

it "records lost event" do
transaction_event = subject.event_from_transaction(Sentry::Transaction.new(hub: hub))
subject.send_event(transaction_event)
expect(subject.transport).to have_recorded_lost_event(:before_send, 'transaction')
end
end
end
end
end
6 changes: 6 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@
expect { subject.before_send = true }.to raise_error(ArgumentError, "before_send must be callable (or nil to disable)")
end

it 'raises error when setting before_send_transaction to anything other than callable or nil' do
subject.before_send_transaction = -> {}
subject.before_send_transaction = nil
expect { subject.before_send_transaction = true }.to raise_error(ArgumentError, "before_send_transaction must be callable (or nil to disable)")
end

it 'raises error when setting before_breadcrumb to anything other than callable or nil' do
subject.before_breadcrumb = -> {}
subject.before_breadcrumb = nil
Expand Down