Skip to content

Commit

Permalink
Add origin to Rails
Browse files Browse the repository at this point in the history
  • Loading branch information
sl0thentr0py committed Jun 6, 2024
1 parent 225b86a commit 5c988f8
Show file tree
Hide file tree
Showing 16 changed files with 84 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@
- Decrease the default number of background worker threads by half ([#2305](https://github.com/getsentry/sentry-ruby/pull/2305))
- Fixes [#2297](https://github.com/getsentry/sentry-ruby/issues/2297)

### Internal

- Add `origin` to spans and transactions to track integration sources for instrumentation ([#2319](https://github.com/getsentry/sentry-ruby/pull/2319))

## 5.17.3

### Internal
Expand Down
9 changes: 8 additions & 1 deletion sentry-rails/lib/sentry/rails/action_cable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Rails
module ActionCableExtensions
class ErrorHandler
OP_NAME = "websocket.server".freeze
SPAN_ORIGIN = "auto.http.rails.actioncable"

class << self
def capture(connection, transaction_name:, extra_context: nil, &block)
Expand Down Expand Up @@ -33,7 +34,13 @@ def capture(connection, transaction_name:, extra_context: nil, &block)
end

def start_transaction(env, scope)
options = { name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME }
options = {
name: scope.transaction_name,
source: scope.transaction_source,
op: OP_NAME,
origin: SPAN_ORIGIN
}

transaction = Sentry.continue_trace(env, **options)
Sentry.start_transaction(transaction: transaction, **options)
end
Expand Down
8 changes: 7 additions & 1 deletion sentry-rails/lib/sentry/rails/active_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def already_supported_by_sentry_integration?

class SentryReporter
OP_NAME = "queue.active_job".freeze
SPAN_ORIGIN = "auto.queue.active_job".freeze

class << self
def record(job, &block)
Expand All @@ -27,7 +28,12 @@ def record(job, &block)
if job.is_a?(::Sentry::SendEventJob)
nil
else
Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME)
Sentry.start_transaction(
name: scope.transaction_name,
source: scope.transaction_source,
op: OP_NAME,
origin: SPAN_ORIGIN
)
end

scope.set_span(transaction) if transaction
Expand Down
8 changes: 7 additions & 1 deletion sentry-rails/lib/sentry/rails/capture_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Sentry
module Rails
class CaptureExceptions < Sentry::Rack::CaptureExceptions
RAILS_7_1 = Gem::Version.new(::Rails.version) >= Gem::Version.new("7.1.0.alpha")
SPAN_ORIGIN = 'auto.http.rails'.freeze

def initialize(_)
super
Expand Down Expand Up @@ -32,7 +33,12 @@ def capture_exception(exception, env)
end

def start_transaction(env, scope)
options = { name: scope.transaction_name, source: scope.transaction_source, op: transaction_op }
options = {
name: scope.transaction_name,
source: scope.transaction_source,
op: transaction_op,
origin: SPAN_ORIGIN
}

if @assets_regexp && scope.transaction_name.match?(@assets_regexp)
options.merge!(sampled: false)
Expand Down
4 changes: 3 additions & 1 deletion sentry-rails/lib/sentry/rails/controller_transaction.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
module Sentry
module Rails
module ControllerTransaction
SPAN_ORIGIN = 'auto.view.rails'.freeze

def self.included(base)
base.prepend_around_action(:sentry_around_action)
end
Expand All @@ -11,7 +13,7 @@ def sentry_around_action
if Sentry.initialized?
transaction_name = "#{self.class}##{action_name}"
Sentry.get_current_scope.set_transaction_name(transaction_name, source: :view)
Sentry.with_child_span(op: "view.process_action.action_controller", description: transaction_name) do |child_span|
Sentry.with_child_span(op: "view.process_action.action_controller", description: transaction_name, origin: SPAN_ORIGIN) do |child_span|
if child_span
begin
result = yield
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ActionControllerSubscriber < AbstractSubscriber

EVENT_NAMES = ["process_action.action_controller"].freeze
OP_NAME = "view.process_action.action_controller".freeze
SPAN_ORIGIN = "auto.view.rails".freeze

def self.subscribe!
Sentry.logger.warn <<~MSG
Expand All @@ -22,6 +23,7 @@ def self.subscribe!

record_on_current_span(
op: OP_NAME,
origin: SPAN_ORIGIN,
start_timestamp: payload[START_TIMESTAMP_NAME],
description: "#{controller}##{action}",
duration: duration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ module Tracing
class ActionViewSubscriber < AbstractSubscriber
EVENT_NAMES = ["render_template.action_view"].freeze
SPAN_PREFIX = "template.".freeze
SPAN_ORIGIN = "auto.template.rails".freeze

def self.subscribe!
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
record_on_current_span(op: SPAN_PREFIX + event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:identifier], duration: duration)
record_on_current_span(
op: SPAN_PREFIX + event_name,
origin: SPAN_ORIGIN,
start_timestamp: payload[START_TIMESTAMP_NAME],
description: payload[:identifier],
duration: duration
)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module Tracing
class ActiveRecordSubscriber < AbstractSubscriber
EVENT_NAMES = ["sql.active_record"].freeze
SPAN_PREFIX = "db.".freeze
SPAN_ORIGIN = "auto.db.rails".freeze
EXCLUDED_EVENTS = ["SCHEMA", "TRANSACTION"].freeze

SUPPORT_SOURCE_LOCATION = ActiveSupport::BacktraceCleaner.method_defined?(:clean_frame)
Expand All @@ -28,7 +29,13 @@ def subscribe!
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
next if EXCLUDED_EVENTS.include? payload[:name]

record_on_current_span(op: SPAN_PREFIX + event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:sql], duration: duration) do |span|
record_on_current_span(
op: SPAN_PREFIX + event_name,
origin: SPAN_ORIGIN,
start_timestamp: payload[START_TIMESTAMP_NAME],
description: payload[:sql],
duration: duration
) do |span|
span.set_tag(:cached, true) if payload.fetch(:cached, false) # cached key is only set for hits in the QueryCache, from Rails 5.1

connection = payload[:connection]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,17 @@ class ActiveStorageSubscriber < AbstractSubscriber
analyze.active_storage
].freeze

SPAN_ORIGIN = "auto.file.rails".freeze

def self.subscribe!
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
record_on_current_span(op: "file.#{event_name}".freeze, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:service], duration: duration) do |span|
record_on_current_span(
op: "file.#{event_name}".freeze,
origin: SPAN_ORIGIN,
start_timestamp: payload[START_TIMESTAMP_NAME],
description: payload[:service],
duration: duration
) do |span|
payload.each do |key, value|
span.set_data(key, value) unless key == START_TIMESTAMP_NAME
end
Expand Down
15 changes: 10 additions & 5 deletions sentry-rails/spec/sentry/rails/action_cable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ def disconnect
expect(transaction["contexts"]).to include(
"trace" => hash_including(
"op" => "websocket.server",
"status" => "internal_error"
"status" => "internal_error",
"origin" => "auto.http.rails.actioncable"
)
)
end
Expand All @@ -230,7 +231,8 @@ def disconnect
expect(subscription_transaction["contexts"]).to include(
"trace" => hash_including(
"op" => "websocket.server",
"status" => "ok"
"status" => "ok",
"origin" => "auto.http.rails.actioncable"
)
)

Expand Down Expand Up @@ -259,7 +261,8 @@ def disconnect
expect(action_transaction["contexts"]).to include(
"trace" => hash_including(
"op" => "websocket.server",
"status" => "internal_error"
"status" => "internal_error",
"origin" => "auto.http.rails.actioncable"
)
)
end
Expand All @@ -281,7 +284,8 @@ def disconnect
expect(subscription_transaction["contexts"]).to include(
"trace" => hash_including(
"op" => "websocket.server",
"status" => "ok"
"status" => "ok",
"origin" => "auto.http.rails.actioncable"
)
)

Expand All @@ -308,7 +312,8 @@ def disconnect
expect(transaction["contexts"]).to include(
"trace" => hash_including(
"op" => "websocket.server",
"status" => "internal_error"
"status" => "internal_error",
"origin" => "auto.http.rails.actioncable"
)
)
end
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 @@ -182,6 +182,7 @@ def post.to_global_id
expect(transaction.contexts.dig(:trace, :span_id)).to be_present
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
expect(transaction.contexts.dig(:trace, :op)).to eq("queue.active_job")
expect(transaction.contexts.dig(:trace, :origin)).to eq("auto.queue.active_job")

expect(transaction.spans.count).to eq(1)
expect(transaction.spans.first[:op]).to eq("db.sql.active_record")
Expand All @@ -199,6 +200,7 @@ def post.to_global_id
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")
expect(transaction.contexts.dig(:trace, :origin)).to eq("auto.queue.active_job")

event = transport.events.last
expect(event.transaction).to eq("FailedWithExtraJob")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

span = transaction[:spans][0]
expect(span[:op]).to eq("view.process_action.action_controller")
expect(span[:origin]).to eq("auto.view.rails")
expect(span[:description]).to eq("HelloController#world")
expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
expect(span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
# ignore the first span, which is for controller action
span = transaction[:spans][1]
expect(span[:op]).to eq("template.render_template.action_view")
expect(span[:origin]).to eq("auto.template.rails")
expect(span[:description]).to match(/test_template\.html\.erb/)
expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

span = transaction[:spans][0]
expect(span[:op]).to eq("db.sql.active_record")
expect(span[:origin]).to eq("auto.db.rails")
expect(span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
expect(span[:tags].key?(:cached)).to eq(false)
expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
Expand Down Expand Up @@ -134,6 +135,7 @@ def foo

cached_query_span = transaction[:spans][1]
expect(cached_query_span[:op]).to eq("db.sql.active_record")
expect(cached_query_span[:origin]).to eq("auto.db.rails")
expect(cached_query_span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
expect(cached_query_span[:tags]).to include({ cached: true })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@
if Rails.version.to_f > 6.1
expect(analysis_transaction[:spans].count).to eq(2)
expect(analysis_transaction[:spans][0][:op]).to eq("file.service_streaming_download.active_storage")
expect(analysis_transaction[:spans][0][:origin]).to eq("auto.file.rails")
expect(analysis_transaction[:spans][1][:op]).to eq("file.analyze.active_storage")
expect(analysis_transaction[:spans][1][:origin]).to eq("auto.file.rails")
else
expect(analysis_transaction[:spans].count).to eq(1)
expect(analysis_transaction[:spans][0][:op]).to eq("file.service_streaming_download.active_storage")
expect(analysis_transaction[:spans][0][:origin]).to eq("auto.file.rails")

Check warning on line 38 in sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb

View check run for this annotation

Codecov / codecov/patch

sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb#L38

Added line #L38 was not covered by tests
end

request_transaction = transport.events.last.to_hash
Expand All @@ -41,6 +44,7 @@

span = request_transaction[:spans][1]
expect(span[:op]).to eq("file.service_upload.active_storage")
expect(span[:origin]).to eq("auto.file.rails")
expect(span[:description]).to eq("Disk")
expect(span.dig(:data, :key)).to eq(p.cover.key)
expect(span[:trace_id]).to eq(request_transaction.dig(:contexts, :trace, :trace_id))
Expand Down
9 changes: 8 additions & 1 deletion sentry-rails/spec/sentry/rails/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,21 @@

expect(transaction[:type]).to eq("transaction")
expect(transaction.dig(:contexts, :trace, :op)).to eq("http.server")
expect(transaction.dig(:contexts, :trace, :origin)).to eq("auto.http.rails")
parent_span_id = transaction.dig(:contexts, :trace, :span_id)
expect(transaction[:spans].count).to eq(2)

first_span = transaction[:spans][0]
expect(first_span[:op]).to eq("view.process_action.action_controller")
expect(first_span[:origin]).to eq("auto.view.rails")
expect(first_span[:description]).to eq("PostsController#index")
expect(first_span[:parent_span_id]).to eq(parent_span_id)
expect(first_span[:status]).to eq("internal_error")
expect(first_span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params])

second_span = transaction[:spans][1]
expect(second_span[:op]).to eq("db.sql.active_record")
expect(second_span[:origin]).to eq("auto.db.rails")
expect(second_span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
expect(second_span[:parent_span_id]).to eq(first_span[:span_id])

Expand All @@ -63,19 +66,21 @@

expect(transaction[:type]).to eq("transaction")
expect(transaction.dig(:contexts, :trace, :op)).to eq("http.server")
expect(transaction.dig(:contexts, :trace, :origin)).to eq("auto.http.rails")
parent_span_id = transaction.dig(:contexts, :trace, :span_id)
expect(transaction[:spans].count).to eq(3)

first_span = transaction[:spans][0]
expect(first_span[:data].keys).to match_array(["http.response.status_code", :format, :method, :path, :params])
expect(first_span[:op]).to eq("view.process_action.action_controller")
expect(first_span[:origin]).to eq("auto.view.rails")
expect(first_span[:description]).to eq("PostsController#show")
expect(first_span[:parent_span_id]).to eq(parent_span_id)
expect(first_span[:status]).to eq("ok")


second_span = transaction[:spans][1]
expect(second_span[:op]).to eq("db.sql.active_record")
expect(second_span[:origin]).to eq("auto.db.rails")
expect(second_span[:description].squeeze("\s")).to eq(
'SELECT "posts".* FROM "posts" WHERE "posts"."id" = ? LIMIT ?'
)
Expand All @@ -86,6 +91,7 @@

third_span = transaction[:spans][2]
expect(third_span[:op]).to eq("template.render_template.action_view")
expect(third_span[:origin]).to eq("auto.template.rails")
expect(third_span[:description].squeeze("\s")).to eq("text template")
expect(third_span[:parent_span_id]).to eq(first_span[:span_id])
end
Expand Down Expand Up @@ -239,6 +245,7 @@
expect(transaction.timestamp).not_to be_nil
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
expect(transaction.contexts.dig(:trace, :origin)).to eq("auto.http.rails")
expect(transaction.spans.count).to eq(3)

# should inherit information from the external_transaction
Expand Down

0 comments on commit 5c988f8

Please sign in to comment.