Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add transaction source annotation #1902

Merged
merged 19 commits into from
Sep 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
- in all outgoing requests in our net/http patch
- in Sentry transactions as [Dynamic Sampling Context](https://develop.sentry.dev/sdk/performance/dynamic-sampling-context/)
- Create new Baggage entries as Head SDK (originator of trace) [#1898](https://github.com/getsentry/sentry-ruby/pull/1898)
- Add Transaction source annotations to classify low quality (high cardinality) transaction names [#1902](https://github.com/getsentry/sentry-ruby/pull/1902)

### Bug Fixes

Expand Down
5 changes: 3 additions & 2 deletions sentry-delayed_job/lib/sentry/delayed_job/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ class Plugin < ::Delayed::Plugin

Sentry.with_scope do |scope|
contexts = generate_contexts(job)
scope.set_transaction_name(contexts.dig(ACTIVE_JOB_CONTEXT_KEY, :job_class) || contexts.dig(DELAYED_JOB_CONTEXT_KEY, :job_class))
name = contexts.dig(ACTIVE_JOB_CONTEXT_KEY, :job_class) || contexts.dig(DELAYED_JOB_CONTEXT_KEY, :job_class)
scope.set_transaction_name(name, source: :task)
scope.set_contexts(**contexts)
scope.set_tags("delayed_job.queue" => job.queue, "delayed_job.id" => job.id.to_s)

transaction = Sentry.start_transaction(name: scope.transaction_name, op: "delayed_job")
transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "delayed_job")
scope.set_span(transaction) if transaction

begin
Expand Down
14 changes: 8 additions & 6 deletions sentry-delayed_job/spec/sentry/delayed_job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def perform
config.rails.skippable_job_adapters << "ActiveJob::QueueAdapters::DelayedJobAdapter"
end
end

it "records transaction" do
ReportingJob.perform_later

Expand All @@ -257,13 +257,13 @@ def perform

expect(transport.events.count).to eq(2)
transaction = transport.events.last

expect(transaction.transaction).to eq("ReportingJob")
expect(transaction.contexts.dig(:trace, :trace_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :span_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
end

it "records transaction with exception" do
FailedJob.perform_later
enqueued_job = Delayed::Backend::ActiveRecord::Job.last
Expand All @@ -272,15 +272,15 @@ def perform
rescue ZeroDivisionError
nil
end

expect(transport.events.count).to eq(2)
transaction = transport.events.last

expect(transaction.transaction).to eq("FailedJob")
expect(transaction.contexts.dig(:trace, :trace_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :span_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :status)).to eq("internal_error")

event = transport.events.last
expect(event.contexts.dig(:trace, :trace_id)).to eq(transaction.contexts.dig(:trace, :trace_id))
end
Expand Down Expand Up @@ -325,6 +325,7 @@ def perform
transaction = transport.events.last

expect(transaction.transaction).to eq("Post#do_nothing")
expect(transaction.transaction_info).to eq({ source: :task })
expect(transaction.contexts.dig(:trace, :trace_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :span_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
Expand All @@ -343,6 +344,7 @@ def perform
transaction = transport.events.last

expect(transaction.transaction).to eq("Post#raise_error")
expect(transaction.transaction_info).to eq({ source: :task })
expect(transaction.contexts.dig(:trace, :trace_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :span_id)).to be_a(String)
expect(transaction.contexts.dig(:trace, :status)).to eq("internal_error")
Expand Down
8 changes: 4 additions & 4 deletions sentry-rails/lib/sentry/rails/action_cable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def capture(connection, transaction_name:, extra_context: nil, &block)
Sentry.with_scope do |scope|
scope.set_rack_env(env)
scope.set_context("action_cable", extra_context) if extra_context
scope.set_transaction_name(transaction_name)
transaction = start_transaction(env, scope.transaction_name)
scope.set_transaction_name(transaction_name, source: :view)
sl0thentr0py marked this conversation as resolved.
Show resolved Hide resolved
transaction = start_transaction(env, scope)
scope.set_span(transaction) if transaction

begin
Expand All @@ -29,11 +29,11 @@ def capture(connection, transaction_name:, extra_context: nil, &block)
end
end

def start_transaction(env, transaction_name)
def start_transaction(env, scope)
sentry_trace = env["HTTP_SENTRY_TRACE"]
baggage = env["HTTP_BAGGAGE"]

options = { name: transaction_name, op: "rails.action_cable".freeze }
options = { name: scope.transaction_name, source: scope.transaction_source, op: "rails.action_cable".freeze }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, **options)
end
Expand Down
4 changes: 2 additions & 2 deletions sentry-rails/lib/sentry/rails/active_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ class << self
def record(job, &block)
Sentry.with_scope do |scope|
begin
scope.set_transaction_name(job.class.name)
scope.set_transaction_name(job.class.name, source: :task)
transaction =
if job.is_a?(::Sentry::SendEventJob)
nil
else
Sentry.start_transaction(name: scope.transaction_name, op: "active_job")
Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "active_job")
end

scope.set_span(transaction) if transaction
Expand Down
9 changes: 1 addition & 8 deletions sentry-rails/lib/sentry/rails/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,6 @@ def capture_exception(exception, env)

# the exception will be swallowed by ShowExceptions middleware
return if request.show_exceptions? && !Sentry.configuration.rails.report_rescued_exceptions

current_scope = Sentry.get_current_scope

if original_transaction = env["sentry.original_transaction"]
current_scope.set_transaction_name(original_transaction)
end

Sentry::Rails.capture_exception(exception).tap do |event|
env[ERROR_EVENT_ID_KEY] = event.event_id if event
end
Expand All @@ -41,7 +34,7 @@ def start_transaction(env, scope)
sentry_trace = env["HTTP_SENTRY_TRACE"]
baggage = env["HTTP_BAGGAGE"]

options = { name: scope.transaction_name, op: transaction_op }
options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op }

if @assets_regex && scope.transaction_name.match?(@assets_regex)
options.merge!(sampled: false)
Expand Down
2 changes: 1 addition & 1 deletion sentry-rails/lib/sentry/rails/controller_transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module Rails
module ControllerTransaction
def self.included(base)
base.prepend_before_action do |controller|
Sentry.get_current_scope.set_transaction_name("#{controller.class}##{controller.action_name}")
Sentry.get_current_scope.set_transaction_name("#{controller.class}##{controller.action_name}", source: :view)
end
end
end
Expand Down
12 changes: 12 additions & 0 deletions sentry-rails/spec/sentry/rails/action_cable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def disconnect

event = transport.events.last.to_json_compatible
expect(event["transaction"]).to eq("FailToOpenConnection#connect")
expect(event["transaction_info"]).to eq({ "source" => "view" })
end
end

Expand All @@ -89,6 +90,7 @@ def disconnect

event = transport.events.last.to_json_compatible
expect(event["transaction"]).to eq("FailToCloseConnection#disconnect")
expect(event["transaction_info"]).to eq({ "source" => "view" })
end
end
end
Expand All @@ -105,6 +107,7 @@ def disconnect

event = transport.events.last.to_json_compatible
expect(event["transaction"]).to eq("ChatChannel#subscribed")
expect(event["transaction_info"]).to eq({ "source" => "view" })
expect(event["contexts"]).to include("action_cable" => { "params" => { "room_id" => 42 } })
end
end
Expand All @@ -119,6 +122,7 @@ def disconnect
event = transport.events.last.to_json_compatible

expect(event["transaction"]).to eq("AppearanceChannel#appear")
expect(event["transaction_info"]).to eq({ "source" => "view" })
expect(event["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 },
Expand All @@ -134,6 +138,7 @@ def disconnect
event = transport.events.last.to_json_compatible

expect(event["transaction"]).to eq("AppearanceChannel#unsubscribed")
expect(event["transaction_info"]).to eq({ "source" => "view" })
expect(event["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand Down Expand Up @@ -164,6 +169,7 @@ def disconnect

expect(transaction["type"]).to eq("transaction")
expect(transaction["transaction"]).to eq("ChatChannel#subscribed")
expect(transaction["transaction_info"]).to eq({ "source" => "view" })
expect(transaction["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand All @@ -189,6 +195,7 @@ def disconnect

expect(subscription_transaction["type"]).to eq("transaction")
expect(subscription_transaction["transaction"]).to eq("AppearanceChannel#subscribed")
expect(subscription_transaction["transaction_info"]).to eq({ "source" => "view" })
expect(subscription_transaction["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand All @@ -204,6 +211,7 @@ def disconnect
event = transport.events[1].to_json_compatible

expect(event["transaction"]).to eq("AppearanceChannel#appear")
expect(event["transaction_info"]).to eq({ "source" => "view" })
expect(event["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 },
Expand All @@ -215,6 +223,7 @@ def disconnect

expect(action_transaction["type"]).to eq("transaction")
expect(action_transaction["transaction"]).to eq("AppearanceChannel#appear")
expect(action_transaction["transaction_info"]).to eq({ "source" => "view" })
expect(action_transaction["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 },
Expand All @@ -237,6 +246,7 @@ def disconnect

expect(subscription_transaction["type"]).to eq("transaction")
expect(subscription_transaction["transaction"]).to eq("AppearanceChannel#subscribed")
expect(subscription_transaction["transaction_info"]).to eq({ "source" => "view" })
expect(subscription_transaction["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand All @@ -252,6 +262,7 @@ def disconnect
event = transport.events[1].to_json_compatible

expect(event["transaction"]).to eq("AppearanceChannel#unsubscribed")
expect(event["transaction_info"]).to eq({ "source" => "view" })
expect(event["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand All @@ -262,6 +273,7 @@ def disconnect

expect(transaction["type"]).to eq("transaction")
expect(transaction["transaction"]).to eq("AppearanceChannel#unsubscribed")
expect(transaction["transaction_info"]).to eq({ "source" => "view" })
expect(transaction["contexts"]).to include(
"action_cable" => {
"params" => { "room_id" => 42 }
Expand Down
2 changes: 2 additions & 0 deletions sentry-rails/spec/sentry/rails/activejob_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ def post.to_global_id
expect(transport.events.count).to eq(1)
transaction = transport.events.last
expect(transaction.transaction).to eq("QueryPostJob")
expect(transaction.transaction_info).to eq({ source: :task })
expect(transaction.contexts.dig(:trace, :trace_id)).to be_present
expect(transaction.contexts.dig(:trace, :span_id)).to be_present
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
Expand All @@ -182,6 +183,7 @@ def post.to_global_id

transaction = transport.events.first
expect(transaction.transaction).to eq("FailedWithExtraJob")
expect(transaction.transaction_info).to eq({ source: :task })
expect(transaction.contexts.dig(:trace, :trace_id)).to be_present
expect(transaction.contexts.dig(:trace, :span_id)).to be_present
expect(transaction.contexts.dig(:trace, :status)).to eq("internal_error")
Expand Down
6 changes: 5 additions & 1 deletion sentry-rails/spec/sentry/rails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@
expect(Sentry.configuration.release).to eq('beta')
end

it "sets transaction to ControllerName#method" do
it "sets transaction to ControllerName#method and sets correct source" do
get "/exception"

expect(transport.events.last.transaction).to eq("HelloController#exception")
expect(transport.events.last.transaction_info).to eq({ source: :view })

get "/posts"

expect(transport.events.last.transaction).to eq("PostsController#index")
expect(transport.events.last.transaction_info).to eq({ source: :view })
end

it "sets correct request url" do
Expand Down Expand Up @@ -196,11 +198,13 @@
expect(transport.events.count).to eq(1)
last_event = transport.events.last
expect(last_event.transaction).to eq("HelloController#exception")
expect(transport.events.last.transaction_info).to eq({ source: :view })
expect(response.body).to match(last_event.event_id)

get "/posts"

expect(transport.events.last.transaction).to eq("PostsController#index")
expect(transport.events.last.transaction_info).to eq({ source: :view })
end

it "sets correct request url" do
Expand Down
5 changes: 3 additions & 2 deletions sentry-resque/lib/sentry/resque.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ def record(queue, worker, payload, &block)
scope.set_contexts(**contexts)
scope.set_tags("resque.queue" => queue)

scope.set_transaction_name(contexts.dig(:"Active-Job", :job_class) || contexts.dig(:"Resque", :job_class))
transaction = Sentry.start_transaction(name: scope.transaction_name, op: "resque")
name = contexts.dig(:"Active-Job", :job_class) || contexts.dig(:"Resque", :job_class)
scope.set_transaction_name(name, source: :task)
transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "resque")
scope.set_span(transaction) if transaction

yield
Expand Down
2 changes: 2 additions & 0 deletions sentry-resque/spec/sentry/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def self.perform(msg)

tracing_event = transport.events.last.to_hash
expect(tracing_event[:transaction]).to eq("MessageJob")
expect(tracing_event[:transaction_info]).to eq({ source: :task })
expect(tracing_event[:type]).to eq("transaction")
expect(tracing_event.dig(:contexts, :trace, :status)).to eq("ok")
expect(tracing_event.dig(:contexts, :trace, :op)).to eq("resque")
Expand All @@ -54,6 +55,7 @@ def self.perform(msg)

tracing_event = transport.events.last.to_hash
expect(tracing_event[:transaction]).to eq("FailedJob")
expect(tracing_event[:transaction_info]).to eq({ source: :task })
expect(tracing_event[:type]).to eq("transaction")
expect(tracing_event.dig(:contexts, :trace, :status)).to eq("internal_error")
expect(tracing_event.dig(:contexts, :trace, :op)).to eq("resque")
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Event
event_id level timestamp
release environment server_name modules
message user tags contexts extra
fingerprint breadcrumbs transaction
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a TransactionEvent-only attribute? If that's the case, can we put it there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had it there first, but then moved it here because it's always coupled to the transaction field.

fingerprint breadcrumbs transaction transaction_info
platform sdk type
)

Expand Down
4 changes: 2 additions & 2 deletions sentry-ruby/lib/sentry/rack/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def call(env)
Sentry.with_scope do |scope|
Sentry.with_session_tracking do
scope.clear_breadcrumbs
scope.set_transaction_name(env["PATH_INFO"]) if env["PATH_INFO"]
scope.set_transaction_name(env["PATH_INFO"], source: :url) if env["PATH_INFO"]
scope.set_rack_env(env)

transaction = start_transaction(env, scope)
Expand Down Expand Up @@ -65,7 +65,7 @@ def start_transaction(env, scope)
sentry_trace = env["HTTP_SENTRY_TRACE"]
baggage = env["HTTP_BAGGAGE"]

options = { name: scope.transaction_name, op: transaction_op }
options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, custom_sampling_context: { env: env }, **options)
end
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module Application
def display_error_message(ex)
Sentry.capture_exception(ex) do |scope|
task_name = top_level_tasks.join(' ')
scope.set_transaction_name(task_name)
scope.set_transaction_name(task_name, source: :task)
scope.set_tag("rake_task", task_name)
end if Sentry.initialized? && !Sentry.configuration.skip_rake_integration

Expand Down
Loading