Update Tracing Span's op names (#1923)

* Update sentry-rails' tracing op names * Update sentry-ruby's tracing op names * Update sentry-resque's tracing op names * Update sentry-sidekiq's tracing op names * Update sentry-delayed_job's tracing op names * Update changelog
getsentry · Oct 31, 2022 · 69073ab · 69073ab
1 parent 279df83
commit 69073ab
Show file tree

Hide file tree

Showing 22 changed files with 53 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,10 @@
   - Fixes [#1911](https://github.com/getsentry/sentry-ruby/issues/1911)
 - Add missing `initialized?` checks to `sentry-rails` [#1919](https://github.com/getsentry/sentry-ruby/pull/1919)
   - Fixes [#1885](https://github.com/getsentry/sentry-ruby/issues/1885)
+- Update Tracing Span's op names [#1923](https://github.com/getsentry/sentry-ruby/pull/1923)
+
+  Currently, Ruby integrations' Span op names aren't aligned with the core specification's convention, so we decided to update them altogether in this PR.
+  **If you rely on Span op names for fine-grained event filtering, this may affect the data your app sends to Sentry.**
 
 ### Refactoring
 

diff --git a/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb b/sentry-delayed_job/lib/sentry/delayed_job/plugin.rb
@@ -7,6 +7,7 @@ class Plugin < ::Delayed::Plugin
       # need to symbolize strings as keyword arguments in Ruby 2.4~2.6
       DELAYED_JOB_CONTEXT_KEY = :"Delayed-Job"
       ACTIVE_JOB_CONTEXT_KEY = :"Active-Job"
+      OP_NAME = "queue.delayed_job".freeze
 
       callbacks do |lifecycle|
         lifecycle.around(:invoke_job) do |job, *args, &block|
@@ -19,7 +20,7 @@ class Plugin < ::Delayed::Plugin
             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, source: scope.transaction_source, op: "delayed_job")
+            transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME)
             scope.set_span(transaction) if transaction
 
             begin
@@ -86,7 +87,7 @@ def self.report?(job)
 
       def self.finish_transaction(transaction, status)
         return unless transaction
-  
+
         transaction.set_http_status(status)
         transaction.finish
       end

diff --git a/sentry-delayed_job/spec/sentry/delayed_job_spec.rb b/sentry-delayed_job/spec/sentry/delayed_job_spec.rb
@@ -329,6 +329,7 @@ def perform
       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")
+      expect(transaction.contexts.dig(:trace, :op)).to eq("queue.delayed_job")
     end
 
     it "records transaction with exception" do

diff --git a/sentry-rails/lib/sentry/rails/action_cable.rb b/sentry-rails/lib/sentry/rails/action_cable.rb
@@ -2,6 +2,8 @@ module Sentry
   module Rails
     module ActionCableExtensions
       class ErrorHandler
+        OP_NAME = "websocket.server".freeze
+
         class << self
           def capture(connection, transaction_name:, extra_context: nil, &block)
             return block.call unless Sentry.initialized?
@@ -33,7 +35,7 @@ def start_transaction(env, scope)
             sentry_trace = env["HTTP_SENTRY_TRACE"]
             baggage = env["HTTP_BAGGAGE"]
 
-            options = { name: scope.transaction_name, source: scope.transaction_source, op: "rails.action_cable".freeze }
+            options = { name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME }
             transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, baggage: baggage, **options) if sentry_trace
             Sentry.start_transaction(transaction: transaction, **options)
           end

diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb
@@ -16,6 +16,8 @@ def already_supported_by_sentry_integration?
       end
 
       class SentryReporter
+        OP_NAME = "queue.active_job".freeze
+
         class << self
           def record(job, &block)
             Sentry.with_scope do |scope|
@@ -25,7 +27,7 @@ def record(job, &block)
                   if job.is_a?(::Sentry::SendEventJob)
                     nil
                   else
-                    Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "active_job")
+                    Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME)
                   end
 
                 scope.set_span(transaction) if transaction

diff --git a/sentry-rails/lib/sentry/rails/capture_exceptions.rb b/sentry-rails/lib/sentry/rails/capture_exceptions.rb
@@ -17,7 +17,7 @@ def collect_exception(env)
       end
 
       def transaction_op
-        "rails.request".freeze
+        "http.server".freeze
       end
 
       def capture_exception(exception, env)

