diff --git a/CHANGELOG.md b/CHANGELOG.md index e08709a7c..8a9e8941a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased + +- Avoid leaking tracing timestamp to breadcrumbs [#1575](https://github.com/getsentry/sentry-ruby/pull/1575) + ## 4.7.2 - Change default environment to 'development' [#1565](https://github.com/getsentry/sentry-ruby/pull/1565) diff --git a/sentry-rails/lib/sentry/rails.rb b/sentry-rails/lib/sentry/rails.rb index a5b895e02..14c8e1b80 100644 --- a/sentry-rails/lib/sentry/rails.rb +++ b/sentry-rails/lib/sentry/rails.rb @@ -1,10 +1,10 @@ require "rails" require "sentry-ruby" require "sentry/integrable" +require "sentry/rails/tracing" require "sentry/rails/configuration" require "sentry/rails/engine" require "sentry/rails/railtie" -require "sentry/rails/tracing" module Sentry module Rails diff --git a/sentry-rails/lib/sentry/rails/instrument_payload_cleanup_helper.rb b/sentry-rails/lib/sentry/rails/instrument_payload_cleanup_helper.rb index c4ce8833b..343d2d41c 100644 --- a/sentry-rails/lib/sentry/rails/instrument_payload_cleanup_helper.rb +++ b/sentry-rails/lib/sentry/rails/instrument_payload_cleanup_helper.rb @@ -1,7 +1,7 @@ module Sentry module Rails module InstrumentPayloadCleanupHelper - IGNORED_DATA_TYPES = [:request, :response, :headers, :exception, :exception_object] + IGNORED_DATA_TYPES = [:request, :response, :headers, :exception, :exception_object, Tracing::START_TIMESTAMP_NAME] def cleanup_data(data) IGNORED_DATA_TYPES.each do |key| diff --git a/sentry-rails/lib/sentry/rails/tracing.rb b/sentry-rails/lib/sentry/rails/tracing.rb index c1df3a113..e4bf52b7a 100644 --- a/sentry-rails/lib/sentry/rails/tracing.rb +++ b/sentry-rails/lib/sentry/rails/tracing.rb @@ -1,6 +1,8 @@ module Sentry module Rails module Tracing + START_TIMESTAMP_NAME = :sentry_start_timestamp + def self.register_subscribers(subscribers) @subscribers = subscribers end @@ -37,7 +39,7 @@ def self.patch_active_support_notifications def instrument(name, payload = {}, &block) is_public_event = name[0] != "!" - payload[:start_timestamp] = Time.now.utc.to_f if is_public_event + payload[START_TIMESTAMP_NAME] = Time.now.utc.to_f if is_public_event super(name, payload, &block) end diff --git a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb index 31b3e0b74..3d69a0ac3 100644 --- a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb @@ -16,7 +16,7 @@ def self.subscribe! record_on_current_span( op: event_name, - start_timestamp: payload[:start_timestamp], + start_timestamp: payload[START_TIMESTAMP_NAME], description: "#{controller}##{action}", duration: duration ) do |span| diff --git a/sentry-rails/lib/sentry/rails/tracing/action_view_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/action_view_subscriber.rb index 1a31e3e37..5c0c429a2 100644 --- a/sentry-rails/lib/sentry/rails/tracing/action_view_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/action_view_subscriber.rb @@ -8,7 +8,7 @@ class ActionViewSubscriber < AbstractSubscriber def self.subscribe! subscribe_to_event(EVENT_NAME) do |event_name, duration, payload| - record_on_current_span(op: event_name, start_timestamp: payload[:start_timestamp], description: payload[:identifier], duration: duration) + record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:identifier], duration: duration) end end end diff --git a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb index d4486266b..262991736 100644 --- a/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb +++ b/sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb @@ -11,7 +11,7 @@ def self.subscribe! subscribe_to_event(EVENT_NAME) do |event_name, duration, payload| next if EXCLUDED_EVENTS.include? payload[:name] - record_on_current_span(op: event_name, start_timestamp: payload[:start_timestamp], description: payload[:sql], duration: duration) do |span| + record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:sql], duration: duration) do |span| span.set_data(:connection_id, payload[:connection_id]) end end diff --git a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_breadcrumbs_spec.rb b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_breadcrumbs_spec.rb index 656d49cce..93914fe31 100644 --- a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_breadcrumbs_spec.rb +++ b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_breadcrumbs_spec.rb @@ -92,4 +92,18 @@ expect(breadcrumb_buffer.count).to be_zero end + + context "when used with tracing" do + it "doesn't add internal start timestamp payload to breadcrumbs data" do + p = Post.create! + + get "/posts/#{p.id}" + + expect(transport.events.count).to eq(1) + + transaction = transport.events.last.to_hash + breadcrumbs = transaction[:breadcrumbs][:values] + expect(breadcrumbs.last[:data].has_key?(Sentry::Rails::Tracing::START_TIMESTAMP_NAME)).to eq(false) + end + end end diff --git a/sentry-rails/spec/spec_helper.rb b/sentry-rails/spec/spec_helper.rb index b3837d232..d19f9257a 100644 --- a/sentry-rails/spec/spec_helper.rb +++ b/sentry-rails/spec/spec_helper.rb @@ -1,4 +1,5 @@ require "bundler/setup" +require "debug" require "pry" require "sentry-ruby"