Skip to content

Commit

Permalink
Don't add most scope data to CheckInEvent (#2217)
Browse files Browse the repository at this point in the history
  • Loading branch information
sl0thentr0py committed Jan 8, 2024
1 parent 918b83d commit e02c5fd
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
```
- Clean up logging [#2216](https://github.com/getsentry/sentry-ruby/pull/2216)
- Pick up config.cron.default_timezone from Rails config [#2213](https://github.com/getsentry/sentry-ruby/pull/2213)
- Don't add most scope data (tags/extra/breadcrumbs) to `CheckInEvent` [#2217](https://github.com/getsentry/sentry-ruby/pull/2217)

## 5.15.2

Expand Down
6 changes: 2 additions & 4 deletions sentry-ruby/lib/sentry-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -446,12 +446,10 @@ def capture_event(event)
# @option options [Integer] duration seconds elapsed since this monitor started
# @option options [Cron::MonitorConfig] monitor_config configuration for this monitor
#
# @yieldparam scope [Scope]
#
# @return [String, nil] The {CheckInEvent#check_in_id} to use for later updates on the same slug
def capture_check_in(slug, status, **options, &block)
def capture_check_in(slug, status, **options)
return unless initialized?
get_current_hub.capture_check_in(slug, status, **options, &block)
get_current_hub.capture_check_in(slug, status, **options)
end

# Takes or initializes a new Sentry::Transaction and makes a sampling decision for it.
Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/lib/sentry/hub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def capture_message(message, **options, &block)
capture_event(event, **options, &block)
end

def capture_check_in(slug, status, **options, &block)
def capture_check_in(slug, status, **options)
check_argument_type!(slug, ::String)
check_argument_includes!(status, Sentry::CheckInEvent::VALID_STATUSES)

Expand All @@ -176,7 +176,7 @@ def capture_check_in(slug, status, **options, &block)

return unless event

capture_event(event, **options, &block)
capture_event(event, **options)
event.check_in_id
end

Expand Down
23 changes: 12 additions & 11 deletions sentry-ruby/lib/sentry/scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,18 @@ def clear
# @param hint [Hash] the hint data that'll be passed to event processors.
# @return [Event]
def apply_to_event(event, hint = nil)
event.tags = tags.merge(event.tags)
event.user = user.merge(event.user)
event.extra = extra.merge(event.extra)
event.contexts = contexts.merge(event.contexts)
event.transaction = transaction_name if transaction_name
event.transaction_info = { source: transaction_source } if transaction_source
unless event.is_a?(CheckInEvent)
event.tags = tags.merge(event.tags)
event.user = user.merge(event.user)
event.extra = extra.merge(event.extra)
event.contexts = contexts.merge(event.contexts)
event.transaction = transaction_name if transaction_name
event.transaction_info = { source: transaction_source } if transaction_source
event.fingerprint = fingerprint
event.level = level
event.breadcrumbs = breadcrumbs
event.rack_env = rack_env if rack_env
end

if span
event.contexts[:trace] ||= span.get_trace_context
Expand All @@ -58,11 +64,6 @@ def apply_to_event(event, hint = nil)
event.dynamic_sampling_context ||= propagation_context.get_dynamic_sampling_context
end

event.fingerprint = fingerprint
event.level = level
event.breadcrumbs = breadcrumbs
event.rack_env = rack_env if rack_env

all_event_processors = self.class.global_event_processors + @event_processors

unless all_event_processors.empty?
Expand Down
13 changes: 10 additions & 3 deletions sentry-ruby/spec/sentry/hub_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,16 @@
expect(event[:monitor_config]).to include({ schedule: { type: :crontab, value: "* * * * *" } })
end

it_behaves_like "capture_helper" do
let(:capture_helper) { :capture_check_in }
let(:capture_subject) { [slug, :ok] }
context "with sending not allowed" do
before do
expect(configuration).to receive(:sending_allowed?).and_return(false)
end

it "doesn't send the event nor assign last_event_id" do
subject.capture_check_in(slug, :ok)
expect(transport.events).to be_empty
expect(subject.last_event_id).to eq(nil)
end
end
end

Expand Down
18 changes: 15 additions & 3 deletions sentry-ruby/spec/sentry/scope_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,8 @@
scope
end

let(:event) do
client.event_from_message("test message")
end
let(:event) { client.event_from_message("test message") }
let(:check_in_event) { client.event_from_check_in("test_slug", :ok) }

it "applies the contextual data to event" do
subject.apply_to_event(event)
Expand All @@ -218,10 +217,23 @@
expect(event.transaction_info).to eq({ source: :view })
expect(event.breadcrumbs).to be_a(Sentry::BreadcrumbBuffer)
expect(event.fingerprint).to eq(["foo"])
expect(event.contexts).to include(:trace)
expect(event.contexts[:os].keys).to match_array([:name, :version, :build, :kernel_version, :machine])
expect(event.contexts.dig(:runtime, :version)).to match(/ruby/)
end

it "does not apply the contextual data to a check-in event" do
subject.apply_to_event(check_in_event)
expect(check_in_event.tags).to eq({})
expect(check_in_event.user).to eq({})
expect(check_in_event.extra).to eq({})
expect(check_in_event.transaction).to eq(nil)
expect(check_in_event.transaction_info).to eq(nil)
expect(check_in_event.breadcrumbs).to eq(nil)
expect(check_in_event.fingerprint).to eq([])
expect(check_in_event.contexts).to include(:trace)
end

it "doesn't override event's pre-existing data" do
event.tags = {foo: "baz"}
event.user = {id: 2}
Expand Down

0 comments on commit e02c5fd

Please sign in to comment.