Consolidate ActiveRecord and Redis database spans with Span::DataConv…

…entions (#2100)

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Make `:value` in `SingleExceptionInterface` writable, so that it can be modified in `before_send` under `event.exception.values[n].value` [#2072](https://github.com/getsentry/sentry-ruby/pull/2072)
 - Add `sampled` field to `dynamic_sampling_context` [#2092](https://github.com/getsentry/sentry-ruby/pull/2092)
 - Consolidate HTTP span data conventions with OpenTelemetry with `Sentry::Span::DataConventions`  [#2093](https://github.com/getsentry/sentry-ruby/pull/2093)
+- Consolidate database span data conventions with OpenTelemetry for ActiveRecord and Redis [#2100](https://github.com/getsentry/sentry-ruby/pull/2100)
 - Add new `config.trace_propagation_targets` option to set targets for which headers are propagated in outgoing HTTP requests [#2079](https://github.com/getsentry/sentry-ruby/pull/2079)
 
   ```rb

sentry-rails/lib/sentry/rails/tracing/active_record_subscriber.rb

@@ -13,8 +13,31 @@ def self.subscribe!
             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|
-              span.set_data(:connection_id, payload[:connection_id])
               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]
+
+              if payload[:connection_id]
+                span.set_data(:connection_id, payload[:connection_id])
+
+                # we fallback to the base connection on rails < 6.0.0 since the payload doesn't have it
+                base_connection = ActiveRecord::Base.connection
+                connection ||= base_connection if payload[:connection_id] == base_connection.object_id
+              end
+
+              next unless connection
+
+              db_config = if connection.pool.respond_to?(:db_config)
+                            connection.pool.db_config.configuration_hash
+                          elsif connection.pool.respond_to?(:spec)
+                            connection.pool.spec.config
+                          end
+
+              span.set_data(Span::DataConventions::DB_SYSTEM, db_config[:adapter]) if db_config[:adapter]
+              span.set_data(Span::DataConventions::DB_NAME, db_config[:database]) if db_config[:database]
+              span.set_data(Span::DataConventions::SERVER_ADDRESS, db_config[:host]) if db_config[:host]
+              span.set_data(Span::DataConventions::SERVER_PORT, db_config[:port]) if db_config[:port]
+              span.set_data(Span::DataConventions::SERVER_SOCKET_ADDRESS, db_config[:socket]) if db_config[:socket]
             end
           end
         end

sentry-rails/spec/sentry/rails/tracing/active_record_subscriber_spec.rb

@@ -32,6 +32,10 @@
       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))
+
+      data = span[:data]
+      expect(data["db.name"]).to include("db")
+      expect(data["db.system"]).to eq("sqlite3")
     end
 
     it "records database cached query events", skip: Rails.version.to_f < 5.1 do
@@ -55,6 +59,10 @@
       expect(cached_query_span[:op]).to eq("db.sql.active_record")
       expect(cached_query_span[:description]).to eq("SELECT \"posts\".* FROM \"posts\"")
       expect(cached_query_span[:tags]).to include({cached: true})
+
+      data = cached_query_span[:data]
+      expect(data["db.name"]).to include("db")
+      expect(data["db.system"]).to eq("sqlite3")
     end
   end
 

sentry-ruby/lib/sentry/redis.rb

@@ -19,7 +19,10 @@ def instrument
 
           if span
             span.set_description(commands_description)
-            span.set_data(:server, server_description)
+            span.set_data(Span::DataConventions::DB_SYSTEM, "redis")
+            span.set_data(Span::DataConventions::DB_NAME, db)
+            span.set_data(Span::DataConventions::SERVER_ADDRESS, host)
+            span.set_data(Span::DataConventions::SERVER_PORT, port)
           end
         end
       end

sentry-ruby/lib/sentry/span.rb

@@ -6,11 +6,39 @@ module Sentry
   class Span
 
     # We will try to be consistent with OpenTelemetry on this front going forward.
+    # https://develop.sentry.dev/sdk/performance/span-data-conventions/
     module DataConventions
       URL = "url"
       HTTP_STATUS_CODE = "http.response.status_code"
       HTTP_QUERY = "http.query"
       HTTP_METHOD = "http.method"
+
+      # An identifier for the database management system (DBMS) product being used.
+      # Example: postgresql
+      DB_SYSTEM = "db.system"
+
+      # The name of the database being accessed.
+      # For commands that switch the database, this should be set to the target database
+      # (even if the command fails).
+      # Example: myDatabase
+      DB_NAME = "db.name"
+
+      # Name of the database host.
+      # Example: example.com
+      SERVER_ADDRESS = "server.address"
+
+      # Logical server port number
+      # Example: 80; 8080; 443
+      SERVER_PORT = "server.port"
+
+      # Physical server IP address or Unix socket address.
+      # Example: 10.5.3.2
+      SERVER_SOCKET_ADDRESS = "server.socket.address"
+
+      # Physical server port.
+      # Recommended: If different than server.port.
+      # Example: 16456
+      SERVER_SOCKET_PORT = "server.socket.port"
     end
 
     STATUS_MAP = {

sentry-ruby/spec/sentry/redis_spec.rb

@@ -27,7 +27,12 @@
           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("SET key value")
-          expect(request_span.data).to eq({ server: "127.0.0.1:6379/0" })
+          expect(request_span.data).to eq({
+            "db.name" => 0,
+            "db.system" => "redis",
+            "server.address" => "127.0.0.1",
+            "server.port" => 6379
+          })
         end
 
         it "removes bad encoding keys and arguments gracefully" do