From 86dcf3cb43cebe322029b512c8469aac7771d891 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Fri, 26 Mar 2021 18:21:20 +0800 Subject: [PATCH] Correct type attribute's usages (#1354) * 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 --- .../spec/sentry/send_event_job_spec.rb | 2 +- sentry-ruby/CHANGELOG.md | 5 ++++ sentry-ruby/lib/sentry/client.rb | 11 ++++---- sentry-ruby/lib/sentry/event.rb | 1 - sentry-ruby/lib/sentry/transaction_event.rb | 4 ++- sentry-ruby/lib/sentry/transport.rb | 27 +++++++------------ sentry-ruby/spec/sentry/rake_spec.rb | 2 +- sentry-ruby/spec/sentry/transport_spec.rb | 2 +- 8 files changed, 27 insertions(+), 27 deletions(-) diff --git a/sentry-rails/spec/sentry/send_event_job_spec.rb b/sentry-rails/spec/sentry/send_event_job_spec.rb index c50130f65..479a439e5 100644 --- a/sentry-rails/spec/sentry/send_event_job_spec.rb +++ b/sentry-rails/spec/sentry/send_event_job_spec.rb @@ -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 diff --git a/sentry-ruby/CHANGELOG.md b/sentry-ruby/CHANGELOG.md index 4f35bcac0..adff5824a 100644 --- a/sentry-ruby/CHANGELOG.md +++ b/sentry-ruby/CHANGELOG.md @@ -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) diff --git a/sentry-ruby/lib/sentry/client.rb b/sentry-ruby/lib/sentry/client.rb index 4c5ead7a5..04f12f2a9 100644 --- a/sentry-ruby/lib/sentry/client.rb +++ b/sentry-ruby/lib/sentry/client.rb @@ -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? @@ -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 @@ -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 diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index 9838cc2ee..768195e89 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -100,7 +100,6 @@ def rack_env=(env) end def type - "event" end def to_hash diff --git a/sentry-ruby/lib/sentry/transaction_event.rb b/sentry-ruby/lib/sentry/transaction_event.rb index 39000004a..ff5b00459 100644 --- a/sentry-ruby/lib/sentry/transaction_event.rb +++ b/sentry-ruby/lib/sentry/transaction_event.rb @@ -2,6 +2,8 @@ module Sentry class TransactionEvent < Event + TYPE = "transaction" + ATTRIBUTES = %i( event_id level timestamp start_timestamp release environment server_name modules @@ -17,7 +19,7 @@ def start_timestamp=(time) end def type - "transaction" + TYPE end def to_hash diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb index 5c2f627df..f677d92d2 100644 --- a/sentry-ruby/lib/sentry/transport.rb +++ b/sentry-ruby/lib/sentry/transport.rb @@ -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 @@ -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 diff --git a/sentry-ruby/spec/sentry/rake_spec.rb b/sentry-ruby/spec/sentry/rake_spec.rb index e74994c2d..e764fe55d 100644 --- a/sentry-ruby/spec/sentry/rake_spec.rb +++ b/sentry-ruby/spec/sentry/rake_spec.rb @@ -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 diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index 3fd890b14..e72608682 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -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