Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default value from peer.service tag #3846

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions lib/datadog/core/telemetry/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ def configuration
config.tracing.contrib.global_default_service_name.enabled,
seq_id
),
conf_value(
'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED',
config.tracing.contrib.peer_service_defaults,
seq_id
),
]

peer_service_mapping_str = ''
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/tracing/configuration/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module Ext
# @public_api
module SpanAttributeSchema
ENV_GLOBAL_DEFAULT_SERVICE_NAME_ENABLED = 'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED'
ENV_PEER_SERVICE_DEFAULTS_ENABLED = 'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED'
ENV_PEER_SERVICE_MAPPING = 'DD_TRACE_PEER_SERVICE_MAPPING'
end

Expand Down
14 changes: 14 additions & 0 deletions lib/datadog/tracing/contrib/extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,20 @@ def self.included(base)
o.default({})
end

# Enables population of default in the `peer.service` span tag.
# Explicitly setting the `peer.service` for an integration will
# still be honored with this option disabled.
#
# Also when disabled, other peer service related configurations have no effect.
#
# @default `DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED` environment variable, otherwise `false`
# @return [Boolean]
option :peer_service_defaults do |o|
o.env Tracing::Configuration::Ext::SpanAttributeSchema::ENV_PEER_SERVICE_DEFAULTS_ENABLED
o.type :bool
o.default false
end

# Global service name behavior
settings :global_default_service_name do
# Overrides default service name to global service name
Expand Down
7 changes: 6 additions & 1 deletion lib/datadog/tracing/contrib/span_attribute_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ def self.fetch_service_name(env, default)
end

def self.set_peer_service!(span, sources)
config = Datadog.configuration.tracing.contrib

# If `peer_service_defaults` is disabled, we only read peer service from an explicitly set `peer.service` tag
sources = Datadog::Tracing::Contrib::SpanAttributeSchema::REFLEXIVE_SOURCES unless config.peer_service_defaults
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is sources set from two sources and then overwritten with one of the source?


# Acquire all peer.service values as well as any potential remapping
peer_service_val, peer_service_source = set_peer_service_from_source(span, sources)
remap_val = Datadog.configuration.tracing.contrib.peer_service_mapping[peer_service_val]
remap_val = config.peer_service_mapping[peer_service_val]

# Only continue to setting peer.service if actual source is found
return false unless peer_service_source
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/tracing/configuration/ext.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module Datadog
ENV_TRACE_ID_128_BIT_GENERATION_ENABLED: "DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED"
module SpanAttributeSchema
ENV_GLOBAL_DEFAULT_SERVICE_NAME_ENABLED: "DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED"
ENV_PEER_SERVICE_DEFAULTS_ENABLED: string
bouwkast marked this conversation as resolved.
Show resolved Hide resolved
ENV_PEER_SERVICE_MAPPING: "DD_TRACE_PEER_SERVICE_MAPPING"
end
module Analytics
Expand Down
4 changes: 2 additions & 2 deletions sig/datadog/tracing/contrib/extensions.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module Datadog
module Configuration
def configure: () ?{ () -> untyped } -> untyped
module Settings
InvalidIntegrationError: untyped
def instrument: (untyped integration_name, ?::Hash[untyped, untyped] options) ?{ () -> untyped } -> untyped
InvalidIntegrationError: StandardError
def instrument: (Symbol integration_name, ?::Hash[Symbol, untyped] options) ?{ () -> untyped } -> Contrib::Integration
alias use instrument
def []: (untyped integration_name, ?::Symbol key) -> untyped
def integrations_pending_activation: () -> untyped
Expand Down
14 changes: 6 additions & 8 deletions sig/datadog/tracing/contrib/span_attribute_schema.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@ module Datadog
module Tracing
module Contrib
module SpanAttributeSchema
PEER_SERVICE_SOURCE_DB: Array[string]
PEER_SERVICE_SOURCE_DEFAULT: Array[string]
PEER_SERVICE_SOURCE_MSG: Array[string]
PEER_SERVICE_SOURCE_RPC: Array[string]
NO_SOURCE: Array[string]
REFLEXIVE_SOURCES: Array[string]

def self?.fetch_service_name: (string env, string default) -> bool

def not_empty_tag?: (string) -> bool
def self.filter_peer_service_sources: (SpanOperation span, Array[string] sources) -> Array[string]

def set_peer_service: -> bool
def self.not_empty_tag?: (string tag) -> bool

