From f45244fb8cd96fe817abe31467576e6556a84705 Mon Sep 17 00:00:00 2001 From: Marco Costa Date: Thu, 6 Jun 2024 15:08:37 -0700 Subject: [PATCH] Add option to inherit service name from the parent span --- docs/GettingStarted.md | 3 +- lib/datadog/tracing/component.rb | 1 + lib/datadog/tracing/configuration/settings.rb | 15 ++++ lib/datadog/tracing/span_operation.rb | 17 +++-- lib/datadog/tracing/trace_operation.rb | 11 ++- lib/datadog/tracing/tracer.rb | 13 +++- .../core/configuration/components_spec.rb | 2 + .../tracing/configuration/settings_spec.rb | 31 ++++++++ spec/datadog/tracing/span_operation_spec.rb | 48 +++++++++++-- spec/datadog/tracing/trace_operation_spec.rb | 31 +++++++- spec/datadog/tracing/tracer_spec.rb | 72 ++++++++++++++----- 11 files changed, 211 insertions(+), 33 deletions(-) diff --git a/docs/GettingStarted.md b/docs/GettingStarted.md index 85334c526d0..be06259e2ab 100644 --- a/docs/GettingStarted.md +++ b/docs/GettingStarted.md @@ -1913,7 +1913,7 @@ For example, if `tracing.sampling.default_rate` is configured by [Remote Configu **Available configuration options:** | Setting | Env Var | Type | Default | Description | -| ------------------------------------------------------ | ------------------------------------------------------- | ------------------------------------- | ---------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|--------------------------------------------------------|---------------------------------------------------------|---------------------------------------|------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | **Global** | | | | | | `agent.host` | `DD_AGENT_HOST` | `String` | `127.0.0.1` | Hostname of Agent to where trace data will be sent. | | `agent.port` | `DD_TRACE_AGENT_PORT` | `Integer` | `8126` | Port of Agent host to where trace data will be sent. If the [Agent configuration](#configuring-trace-data-ingestion) sets `receiver_port` or `DD_APM_RECEIVER_PORT` to something other than the default `8126`, then `DD_TRACE_AGENT_PORT` or `DD_TRACE_AGENT_URL` must match it. | @@ -1935,6 +1935,7 @@ For example, if `tracing.sampling.default_rate` is configured by [Remote Configu | `tracing.propagation_style` | `DD_TRACE_PROPAGATION_STYLE` | `Array` | `nil` | Distributed tracing propagation formats to extract and inject. See [Distributed Tracing](#distributed-tracing) for more details. | | `tracing.enabled` | `DD_TRACE_ENABLED` | `Bool` | `true` | Enables or disables tracing. If set to `false` instrumentation will still run, but no traces are sent to the trace agent. | | `tracing.header_tags` | `DD_TRACE_HEADER_TAGS` | `Array` | `nil` | Record HTTP headers as span tags. See [Applying header tags to root spans][header tags] for more information. | +| `tracing.inherit_parent_service` | | `Bool` | `false` | Inherit the parent span's service name when creating a new child span. | | `tracing.instrument(, )` | | | | Activates instrumentation for a specific library. See [Integration instrumentation](#integration-instrumentation) for more details. | | `tracing.log_injection` | `DD_LOGS_INJECTION` | `Bool` | `true` | Injects [Trace Correlation](#trace-correlation) information into Rails logs if present. Supports the default logger (`ActiveSupport::TaggedLogging`), `lograge`, and `semantic_logger`. | | `tracing.partial_flush.enabled` | | `Bool` | `false` | Enables or disables partial flushing. Partial flushing submits completed portions of a trace to the agent. Used when tracing instruments long running tasks (e.g. jobs) with many spans. | diff --git a/lib/datadog/tracing/component.rb b/lib/datadog/tracing/component.rb index 96c91ad48f2..d604b5d52b2 100644 --- a/lib/datadog/tracing/component.rb +++ b/lib/datadog/tracing/component.rb @@ -54,6 +54,7 @@ def build_tracer(settings, agent_settings, logger:) span_sampler: build_span_sampler(settings), writer: writer, tags: build_tracer_tags(settings), + inherit_parent_service: settings.tracing.inherit_parent_service ) end diff --git a/lib/datadog/tracing/configuration/settings.rb b/lib/datadog/tracing/configuration/settings.rb index 35ef08cac04..83b834781ef 100644 --- a/lib/datadog/tracing/configuration/settings.rb +++ b/lib/datadog/tracing/configuration/settings.rb @@ -144,6 +144,21 @@ def self.extended(base) o.setter { |header_tags, _| Configuration::HTTP::HeaderTags.new(header_tags) } end + # Whether spans inherit their parent's service name, when span `service` is not explicitly set. + # If `true`, the spans will use the service name from its parent span. + # If `false`, the spans will use the global service name (`c.service`). + # + # DEV3.0: All other tracer languages inherit the parent's service by default. + # DEV3.0: This option should be set to `true` by default in the next major version, + # DEV3.0: with a removal deprecation warning, then removed in 4.0. + # + # @default `false` + # @return [Boolean] + option :inherit_parent_service do |o| + o.default false + o.type :bool + end + # Enable 128 bit trace id generation. # # @default `DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED` environment variable, otherwise `true` diff --git a/lib/datadog/tracing/span_operation.rb b/lib/datadog/tracing/span_operation.rb index cb3edde7862..ff50ecda33e 100644 --- a/lib/datadog/tracing/span_operation.rb +++ b/lib/datadog/tracing/span_operation.rb @@ -48,7 +48,8 @@ def initialize( tags: nil, trace_id: nil, type: nil, - links: nil + links: nil, + inherit_parent_service: false ) # Ensure dynamically created strings are UTF-8 encoded. # @@ -90,6 +91,8 @@ def initialize( # Subscribe :on_error event @events.on_error.wrap_default(&on_error) if on_error.is_a?(Proc) + @inherit_parent_service = inherit_parent_service + # Start the span with start time, if given. start(start_time) if start_time end @@ -428,7 +431,8 @@ def message # it has been finished. attr_reader \ :events, - :span + :span, + :inherit_parent_service # Stored only for `service_entry` calculation. # Use `parent_id` for the effective parent span id. @@ -439,6 +443,11 @@ def message # mutation by reference; when this span is returned, # we don't want this SpanOperation to modify it further. def build_span + # Use parent service if no service is set and @inherit_parent_service is set + service = self.service || (@parent&.service if @inherit_parent_service) + + service_entry = @parent.nil? || (service && @parent.service != service) + Span.new( @name, duration: duration, @@ -448,13 +457,13 @@ def build_span metrics: Core::Utils::SafeDup.frozen_or_dup(metrics), parent_id: @parent_id, resource: @resource, - service: @service, + service: service, start_time: @start_time, status: @status, type: @type, trace_id: @trace_id, links: @links, - service_entry: parent.nil? || (service && parent.service != service) + service_entry: service_entry ) end diff --git a/lib/datadog/tracing/trace_operation.rb b/lib/datadog/tracing/trace_operation.rb index 615f8c2d68b..31d293529dc 100644 --- a/lib/datadog/tracing/trace_operation.rb +++ b/lib/datadog/tracing/trace_operation.rb @@ -70,12 +70,14 @@ def initialize( sampled: nil, sampling_priority: nil, service: nil, + default_service: nil, profiling_enabled: nil, tags: nil, metrics: nil, trace_state: nil, trace_state_unknown_fields: nil, - remote_parent: false + remote_parent: false, + inherit_parent_service: false ) # Attributes @id = id || Tracing::Utils::TraceId.next_id @@ -83,6 +85,7 @@ def initialize( @parent_span_id = parent_span_id @sampled = sampled.nil? ? true : sampled @remote_parent = remote_parent + @inherit_parent_service = inherit_parent_service # Tags @agent_sample_rate = agent_sample_rate @@ -95,6 +98,7 @@ def initialize( @sample_rate = sample_rate @sampling_priority = sampling_priority @service = service + @default_service = default_service @profiling_enabled = profiling_enabled @trace_state = trace_state @trace_state_unknown_fields = trace_state_unknown_fields @@ -169,7 +173,7 @@ def resource_override? end def service - @service || (root_span && root_span.service) + @service || (root_span && root_span.service) || @default_service end def measure( @@ -249,7 +253,8 @@ def build_span( start_time: start_time, tags: tags, trace_id: trace_id, - type: type + type: type, + inherit_parent_service: @inherit_parent_service ) rescue StandardError => e Datadog.logger.debug { "Failed to build new span: #{e}" } diff --git a/lib/datadog/tracing/tracer.rb b/lib/datadog/tracing/tracer.rb index b9f076bb346..26d319eea5f 100644 --- a/lib/datadog/tracing/tracer.rb +++ b/lib/datadog/tracing/tracer.rb @@ -58,7 +58,8 @@ def initialize( ), span_sampler: Sampling::Span::Sampler.new, tags: {}, - writer: Writer.new + writer: Writer.new, + inherit_parent_service: false ) @trace_flush = trace_flush @default_service = default_service @@ -68,6 +69,7 @@ def initialize( @span_sampler = span_sampler @tags = tags @writer = writer + @inherit_parent_service = inherit_parent_service end # Return a {Datadog::Tracing::SpanOperation span_op} and {Datadog::Tracing::TraceOperation trace_op} @@ -327,12 +329,16 @@ def build_trace(digest = nil) trace_state: digest.trace_state, trace_state_unknown_fields: digest.trace_state_unknown_fields, remote_parent: digest.span_remote, + default_service: @default_service, + inherit_parent_service: @inherit_parent_service, ) else TraceOperation.new( hostname: hostname, profiling_enabled: profiling_enabled, remote_parent: false, + default_service: @default_service, + inherit_parent_service: @inherit_parent_service, ) end end @@ -341,12 +347,13 @@ def bind_trace_events!(trace_op) events = trace_op.send(:events) events.span_before_start.subscribe do |event_span_op, event_trace_op| - event_trace_op.service ||= @default_service - event_span_op.service ||= @default_service sample_trace(event_trace_op) if event_span_op && event_span_op.parent_id == 0 end events.span_finished.subscribe do |event_span, event_trace_op| + # Fallback in case the service was never set + event_span.service ||= @default_service + sample_span(event_trace_op, event_span) flush_trace(event_trace_op) end diff --git a/spec/datadog/core/configuration/components_spec.rb b/spec/datadog/core/configuration/components_spec.rb index e04fad93fcf..35ec8da70e6 100644 --- a/spec/datadog/core/configuration/components_spec.rb +++ b/spec/datadog/core/configuration/components_spec.rb @@ -425,6 +425,7 @@ end end let(:span_sampler) { be_a(Datadog::Tracing::Sampling::Span::Sampler) } + let(:inherit_parent_service) { false } let(:default_options) do { default_service: settings.service, @@ -434,6 +435,7 @@ sampler: sampler, span_sampler: span_sampler, writer: writer, + inherit_parent_service: inherit_parent_service, } end diff --git a/spec/datadog/tracing/configuration/settings_spec.rb b/spec/datadog/tracing/configuration/settings_spec.rb index 32d302d3f98..ef8f01a5b2e 100644 --- a/spec/datadog/tracing/configuration/settings_spec.rb +++ b/spec/datadog/tracing/configuration/settings_spec.rb @@ -284,6 +284,37 @@ def propagation_style_inject end end + describe '#inherit_parent_service' do + subject(:inherit_parent_service) { settings.tracing.inherit_parent_service } + + it { is_expected.to be false } + + context 'when inherit_parent_service' do + before { settings.tracing.inherit_parent_service = value } + + context 'is set to true' do + let(:value) { true } + + it { is_expected.to be true } + end + + context 'is set to false' do + let(:value) { false } + + it { is_expected.to be false } + end + end + end + + describe '#inherit_parent_service=' do + it 'updates the #inherit_parent_service setting' do + expect { settings.tracing.inherit_parent_service = true } + .to change { settings.tracing.inherit_parent_service } + .from(false) + .to(true) + end + end + describe '#header_tags' do subject(:header_tags) { settings.tracing.header_tags } diff --git a/spec/datadog/tracing/span_operation_spec.rb b/spec/datadog/tracing/span_operation_spec.rb index a8de6c79873..06076ee531b 100644 --- a/spec/datadog/tracing/span_operation_spec.rb +++ b/spec/datadog/tracing/span_operation_spec.rb @@ -462,7 +462,7 @@ end context 'identifying service_entry_span' do - context 'when service of root and child are `nil`' do + context 'when service of parent and child are `nil`' do it do root_span_op = described_class.new('root') child_span_op = described_class.new('child_1') @@ -482,7 +482,7 @@ end end - context 'when service of root and child are identical' do + context 'when service of parent and child are identical' do it do root_span_op = described_class.new('root', service: 'root_service') child_span_op = described_class.new('child_1', service: root_span_op.service) @@ -502,7 +502,7 @@ end end - context 'when service of root and child are different' do + context 'when service of parent and child are different' do it do root_span_op = described_class.new('root') child_span_op = described_class.new('child_1', service: 'child_service') @@ -522,7 +522,7 @@ end end - context 'when service of root and child are different, overriden within the measure block' do + context 'when service of parent and child are different, overriden within the measure block' do it do root_span_op = described_class.new('root') child_span_op = described_class.new('child_1') @@ -806,6 +806,46 @@ expect(span_op.finish).to be(original_span) end end + + context 'with a parent span' do + let(:parent_span) { described_class.new('parent', service: 'parent_service') } + + before do + span_op.send(:parent=, parent_span) + end + + context 'with no service set' do + it 'span has no service' do + expect(finish.service).to eq(nil) + end + end + + context 'with a service set' do + let(:parent_service) { 'parent_service' } + + context 'without inherit_parent_service set' do + it 'span does not inherit the service' do + expect(finish.service).to eq(nil) + end + end + + context 'with inherit_parent_service set' do + let(:options) { { inherit_parent_service: true } } + + it 'span inherits the service' do + expect(finish.service).to eq('parent_service') + end + end + + context 'and child span service set' do + let(:options) { { service: 'child_service' } } + + it 'span does not inherit the service' do + expect(finish.service).to eq('child_service') + end + end + end + end end describe '#finished?' do diff --git a/spec/datadog/tracing/trace_operation_spec.rb b/spec/datadog/tracing/trace_operation_spec.rb index 64e4de4ac26..82aed27f059 100644 --- a/spec/datadog/tracing/trace_operation_spec.rb +++ b/spec/datadog/tracing/trace_operation_spec.rb @@ -32,12 +32,14 @@ sampled: sampled, sampling_priority: sampling_priority, service: service, + default_service: default_service, profiling_enabled: profiling_enabled, tags: tags, metrics: metrics, trace_state: trace_state, trace_state_unknown_fields: trace_state_unknown_fields, remote_parent: remote_parent, + inherit_parent_service: inherit_parent_service } end @@ -53,6 +55,7 @@ let(:sampled) { true } let(:sampling_priority) { Datadog::Tracing::Sampling::Ext::Priority::USER_KEEP } let(:service) { 'billing-api' } + let(:default_service) { 'default-service' } let(:profiling_enabled) { 'profiling_enabled' } let(:tags) { { 'foo' => 'bar' }.merge(distributed_tags) } let(:metrics) { { 'baz' => 42.0 } } @@ -61,6 +64,7 @@ let(:distributed_tags) { { '_dd.p.test' => 'value' } } let(:remote_parent) { true } + let(:inherit_parent_service) { double('inherit_parent_service') } end shared_examples 'a span with default events' do @@ -236,9 +240,26 @@ context ':service' do subject(:options) { { service: service } } - let(:service) { 'billing-worker' } - it { expect(trace_op.service).to eq(service) } + context 'with no service set' do + let(:service) { nil } + it { expect(trace_op.service).to eq('default-service') } + end + + context 'with a root span with service set' do + let(:service) { nil } + + before { trace_op.measure('root', service: 'root-service') {} } + + it { expect(trace_op.service).to eq('root-service') } + + context 'with a trace service set' do + let(:service) { 'service' } + it { expect(trace_op.service).to eq('service') } + end + end + + include_context 'trace attributes' end context ':tags' do @@ -1221,6 +1242,12 @@ it { expect(span.send(:meta)).to include('foo' => 'bar') } end end + + context ':inherit_parent_service' do + include_context 'trace attributes' + + it { expect(span.send(:inherit_parent_service)).to eq(inherit_parent_service) } + end end context 'when the trace' do diff --git a/spec/datadog/tracing/tracer_spec.rb b/spec/datadog/tracing/tracer_spec.rb index cbed78e91f5..cc444ae2746 100644 --- a/spec/datadog/tracing/tracer_spec.rb +++ b/spec/datadog/tracing/tracer_spec.rb @@ -186,6 +186,20 @@ expect(span.name).to eq(name) end + it 'passes options to the trace operation' do + expect(Datadog::Profiling).to receive(:enabled?).and_return(true) + + expect(Datadog::Tracing::TraceOperation).to receive(:new).with( + hostname: nil, + profiling_enabled: true, + remote_parent: false, + default_service: tracer.default_service, + inherit_parent_service: false + ).and_call_original + + trace + end + context 'with diagnostics debug enabled' do include_context 'tracer logging' @@ -259,29 +273,55 @@ end context 'when nesting spans' do - it 'propagates parent span and uses default service name' do + subject!(:nested_spans) do tracer.trace('parent', service: 'service-parent') do tracer.trace('child1') { |s| s.set_tag('tag', 'tag_1') } tracer.trace('child2', service: 'service-child2') { |s| s.set_tag('tag', 'tag_2') } end + end - expect(spans).to have(3).items + context 'when inherit_parent_service is false' do + it 'propagates parent span and uses default service name' do + expect(spans).to have(3).items - child1, child2, parent = spans # Spans are sorted alphabetically by operation name + child1, child2, parent = spans # Spans are sorted alphabetically by operation name - expect(parent).to be_root_span - expect(parent.name).to eq('parent') - expect(parent.service).to eq('service-parent') - - expect(child1.parent_id).to be(parent.id) - expect(child1.name).to eq('child1') - expect(child1.service).to eq(tracer.default_service) - expect(child1.get_tag('tag')).to eq('tag_1') - - expect(child2.parent_id).to be(parent.id) - expect(child2.name).to eq('child2') - expect(child2.service).to eq('service-child2') - expect(child2.get_tag('tag')).to eq('tag_2') + expect(parent).to be_root_span + expect(parent.name).to eq('parent') + expect(parent.service).to eq('service-parent') + + expect(child1.parent_id).to be(parent.id) + expect(child1.name).to eq('child1') + expect(child1.service).to eq(tracer.default_service) + expect(child1.get_tag('tag')).to eq('tag_1') + + expect(child2.parent_id).to be(parent.id) + expect(child2.name).to eq('child2') + expect(child2.service).to eq('service-child2') + expect(child2.get_tag('tag')).to eq('tag_2') + end + end + + context 'when inherit_parent_service is true' do + let(:tracer_options) { super().merge(inherit_parent_service: true) } + + it "propagates parent span and uses parent's service name" do + expect(spans).to have(3).items + + child1, child2, parent = spans # Spans are sorted alphabetically by operation name + + expect(parent).to be_root_span + expect(parent.name).to eq('parent') + expect(parent.service).to eq('service-parent') + + expect(child1.parent_id).to be(parent.id) + expect(child1.name).to eq('child1') + expect(child1.service).to eq('service-parent') + + expect(child2.parent_id).to be(parent.id) + expect(child2.name).to eq('child2') + expect(child2.service).to eq('service-child2') + end end it 'trace has a runtime ID and PID tags' do