diff --git a/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/action_controller_subscriber.rb
@@ -8,14 +8,15 @@ class ActionControllerSubscriber < AbstractSubscriber
         extend InstrumentPayloadCleanupHelper
 
         EVENT_NAMES = ["process_action.action_controller"].freeze
+        OP_NAME = "view.process_action.action_controller".freeze
 
         def self.subscribe!
           subscribe_to_event(EVENT_NAMES) do |event_name, duration, payload|
             controller = payload[:controller]
             action = payload[:action]
 
             record_on_current_span(
-              op: event_name,
+              op: OP_NAME,
               start_timestamp: payload[START_TIMESTAMP_NAME],
               description: "#{controller}##{action}",
               duration: duration

diff --git a/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb b/sentry-rails/lib/sentry/rails/tracing/active_storage_subscriber.rb
@@ -21,7 +21,7 @@ class ActiveStorageSubscriber < AbstractSubscriber
 
         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[:service], duration: duration) do |span|
+            record_on_current_span(op: "file.#{event_name}".freeze, 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

diff --git a/sentry-rails/spec/sentry/rails/action_cable_spec.rb b/sentry-rails/spec/sentry/rails/action_cable_spec.rb
@@ -177,7 +177,7 @@ def disconnect
           )
           expect(transaction["contexts"]).to include(
             "trace" => hash_including(
-              "op" => "rails.action_cable",
+              "op" => "websocket.server",
               "status" => "internal_error"
             )
           )
@@ -203,7 +203,7 @@ def disconnect
           )
           expect(subscription_transaction["contexts"]).to include(
             "trace" => hash_including(
-              "op" => "rails.action_cable",
+              "op" => "websocket.server",
               "status" => "ok"
             )
           )
@@ -232,7 +232,7 @@ def disconnect
           )
           expect(action_transaction["contexts"]).to include(
             "trace" => hash_including(
-              "op" => "rails.action_cable",
+              "op" => "websocket.server",
               "status" => "internal_error"
             )
           )
@@ -254,7 +254,7 @@ def disconnect
           )
           expect(subscription_transaction["contexts"]).to include(
             "trace" => hash_including(
-              "op" => "rails.action_cable",
+              "op" => "websocket.server",
               "status" => "ok"
             )
           )
@@ -281,7 +281,7 @@ def disconnect
           )
           expect(transaction["contexts"]).to include(
             "trace" => hash_including(
-              "op" => "rails.action_cable",
+              "op" => "websocket.server",
               "status" => "internal_error"
             )
           )

diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb
@@ -170,6 +170,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("ok")
+      expect(transaction.contexts.dig(:trace, :op)).to eq("queue.active_job")
 
       expect(transaction.spans.count).to eq(1)
       expect(transaction.spans.first[:op]).to eq("db.sql.active_record")

diff --git a/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/tracing/action_controller_subscriber_spec.rb
@@ -23,7 +23,7 @@
       expect(transaction[:spans].count).to eq(1)
 
       span = transaction[:spans][0]
-      expect(span[:op]).to eq("process_action.action_controller")
+      expect(span[:op]).to eq("view.process_action.action_controller")
       expect(span[:description]).to eq("HelloController#world")
       expect(span[:trace_id]).to eq(transaction.dig(:contexts, :trace, :trace_id))
       expect(span[:data][:payload].keys).not_to include(:request)

diff --git a/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb b/sentry-rails/spec/sentry/rails/tracing/active_storage_subscriber_spec.rb
@@ -28,19 +28,19 @@
 
       if Rails.version.to_f > 6.1
         expect(analysis_transaction[:spans].count).to eq(2)
-        expect(analysis_transaction[:spans][0][:op]).to eq("service_streaming_download.active_storage")
-        expect(analysis_transaction[:spans][1][:op]).to eq("analyze.active_storage")
+        expect(analysis_transaction[:spans][0][:op]).to eq("file.service_streaming_download.active_storage")
+        expect(analysis_transaction[:spans][1][:op]).to eq("file.analyze.active_storage")
       else
         expect(analysis_transaction[:spans].count).to eq(1)
-        expect(analysis_transaction[:spans][0][:op]).to eq("service_streaming_download.active_storage")
+        expect(analysis_transaction[:spans][0][:op]).to eq("file.service_streaming_download.active_storage")
       end
 
       request_transaction = transport.events.last.to_hash
       expect(request_transaction[:type]).to eq("transaction")
       expect(request_transaction[:spans].count).to eq(1)
 
       span = request_transaction[:spans][0]
