Skip to content

Commit

Permalink
Avoid leaking tracing timestamp to breadcrumbs (#1575)
Browse files Browse the repository at this point in the history
* Require debugger in specs

* Don't record 'start_timestamp' attribute in breadcrumbs

* Update changelog
  • Loading branch information
st0012 committed Sep 17, 2021
1 parent fddb235 commit a6ce224
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 6 deletions.
4 changes: 4 additions & 0 deletions 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)
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
@@ -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|
Expand Down
4 changes: 3 additions & 1 deletion 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
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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|
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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
1 change: 1 addition & 0 deletions sentry-rails/spec/spec_helper.rb
@@ -1,4 +1,5 @@
require "bundler/setup"
require "debug"
require "pry"

require "sentry-ruby"
Expand Down

0 comments on commit a6ce224

Please sign in to comment.