Skip to content

Commit

Permalink
Correct type attribute's usages (#1354)
Browse files Browse the repository at this point in the history
* Correct Transport's encoding logging

* Correct type declaration for events

Normal events shouldn't have the `type` field. Only transactions should
have `type: "transaction"`.

But in the logs, we should still print normal events' type as `"event"`.

For more information about the rules, please see https://github.com/getsentry/sentry-ruby/pull/1336/files#r596851969

* Update changelog
  • Loading branch information
st0012 authored Mar 26, 2021
1 parent 5337548 commit 86dcf3c
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 27 deletions.
2 changes: 1 addition & 1 deletion sentry-rails/spec/sentry/send_event_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@

expect(transport.events.count).to eq(1)
event = transport.events.first
expect(event.type).to eq("event")
expect(event.type).to eq(nil)
end

context "when ApplicationJob is not defined" do
Expand Down
5 changes: 5 additions & 0 deletions sentry-ruby/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## Unreleased

- Correct type attribute's usages [#1354](https://github.com/getsentry/sentry-ruby/pull/1354)


## 4.3.1

- Add Sentry.set_context helper [#1337](https://github.com/getsentry/sentry-ruby/pull/1337)
Expand Down
11 changes: 6 additions & 5 deletions sentry-ruby/lib/sentry/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def event_from_transaction(transaction)
def send_event(event, hint = nil)
event_type = event.is_a?(Event) ? event.type : event["type"]

if event_type == "event" && configuration.before_send
if event_type != TransactionEvent::TYPE && configuration.before_send
event = configuration.before_send.call(event, hint)

if event.nil?
Expand All @@ -85,8 +85,9 @@ def send_event(event, hint = nil)

event
rescue => e
logger.error(LOGGER_PROGNAME) { "#{event_type.capitalize} sending failed: #{e.message}" }
logger.error(LOGGER_PROGNAME) { "Unreported #{event_type.capitalize}: #{Event.get_log_message(event.to_hash)}" }
loggable_event_type = (event_type || "event").capitalize
logger.error(LOGGER_PROGNAME) { "#{loggable_event_type} sending failed: #{e.message}" }
logger.error(LOGGER_PROGNAME) { "Unreported #{loggable_event_type}: #{Event.get_log_message(event.to_hash)}" }
raise
end

Expand All @@ -110,8 +111,8 @@ def dispatch_async_event(async_block, event, hint)
async_block.call(event_hash)
end
rescue => e
event_type = event_hash["type"]
logger.error(LOGGER_PROGNAME) { "Async #{event_type} sending failed: #{e.message}" }
loggable_event_type = event_hash["type"] || "event"
logger.error(LOGGER_PROGNAME) { "Async #{loggable_event_type} sending failed: #{e.message}" }
send_event(event, hint)
end

Expand Down
1 change: 0 additions & 1 deletion sentry-ruby/lib/sentry/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ def rack_env=(env)
end

def type
"event"
end

def to_hash
Expand Down
4 changes: 3 additions & 1 deletion sentry-ruby/lib/sentry/transaction_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module Sentry
class TransactionEvent < Event
TYPE = "transaction"

ATTRIBUTES = %i(
event_id level timestamp start_timestamp
release environment server_name modules
Expand All @@ -17,7 +19,7 @@ def start_timestamp=(time)
end

def type
"transaction"
TYPE
end

def to_hash
Expand Down
27 changes: 10 additions & 17 deletions sentry-ruby/lib/sentry/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def send_event(event)
return
end

encoded_data = prepare_encoded_event(event)
encoded_data = encode(event)

return nil unless encoded_data

Expand All @@ -45,29 +45,22 @@ def generate_auth_header
'Sentry ' + fields.map { |key, value| "#{key}=#{value}" }.join(', ')
end

def encode(event_hash)
event_id = event_hash[:event_id] || event_hash['event_id']
event_type = event_hash[:type] || event_hash['type']
def encode(event)
# Convert to hash
event_hash = event.to_hash

event_id = event_hash[:event_id] || event_hash["event_id"]
item_type = event_hash[:type] || event_hash["type"] || "event"

envelope = <<~ENVELOPE
{"event_id":"#{event_id}","dsn":"#{configuration.dsn.to_s}","sdk":#{Sentry.sdk_meta.to_json},"sent_at":"#{Sentry.utc_now.iso8601}"}
{"type":"#{event_type}","content_type":"application/json"}
{"type":"#{item_type}","content_type":"application/json"}
#{JSON.generate(event_hash)}
ENVELOPE

envelope
end

private

def prepare_encoded_event(event)
# Convert to hash
event_hash = event.to_hash
configuration.logger.info(LOGGER_PROGNAME) { "Sending envelope [#{item_type}] #{event_id} to Sentry" }

event_id = event_hash[:event_id] || event_hash["event_id"]
event_type = event_hash[:type] || event_hash["type"]
configuration.logger.info(LOGGER_PROGNAME) { "Sending #{event_type} #{event_id} to Sentry" }
encode(event_hash)
envelope
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/rake_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
message = `cd spec/support && bundle exec rake raise_exception 2>&1`
end.join

expect(message).to match(/Sending event [abcdef0-9]+ to Sentry/)
expect(message).to match(/Sending envelope \[event\] [abcdef0-9]+ to Sentry/)
end
end
2 changes: 1 addition & 1 deletion sentry-ruby/spec/sentry/transport_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
expect(subject.send_event(event)).to eq(event)

expect(io.string).to match(
/INFO -- sentry: Sending event #{event.event_id} to Sentry/
/INFO -- sentry: Sending envelope \[event\] #{event.event_id} to Sentry/
)
end
end
Expand Down

0 comments on commit 86dcf3c

Please sign in to comment.