-      expect(span[:op]).to eq("service_upload.active_storage")
+      expect(span[:op]).to eq("file.service_upload.active_storage")
       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))

diff --git a/sentry-rails/spec/sentry/rails/tracing_spec.rb b/sentry-rails/spec/sentry/rails/tracing_spec.rb
@@ -31,7 +31,7 @@
       expect(event.dig(:contexts, :trace, :trace_id)).to eq(transaction.dig(:contexts, :trace, :trace_id))
 
       expect(transaction[:type]).to eq("transaction")
-      expect(transaction.dig(:contexts, :trace, :op)).to eq("rails.request")
+      expect(transaction.dig(:contexts, :trace, :op)).to eq("http.server")
       parent_span_id = transaction.dig(:contexts, :trace, :span_id)
       expect(transaction[:spans].count).to eq(2)
 
@@ -44,7 +44,7 @@
       expect(first_span[:timestamp] - first_span[:start_timestamp]).to be_between(10.0 / 1_000_000, 10.0 / 1000)
 
       second_span = transaction[:spans][1]
-      expect(second_span[:op]).to eq("process_action.action_controller")
+      expect(second_span[:op]).to eq("view.process_action.action_controller")
       expect(second_span[:description]).to eq("PostsController#index")
       expect(second_span[:parent_span_id]).to eq(parent_span_id)
     end
@@ -60,7 +60,7 @@
       transaction = transport.events.last.to_hash
 
       expect(transaction[:type]).to eq("transaction")
-      expect(transaction.dig(:contexts, :trace, :op)).to eq("rails.request")
+      expect(transaction.dig(:contexts, :trace, :op)).to eq("http.server")
       parent_span_id = transaction.dig(:contexts, :trace, :span_id)
       expect(transaction[:spans].count).to eq(3)
 
@@ -78,7 +78,7 @@
       expect(last_span[:data][:payload].keys).not_to include(:headers)
       expect(last_span[:data][:payload].keys).not_to include(:request)
       expect(last_span[:data][:payload].keys).not_to include(:response)
-      expect(last_span[:op]).to eq("process_action.action_controller")
+      expect(last_span[:op]).to eq("view.process_action.action_controller")
       expect(last_span[:description]).to eq("PostsController#show")
       expect(last_span[:parent_span_id]).to eq(parent_span_id)
     end
@@ -214,7 +214,7 @@
         expect(transaction.type).to eq("transaction")
         expect(transaction.timestamp).not_to be_nil
         expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
-        expect(transaction.contexts.dig(:trace, :op)).to eq("rails.request")
+        expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
         expect(transaction.spans.count).to eq(3)
 
         # should inherit information from the external_transaction

diff --git a/sentry-resque/lib/sentry/resque.rb b/sentry-resque/lib/sentry/resque.rb
@@ -25,7 +25,7 @@ def record(queue, worker, payload, &block)
 
               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")
+              transaction = Sentry.start_transaction(name: scope.transaction_name, source: scope.transaction_source, op: "queue.resque")
               scope.set_span(transaction) if transaction
 
               yield

diff --git a/sentry-resque/spec/sentry/tracing_spec.rb b/sentry-resque/spec/sentry/tracing_spec.rb
@@ -41,7 +41,7 @@ def self.perform(msg)
     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")
+    expect(tracing_event.dig(:contexts, :trace, :op)).to eq("queue.resque")
   end
 
   it "records tracing events with exceptions" do
@@ -58,6 +58,6 @@ def self.perform(msg)
     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")
+    expect(tracing_event.dig(:contexts, :trace, :op)).to eq("queue.resque")
   end
 end
diff --git a/sentry-ruby/lib/sentry/rack/capture_exceptions.rb b/sentry-ruby/lib/sentry/rack/capture_exceptions.rb
@@ -52,7 +52,7 @@ def collect_exception(env)
       end
 
       def transaction_op
-        "rack.request".freeze
+        "http.server".freeze
       end
 
       def capture_exception(exception, env)

diff --git a/sentry-ruby/lib/sentry/redis.rb b/sentry-ruby/lib/sentry/redis.rb
@@ -3,7 +3,7 @@
 module Sentry
   # @api private
   class Redis
-    OP_NAME = "db.redis.command"
+    OP_NAME = "db.redis"
     LOGGER_NAME = :redis_logger
 
     def initialize(commands, host, port, db)

