Skip to content

Commit

Permalink
feat(performance): Sync activerecord and net-http span names (#1681)
Browse files Browse the repository at this point in the history
* feat(performance): Sync activerecord db.* and net-http http.* spans to work with discover slow ops dashboards

* Add changelog entry

* Add template. prefix to action_view spans

* Update changelog

Co-authored-by: Stan Lo <stan001212@gmail.com>
  • Loading branch information
sl0thentr0py and st0012 committed Feb 1, 2022
1 parent 9889bbe commit 8538717
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Features

- Sync activerecord, actionview and net-http span names [#1681](https://github.com/getsentry/sentry-ruby/pull/1681)
- Support serializing ActiveRecord job arguments in global id form [#1688](https://github.com/getsentry/sentry-ruby/pull/1688)
- Register Sentry's ErrorSubscriber for Rails 7.0+ apps [#1705](https://github.com/getsentry/sentry-ruby/pull/1705)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ module Rails
module Tracing
class ActionViewSubscriber < AbstractSubscriber
EVENT_NAMES = ["render_template.action_view"].freeze
SPAN_PREFIX = "template.".freeze

def self.subscribe!
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:identifier], duration: duration)
record_on_current_span(op: SPAN_PREFIX + event_name, 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 @@ -5,13 +5,14 @@ module Rails
module Tracing
class ActiveRecordSubscriber < AbstractSubscriber
EVENT_NAMES = ["sql.active_record"].freeze
SPAN_PREFIX = "db.".freeze
EXCLUDED_EVENTS = ["SCHEMA", "TRANSACTION"].freeze

def self.subscribe!
subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
next if EXCLUDED_EVENTS.include? payload[:name]

record_on_current_span(op: event_name, start_timestamp: payload[START_TIMESTAMP_NAME], description: payload[:sql], duration: duration) do |span|
record_on_current_span(op: SPAN_PREFIX + 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
2 changes: 1 addition & 1 deletion sentry-rails/spec/sentry/rails/activejob_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def post.to_global_id
expect(transaction.contexts.dig(:trace, :status)).to eq("ok")

expect(transaction.spans.count).to eq(1)
expect(transaction.spans.first[:op]).to eq("sql.active_record")
expect(transaction.spans.first[:op]).to eq("db.sql.active_record")
end

context "with error" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
expect(transaction[:spans].count).to eq(1)

span = transaction[:spans][0]
expect(span[:op]).to eq("render_template.action_view")
expect(span[:op]).to eq("template.render_template.action_view")
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 @@ -28,7 +28,7 @@
expect(transaction[:spans].count).to eq(1)

span = transaction[:spans][0]
expect(span[:op]).to eq("sql.active_record")
expect(span[:op]).to eq("db.sql.active_record")
expect(span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
end
Expand Down
4 changes: 2 additions & 2 deletions sentry-rails/spec/sentry/rails/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
expect(transaction[:spans].count).to eq(2)

first_span = transaction[:spans][0]
expect(first_span[:op]).to eq("sql.active_record")
expect(first_span[:op]).to eq("db.sql.active_record")
expect(first_span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
expect(first_span[:parent_span_id]).to eq(parent_span_id)

Expand Down Expand Up @@ -65,7 +65,7 @@
expect(transaction[:spans].count).to eq(3)

first_span = transaction[:spans][0]
expect(first_span[:op]).to eq("sql.active_record")
expect(first_span[:op]).to eq("db.sql.active_record")
expect(first_span[:description].squeeze("\s")).to eq(
'SELECT "posts".* FROM "posts" WHERE "posts"."id" = ? LIMIT ?'
)
Expand Down
5 changes: 3 additions & 2 deletions sentry-ruby/lib/sentry/net/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ module Sentry
# @api private
module Net
module HTTP
OP_NAME = "net.http"
OP_NAME = "http.client"
BREADCRUMB_CATEGORY = "net.http"

# To explain how the entire thing works, we need to know how the original Net::HTTP#request works
# Here's part of its definition. As you can see, it usually calls itself inside a #start block
Expand Down Expand Up @@ -53,7 +54,7 @@ def record_sentry_breadcrumb(req, res)

crumb = Sentry::Breadcrumb.new(
level: :info,
category: OP_NAME,
category: BREADCRUMB_CATEGORY,
type: :info,
data: {
status: res.code.to_i,
Expand Down
8 changes: 4 additions & 4 deletions sentry-ruby/spec/sentry/net/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
expect(transaction.span_recorder.spans.count).to eq(2)

request_span = transaction.span_recorder.spans.last
expect(request_span.op).to eq("net.http")
expect(request_span.op).to eq("http.client")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
Expand All @@ -63,7 +63,7 @@
expect(transaction.span_recorder.spans.count).to eq(2)

request_span = transaction.span_recorder.spans.last
expect(request_span.op).to eq("net.http")
expect(request_span.op).to eq("http.client")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
Expand Down Expand Up @@ -142,15 +142,15 @@ def verify_spans(transaction)
expect(transaction.span_recorder.spans[0]).to eq(transaction)

request_span = transaction.span_recorder.spans[1]
expect(request_span.op).to eq("net.http")
expect(request_span.op).to eq("http.client")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
expect(request_span.description).to eq("GET http://example.com/path")
expect(request_span.data).to eq({ status: 200 })

request_span = transaction.span_recorder.spans[2]
expect(request_span.op).to eq("net.http")
expect(request_span.op).to eq("http.client")
expect(request_span.start_timestamp).not_to be_nil
expect(request_span.timestamp).not_to be_nil
expect(request_span.start_timestamp).not_to eq(request_span.timestamp)
Expand Down

0 comments on commit 8538717

Please sign in to comment.