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.

Sign up for GitHub

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
+161 −61
Open

Remove default value from peer.service tag #3846

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccffd07
Remove default value from peer.service tag
marcotc Aug 14, 2024
f449211
Update comment on new option
marcotc Aug 15, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
11 changes: 11 additions & 0 deletions lib/datadog/tracing/contrib/extensions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@ def self.included(base)
o.default({})
end

# Enables population the `peer.service` tag.
# When disabled, other peer service related configurations have no effect.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to ask for this (as I'm not sure), but isn't it peer.service also supposed to be calculated for v1 schema (is that a thing in Ruby I don't actually see it in the repo?).

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I understand the question, maybe I don't know exactly what you mean by v1 schema.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, in .NET: https://github.com/DataDog/dd-trace-dotnet/blob/45a66abc870d1d45ce0778909162203e55dcfa2a/tracer/src/Datadog.Trace/Configuration/ConfigurationKeys.cs#L428

Maybe it isn't a thing for Ruby?
https://github.com/search?q=org%3ADataDog%20DD_TRACE_SPAN_ATTRIBUTE_SCHEMA&type=code

Ruby only has one use of it and it appears to be a fake usage to get system-tests working, so maybe this is just something that wasn't needed for Ruby?

dd-trace-rb/spec/datadog/tracing/contrib/span_attribute_schema_examples.rb

Line 7 in a534220

# setting DD_TRACE_SPAN_ATTRIBUTE_SCHEMA as the APM Test Agent relies on this ENV to run service naming assertions

#
# @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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
46 changes: 37 additions & 9 deletions spec/datadog/tracing/contrib/peer_service_configuration_examples.rb
View file
Open in desktop
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