diff --git a/sentry-ruby/spec/sentry/breadcrumb/redis_logger_spec.rb b/sentry-ruby/spec/sentry/breadcrumb/redis_logger_spec.rb
@@ -19,7 +19,7 @@
 
       expect(result).to eq("OK")
       expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
-        category: "db.redis.command",
+        category: "db.redis",
         data: { commands: [{ command: "SET", key: "key" }], server: "127.0.0.1:6379/0" }
       )
     end
@@ -33,7 +33,7 @@
 
       expect(result).to eq("OK")
       expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
-        category: "db.redis.command",
+        category: "db.redis",
         data: { commands: [{ command: "SET", key: "key", arguments: "value" }], server: "127.0.0.1:6379/0" }
       )
     end
@@ -61,7 +61,7 @@
     it "doesn't cause an error" do
       expect(result).to include("uptime_in_days" => 0)
       expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
-        category: "db.redis.command",
+        category: "db.redis",
         data: { commands: [{ command: "INFO", key: nil }], server: "127.0.0.1:6379/0" }
       )
     end
@@ -80,7 +80,7 @@
 
       expect(result).to contain_exactly("OK", kind_of(Numeric))
       expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
-        category: "db.redis.command",
+        category: "db.redis",
         data: {
           commands: [
             { command: "MULTI", key: nil },
@@ -104,7 +104,7 @@
 
       expect(result).to eq("OK")
       expect(Sentry.get_current_scope.breadcrumbs.peek).to have_attributes(
-        category: "db.redis.command",
+        category: "db.redis",
         data: { commands: [{ command: "SET", key: "key" }], server: "127.0.0.1:6379/0" }
       )
     end

diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb
@@ -252,7 +252,7 @@ def verify_transaction_attributes(transaction)
         expect(transaction.transaction_info).to eq({ source: :url })
         expect(transaction.timestamp).not_to be_nil
         expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
-        expect(transaction.contexts.dig(:trace, :op)).to eq("rack.request")
+        expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
         expect(transaction.spans.count).to eq(0)
       end
 
@@ -436,7 +436,7 @@ def will_be_sampled_by_sdk
         expect(transaction.transaction_info).to eq({ source: :url })
         expect(transaction.timestamp).not_to be_nil
         expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
-        expect(transaction.contexts.dig(:trace, :op)).to eq("rack.request")
+        expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
         expect(transaction.spans.count).to eq(0)
       end
 
@@ -460,7 +460,7 @@ def will_be_sampled_by_sdk
           expect(transaction.transaction).to eq("/test")
           expect(transaction.transaction_info).to eq({ source: :url })
           expect(transaction.contexts.dig(:trace, :status)).to eq("ok")
-          expect(transaction.contexts.dig(:trace, :op)).to eq("rack.request")
+          expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
           expect(transaction.spans.count).to eq(2)
 
           first_span = transaction.spans.first
@@ -518,7 +518,7 @@ def will_be_sampled_by_sdk
         expect(transaction.type).to eq("transaction")
         expect(transaction.timestamp).not_to be_nil
         expect(transaction.contexts.dig(:trace, :status)).to eq("internal_error")
-        expect(transaction.contexts.dig(:trace, :op)).to eq("rack.request")
+        expect(transaction.contexts.dig(:trace, :op)).to eq("http.server")
         expect(transaction.spans.count).to eq(0)
       end
     end

diff --git a/sentry-ruby/spec/sentry/redis_spec.rb b/sentry-ruby/spec/sentry/redis_spec.rb
@@ -24,7 +24,7 @@
 
           expect(result).to eq("OK")
           request_span = transaction.span_recorder.spans.last
-          expect(request_span.op).to eq("db.redis.command")
+          expect(request_span.op).to eq("db.redis")
           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)

diff --git a/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb b/sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb
@@ -3,6 +3,8 @@
 module Sentry
   module Sidekiq
     class SentryContextServerMiddleware
+      OP_NAME = "queue.sidekiq".freeze
+
       def call(_worker, job, queue)
         return yield unless Sentry.initialized?
 
@@ -38,7 +40,7 @@ def build_tags(tags)
       end
 
       def start_transaction(scope, sentry_trace)
-        options = { name: scope.transaction_name, source: scope.transaction_source, op: "sidekiq" }
+        options = { name: scope.transaction_name, source: scope.transaction_source, op: OP_NAME }
         transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
         Sentry.start_transaction(transaction: transaction, **options)
       end