diff --git a/Steepfile b/Steepfile index 83d86357de3..02fb04de0e0 100644 --- a/Steepfile +++ b/Steepfile @@ -375,10 +375,6 @@ target :datadog do ignore 'lib/datadog/tracing/contrib/presto/instrumentation.rb' ignore 'lib/datadog/tracing/contrib/presto/integration.rb' ignore 'lib/datadog/tracing/contrib/presto/patcher.rb' - ignore 'lib/datadog/tracing/contrib/propagation/sql_comment.rb' - ignore 'lib/datadog/tracing/contrib/propagation/sql_comment/comment.rb' - ignore 'lib/datadog/tracing/contrib/propagation/sql_comment/ext.rb' - ignore 'lib/datadog/tracing/contrib/propagation/sql_comment/mode.rb' ignore 'lib/datadog/tracing/contrib/que/configuration/settings.rb' ignore 'lib/datadog/tracing/contrib/que/ext.rb' ignore 'lib/datadog/tracing/contrib/que/integration.rb' diff --git a/lib/datadog/tracing/contrib/active_record/events/sql.rb b/lib/datadog/tracing/contrib/active_record/events/sql.rb index 9b9c79684b0..1e1c1a87382 100644 --- a/lib/datadog/tracing/contrib/active_record/events/sql.rb +++ b/lib/datadog/tracing/contrib/active_record/events/sql.rb @@ -62,6 +62,7 @@ def on_start(span, event, _id, payload) cached = payload[:cached] || (payload[:name] == PAYLOAD_CACHE) span.set_tag(Ext::TAG_DB_VENDOR, adapter_name) + span.set_tag(Contrib::Ext::DB::TAG_INSTANCE, config[:database]) span.set_tag(Ext::TAG_DB_NAME, config[:database]) span.set_tag(Ext::TAG_DB_CACHED, cached) if cached span.set_tag(Tracing::Metadata::Ext::NET::TAG_TARGET_HOST, config[:host]) if config[:host] diff --git a/lib/datadog/tracing/contrib/ext.rb b/lib/datadog/tracing/contrib/ext.rb index b5c8aa11d89..0861b1141c2 100644 --- a/lib/datadog/tracing/contrib/ext.rb +++ b/lib/datadog/tracing/contrib/ext.rb @@ -7,7 +7,21 @@ module Contrib module Ext # @public_api module DB + # Name of the database. This is *not* the database hostname. + # + # For databases which support such a concept, the default schema/database/namespace + # as configured in the connection string. + # + # If the tracer is already tracking changes to the default schema/database throughout the lifetime of + # the session (i.e. the client executes USE {NEW_SCHEMA} and now the default schema has changed from what + # was set upon connection initialization), then ideally this attribute reflects the “current” value. + # If the tracer is not already tracking changes then just leaving it to the default value set upon + # initialization is OK. + # + # This is the equivalent of OTel’s `db.namespace` + # @see https://opentelemetry.io/docs/specs/semconv/database/database-spans/#common-attributes TAG_INSTANCE = 'db.instance' + TAG_USER = 'db.user' TAG_SYSTEM = 'db.system' TAG_STATEMENT = 'db.statement' diff --git a/lib/datadog/tracing/contrib/mysql2/instrumentation.rb b/lib/datadog/tracing/contrib/mysql2/instrumentation.rb index c988f118236..e58286730c4 100644 --- a/lib/datadog/tracing/contrib/mysql2/instrumentation.rb +++ b/lib/datadog/tracing/contrib/mysql2/instrumentation.rb @@ -50,6 +50,7 @@ def query(sql, options = {}) # Set analytics sample rate Contrib::Analytics.set_sample_rate(span, analytics_sample_rate) if analytics_enabled? + span.set_tag(Contrib::Ext::DB::TAG_INSTANCE, query_options[:database]) span.set_tag(Ext::TAG_DB_NAME, query_options[:database]) span.set_tag(Tracing::Metadata::Ext::NET::TAG_TARGET_HOST, query_options[:host]) span.set_tag(Tracing::Metadata::Ext::NET::TAG_TARGET_PORT, query_options[:port]) diff --git a/lib/datadog/tracing/contrib/propagation/sql_comment.rb b/lib/datadog/tracing/contrib/propagation/sql_comment.rb index ee32914b0d1..fd557489331 100644 --- a/lib/datadog/tracing/contrib/propagation/sql_comment.rb +++ b/lib/datadog/tracing/contrib/propagation/sql_comment.rb @@ -22,13 +22,23 @@ def self.annotate!(span_op, mode) def self.prepend_comment(sql, span_op, trace_op, mode) return sql unless mode.enabled? + parent_service = datadog_configuration.service + peer_service = span_op.get_tag(Tracing::Metadata::Ext::TAG_PEER_SERVICE) + tags = { - Ext::KEY_DATABASE_SERVICE => span_op.get_tag(Tracing::Metadata::Ext::TAG_PEER_SERVICE) || span_op.service, Ext::KEY_ENVIRONMENT => datadog_configuration.env, - Ext::KEY_PARENT_SERVICE => datadog_configuration.service, - Ext::KEY_VERSION => datadog_configuration.version + Ext::KEY_PARENT_SERVICE => parent_service, + Ext::KEY_VERSION => datadog_configuration.version, + Ext::KEY_HOSTNAME => span_op.get_tag(Tracing::Metadata::Ext::TAG_PEER_HOSTNAME), + Ext::KEY_DB_NAME => span_op.get_tag(Contrib::Ext::DB::TAG_INSTANCE), + Ext::KEY_PEER_SERVICE => peer_service, } + db_service = peer_service || span_op.service + if parent_service != db_service # Only set if it's different from parent_service; otherwise it's redundant + tags[Ext::KEY_DATABASE_SERVICE] = db_service + end + if mode.full? # When tracing is disabled, trace_operation is a dummy object that does not contain data to build traceparent if datadog_configuration.tracing.enabled diff --git a/lib/datadog/tracing/contrib/propagation/sql_comment/ext.rb b/lib/datadog/tracing/contrib/propagation/sql_comment/ext.rb index 3d0325f30a5..216bd070142 100644 --- a/lib/datadog/tracing/contrib/propagation/sql_comment/ext.rb +++ b/lib/datadog/tracing/contrib/propagation/sql_comment/ext.rb @@ -20,10 +20,38 @@ module Ext # The value should be `true` when `full` mode TAG_DBM_TRACE_INJECTED = '_dd.dbm_trace_injected' + # Database service/sql span service (i.e. the service executing the actual query) + # + # If fake services are disabled: + # This value will be the same as the parent service + # + # If fake services are enabled: + # This value is NOT the same as the parent service + # + # This should NOT be overridden by peer.service. KEY_DATABASE_SERVICE = 'dddbs' + + # The global service environment (e.g. DD_ENV) KEY_ENVIRONMENT = 'dde' + + # The global service name (e.g. DD_SERVICE) KEY_PARENT_SERVICE = 'ddps' + + # The global service version (e.g. DD_VERSION) KEY_VERSION = 'ddpv' + + # The hostname of the database server, as provided to the database client upon instantiation. + # @see Datadog::Tracing::Metadata::Ext::TAG_PEER_HOSTNAME + KEY_HOSTNAME = 'ddh' + + # @see Datadog::Tracing::Contrib::Ext::DB::TAG_INSTANCE + KEY_DB_NAME = 'dddb' + + # Users can use this attribute to specify the identity of the dependency/database they are connecting to. + # We should grab this attribute only if the user is EXPLICITLY specifying it. + # @see Datadog::Tracing::Metadata::Ext::TAG_PEER_SERVICE + KEY_PEER_SERVICE = 'ddprs' + KEY_TRACEPARENT = 'traceparent' end end diff --git a/lib/datadog/tracing/contrib/trilogy/instrumentation.rb b/lib/datadog/tracing/contrib/trilogy/instrumentation.rb index e35df9e25df..df31995f4d7 100644 --- a/lib/datadog/tracing/contrib/trilogy/instrumentation.rb +++ b/lib/datadog/tracing/contrib/trilogy/instrumentation.rb @@ -49,6 +49,7 @@ def query(sql) # Set analytics sample rate Contrib::Analytics.set_sample_rate(span, analytics_sample_rate) if analytics_enabled? + span.set_tag(Contrib::Ext::DB::TAG_INSTANCE, connection_options[:database]) span.set_tag(Ext::TAG_DB_NAME, connection_options[:database]) span.set_tag(Tracing::Metadata::Ext::NET::TAG_TARGET_HOST, connection_options[:host]) span.set_tag(Tracing::Metadata::Ext::NET::TAG_TARGET_PORT, connection_options[:port]) diff --git a/lib/datadog/tracing/metadata/ext.rb b/lib/datadog/tracing/metadata/ext.rb index 0034cd77a46..02b87b86877 100644 --- a/lib/datadog/tracing/metadata/ext.rb +++ b/lib/datadog/tracing/metadata/ext.rb @@ -14,6 +14,10 @@ module Ext # Type of operation being performed (e.g. ) TAG_OPERATION = 'operation' # Hostname of external service interacted with + # + # This tag also doesn't strictly need to be a “hostname”. It can be a raw IP address and in some cases it + # can even be a unix domain socket (i.e. postgres client setting host=/var/run/postgres). + # It should be whatever the client uses to point at the server it’s trying to talk to. TAG_PEER_HOSTNAME = 'peer.hostname' # Name of external service that performed the work TAG_PEER_SERVICE = 'peer.service' diff --git a/sig/datadog/tracing/contrib/ext.rbs b/sig/datadog/tracing/contrib/ext.rbs index 6771cb8415a..c9ae3ec57ec 100644 --- a/sig/datadog/tracing/contrib/ext.rbs +++ b/sig/datadog/tracing/contrib/ext.rbs @@ -6,6 +6,7 @@ module Datadog PEER_SERVICE_SOURCES: Array[string] TAG_INSTANCE: "db.instance" + TAG_PEER_DB_NAME: string TAG_USER: "db.user" TAG_SYSTEM: "db.system" diff --git a/sig/datadog/tracing/contrib/propagation/sql_comment.rbs b/sig/datadog/tracing/contrib/propagation/sql_comment.rbs index b803269f682..1f28f3526dc 100644 --- a/sig/datadog/tracing/contrib/propagation/sql_comment.rbs +++ b/sig/datadog/tracing/contrib/propagation/sql_comment.rbs @@ -4,7 +4,7 @@ module Datadog module Propagation module SqlComment def self.annotate!: (untyped span_op, untyped mode) -> (nil | untyped) - def self.prepend_comment: (untyped sql, untyped span_op, untyped trace_op, untyped mode) -> (untyped | ::String) + def self.prepend_comment: (String sql, SpanOperation span_op, TraceOperation trace_op, untyped mode) -> String def self.datadog_configuration: () -> untyped end diff --git a/sig/datadog/tracing/contrib/propagation/sql_comment/ext.rbs b/sig/datadog/tracing/contrib/propagation/sql_comment/ext.rbs index 2f2affe407f..bb75fdbde97 100644 --- a/sig/datadog/tracing/contrib/propagation/sql_comment/ext.rbs +++ b/sig/datadog/tracing/contrib/propagation/sql_comment/ext.rbs @@ -6,6 +6,9 @@ module Datadog module Ext ENV_DBM_PROPAGATION_MODE: "DD_DBM_PROPAGATION_MODE" DISABLED: "disabled" + KEY_DB_NAME: string + KEY_HOSTNAME: string + KEY_PEER_SERVICE: string SERVICE: "service" FULL: "full" TAG_DBM_TRACE_INJECTED: "_dd.dbm_trace_injected" diff --git a/sig/datadog/tracing/contrib/propagation/sql_comment/mode.rbs b/sig/datadog/tracing/contrib/propagation/sql_comment/mode.rbs index 50b5df36f43..1f2f2d80bb7 100644 --- a/sig/datadog/tracing/contrib/propagation/sql_comment/mode.rbs +++ b/sig/datadog/tracing/contrib/propagation/sql_comment/mode.rbs @@ -3,7 +3,13 @@ module Datadog module Contrib module Propagation module SqlComment - Mode: untyped + class Mode < ::Struct[String] + attr_accessor mode: String + + def enabled?: -> bool + def service?: -> bool + def full?: -> bool + end end end end diff --git a/spec/datadog/tracing/contrib/active_record/tracer_spec.rb b/spec/datadog/tracing/contrib/active_record/tracer_spec.rb index ae3a43e4fb7..e599b42b88c 100644 --- a/spec/datadog/tracing/contrib/active_record/tracer_spec.rb +++ b/spec/datadog/tracing/contrib/active_record/tracer_spec.rb @@ -48,6 +48,7 @@ expect(span.type).to eq('sql') expect(span.resource.strip).to eq('SELECT COUNT(*) FROM `articles`') expect(span.get_tag('active_record.db.vendor')).to eq('mysql2') + expect(span.get_tag('db.instance')).to eq('mysql') expect(span.get_tag('active_record.db.name')).to eq('mysql') expect(span.get_tag('active_record.db.cached')).to eq(nil) expect(span.get_tag('out.host')).to eq(ENV.fetch('TEST_MYSQL_HOST', '127.0.0.1')) diff --git a/spec/datadog/tracing/contrib/mysql2/patcher_spec.rb b/spec/datadog/tracing/contrib/mysql2/patcher_spec.rb index f7380c8ff43..b3bba5563cc 100644 --- a/spec/datadog/tracing/contrib/mysql2/patcher_spec.rb +++ b/spec/datadog/tracing/contrib/mysql2/patcher_spec.rb @@ -99,6 +99,7 @@ expect(spans.count).to eq(1) expect(span.get_tag('span.kind')).to eq('client') + expect(span.get_tag('db.instance')).to eq(database) expect(span.get_tag('mysql2.db.name')).to eq(database) expect(span.get_tag('out.host')).to eq(host) expect(span.get_tag('out.port')).to eq(port) diff --git a/spec/datadog/tracing/contrib/propagation/sql_comment_spec.rb b/spec/datadog/tracing/contrib/propagation/sql_comment_spec.rb index 24349aaa5d5..87df9b402d1 100644 --- a/spec/datadog/tracing/contrib/propagation/sql_comment_spec.rb +++ b/spec/datadog/tracing/contrib/propagation/sql_comment_spec.rb @@ -5,7 +5,7 @@ let(:propagation_mode) { Datadog::Tracing::Contrib::Propagation::SqlComment::Mode.new(mode) } describe '.annotate!' do - let(:span_op) { Datadog::Tracing::SpanOperation.new('sql_comment_propagation_span', service: 'database_service') } + let(:span_op) { Datadog::Tracing::SpanOperation.new('sql_comment_propagation_span', service: 'db_service') } context 'when `disabled` mode' do let(:mode) { 'disabled' } @@ -50,16 +50,16 @@ context 'when tracing is enabled' do before do Datadog.configure do |c| - c.env = 'production' - c.service = "Traders' Joe" - c.version = '1.0.0' + c.env = 'dev' + c.service = 'api' + c.version = '1.2' end end let(:span_op) do Datadog::Tracing::SpanOperation.new( 'sample_span', - service: 'database_service' + service: 'db_service' ) end let(:trace_op) do @@ -85,7 +85,7 @@ it do is_expected.to eq( - "/*dddbs='database_service',dde='production',ddps='Traders%27%20Joe',ddpv='1.0.0'*/ #{sql_statement}" + "/*dde='dev',ddps='api',ddpv='1.2',dddbs='db_service'*/ #{sql_statement}" ) end @@ -93,14 +93,62 @@ let(:span_op) do Datadog::Tracing::SpanOperation.new( 'sample_span', - service: 'database_service', - tags: { 'peer.service' => 'sample_peer_service' } + service: 'db_service', + tags: { 'peer.service' => 'db_peer_service' } ) end it do is_expected.to eq( - "/*dddbs='sample_peer_service',dde='production',ddps='Traders%27%20Joe',ddpv='1.0.0'*/ #{sql_statement}" + "/*dde='dev',ddps='api',ddpv='1.2',ddprs='db_peer_service',dddbs='db_peer_service'*/ #{sql_statement}" + ) + end + + context 'matching the global service' do + let(:span_op) do + Datadog::Tracing::SpanOperation.new( + 'sample_span', + service: 'db_service', + tags: { 'peer.service' => 'api' } + ) + end + + it 'omits the redundant dddbs' do + is_expected.to eq( + "/*dde='dev',ddps='api',ddpv='1.2',ddprs='api'*/ #{sql_statement}" + ) + end + end + end + + context 'when given a span operation tagged with db.instance' do + let(:span_op) do + Datadog::Tracing::SpanOperation.new( + 'sample_span', + service: 'db_service', + tags: { 'db.instance' => 'db_name' } + ) + end + + it do + is_expected.to eq( + "/*dde='dev',ddps='api',ddpv='1.2',dddb='db_name',dddbs='db_service'*/ #{sql_statement}" + ) + end + end + + context 'when given a span operation tagged with peer.hostname' do + let(:span_op) do + Datadog::Tracing::SpanOperation.new( + 'sample_span', + service: 'db_service', + tags: { 'peer.hostname' => 'db_host' } + ) + end + + it do + is_expected.to eq( + "/*dde='dev',ddps='api',ddpv='1.2',ddh='db_host',dddbs='db_service'*/ #{sql_statement}" ) end end @@ -112,10 +160,11 @@ it { is_expected.to eq( - "/*dddbs='database_service',"\ - "dde='production',"\ - "ddps='Traders%27%20Joe',"\ - "ddpv='1.0.0',"\ + '/*'\ + "dde='dev',"\ + "ddps='api',"\ + "ddpv='1.2',"\ + "dddbs='db_service',"\ "traceparent='#{traceparent}'*/ "\ "#{sql_statement}" ) @@ -125,17 +174,19 @@ let(:span_op) do Datadog::Tracing::SpanOperation.new( 'sample_span', - service: 'database_service', - tags: { 'peer.service' => 'sample_peer_service' } + service: 'db_service', + tags: { 'peer.service' => 'db_peer_service' } ) end it { is_expected.to eq( - "/*dddbs='sample_peer_service',"\ - "dde='production',"\ - "ddps='Traders%27%20Joe',"\ - "ddpv='1.0.0',"\ + '/*'\ + "dde='dev',"\ + "ddps='api',"\ + "ddpv='1.2',"\ + "ddprs='db_peer_service',"\ + "dddbs='db_peer_service',"\ "traceparent='#{traceparent}'*/ "\ "#{sql_statement}" ) @@ -147,9 +198,9 @@ describe 'when propagates with `full` mode but tracing is disabled ' do before do Datadog.configure do |c| - c.env = 'production' - c.service = "Traders' Joe" - c.version = '1.0.0' + c.env = 'dev' + c.service = 'api' + c.version = '1.2' c.tracing.enabled = false end end @@ -160,7 +211,7 @@ result = nil Datadog::Tracing.trace('dummy.sql') do |span_op, trace_op| - span_op.service = 'database_service' + span_op.service = 'db_service' result = described_class.prepend_comment(sql_statement, span_op, trace_op, propagation_mode) end @@ -170,7 +221,7 @@ it do is_expected.to eq( - "/*dddbs='database_service',dde='production',ddps='Traders%27%20Joe',ddpv='1.0.0'*/ #{sql_statement}" + "/*dde='dev',ddps='api',ddpv='1.2',dddbs='db_service'*/ #{sql_statement}" ) end diff --git a/spec/datadog/tracing/contrib/rails/database_spec.rb b/spec/datadog/tracing/contrib/rails/database_spec.rb index 975f4fd7b4d..0de3cc3bfd3 100644 --- a/spec/datadog/tracing/contrib/rails/database_spec.rb +++ b/spec/datadog/tracing/contrib/rails/database_spec.rb @@ -45,6 +45,7 @@ expect(span.type).to eq('sql') expect(span.service).to eq(adapter_name) expect(span.get_tag('active_record.db.vendor')).to eq(adapter_name) + expect(span.get_tag('db.instance')).to eq(database_name) expect(span.get_tag('active_record.db.name')).to eq(database_name) expect(span.get_tag('active_record.db.cached')).to be_nil expect(adapter_host.to_s).to eq(span.get_tag('out.host')) diff --git a/spec/datadog/tracing/contrib/trilogy/patcher_spec.rb b/spec/datadog/tracing/contrib/trilogy/patcher_spec.rb index c7641f4b070..0ba135328f0 100644 --- a/spec/datadog/tracing/contrib/trilogy/patcher_spec.rb +++ b/spec/datadog/tracing/contrib/trilogy/patcher_spec.rb @@ -85,6 +85,7 @@ expect(spans.count).to eq(1) expect(span.get_tag('span.kind')).to eq('client') + expect(span.get_tag('db.instance')).to eq(database) expect(span.get_tag('trilogy.db.name')).to eq(database) expect(span.get_tag('out.host')).to eq(host) expect(span.get_tag('out.port')).to eq(port.to_f)