def set_peer_service?: -> bool
def self.set_peer_service!: (SpanOperation span, Array[string] sources) -> bool

def set_peer_service_from_source: -> bool
def self.set_peer_service_from_source: (SpanOperation span, Array[string] sources) -> [string, string]
end
end
end
Expand Down
1 change: 1 addition & 0 deletions spec/datadog/core/telemetry/event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def contain_configuration(*array)
['DD_AGENT_TRANSPORT', 'TCP'],
['DD_TRACE_SAMPLE_RATE', '0.5'],
['DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED', true],
['DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED', false],
['DD_TRACE_PEER_SERVICE_MAPPING', 'foo:bar'],
['logger.level', 0],
['profiling.advanced.code_provenance_enabled', true],
Expand Down
24 changes: 24 additions & 0 deletions spec/datadog/tracing/contrib/extensions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,30 @@
end
end

describe '#peer_service_defaults' do
subject { settings.contrib.peer_service_defaults }

context 'when given environment variable DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED' do
around do |example|
ClimateControl.modify('DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED' => env_var) do
example.run
end
end

context 'is not defined' do
let(:env_var) { nil }

it { is_expected.to be false }
end

context 'is defined' do
let(:env_var) { 'true' }

it { is_expected.to be true }
end
end
end

describe '#global_default_service_name_enabled' do
subject { settings.contrib.global_default_service_name.enabled }

Expand Down
1 change: 0 additions & 1 deletion spec/datadog/tracing/contrib/http/miniapp_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
http_spans.each do |span|
expect(span.name).to eq('http.request')
expect(span.service).to eq('net/http')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_PEER_SERVICE)).to eq(host)
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_PEER_HOSTNAME)).to eq(host)
expect(span.resource).to eq('GET')
expect(span.get_tag('http.url')).to eq('/my/path')
Expand Down
15 changes: 14 additions & 1 deletion spec/datadog/tracing/contrib/integration_examples.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,25 @@
skip('No let(:peer_service_source) defined.') unless defined?(peer_service_source)
end

context 'extracted peer service' do
context 'with default peer services enabled' do
around do |example|
ClimateControl.modify('DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED' => 'true') do
example.run
end
end

it 'contains extracted peer service tag' do
expect(span.get_tag('peer.service')).to eq(peer_service_val)
expect(span.get_tag('_dd.peer.service.source')).to eq(peer_service_source)
end
end

context 'with default peer services disabled' do
it 'does not contain extracted peer service tag' do
expect(span.get_tag('peer.service')).to be_nil
expect(span.get_tag('_dd.peer.service.source')).to be_nil
end
end
end

RSpec.shared_examples 'a non-peer service span' do
Expand Down
1 change: 0 additions & 1 deletion spec/datadog/tracing/contrib/mysql2/patcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
expect(span.service).to eq(service_name)
expect(span.get_tag('span.kind')).to eq('client')
expect(span.get_tag('db.system')).to eq('mysql')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_PEER_SERVICE)).to eq(database)
end

it_behaves_like 'with sql comment propagation', span_op_name: 'mysql2.query'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,47 @@
end
end

context 'when env_var configured' do
it 'expects peer.service to equal env var value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq('configured_peer_service_via_env_var')
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
context 'with default peer services enabled' do
around do |example|
ClimateControl.modify('DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED' => 'true') do
example.run
end
end

context 'when env_var configured' do
it 'expects peer.service to equal env var value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq('configured_peer_service_via_env_var')
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
end
end

context 'when peer_service option is configured' do
let(:configuration_options) { { peer_service: 'configured_peer_service' } }

it 'expects peer.service to equal configured value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq(configuration_options[:peer_service])
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
end
end
end

context 'when peer_service option is configured' do
let(:configuration_options) { { peer_service: 'configured_peer_service' } }
context 'with default peer services disabled' do
# We still set the `peer.service` tag when it is explicitly configured

context 'when env_var configured' do
it 'expects peer.service to equal env var value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq('configured_peer_service_via_env_var')
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
end
end

context 'when peer_service option is configured' do
let(:configuration_options) { { peer_service: 'configured_peer_service' } }

it 'expects peer.service to equal configured value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq(configuration_options[:peer_service])
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
it 'expects peer.service to equal configured value and source to be peer.service' do
expect(span.get_tag('peer.service')).to eq(configuration_options[:peer_service])
expect(span.get_tag('_dd.peer.service.source')).to eq('peer.service')
end
end
end
end
Expand Down
Loading
Loading