Skip to content

Commit

Permalink
Propagation: Always parse W3C tracestate if present
Browse files Browse the repository at this point in the history
  • Loading branch information
marcotc committed Nov 9, 2023
1 parent 6a35f40 commit 3e047c6
Show file tree
Hide file tree
Showing 8 changed files with 197 additions and 36 deletions.
1 change: 1 addition & 0 deletions docs/GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -2064,6 +2064,7 @@ For example, if `tracing.sampling.default_rate` is configured by [Remote Configu
| `tracing.analytics.enabled` | `DD_TRACE_ANALYTICS_ENABLED` | `nil` | Enables or disables trace analytics. See [Sampling](#sampling) for more details. |
| `tracing.contrib.peer_service_mapping` | `DD_TRACE_PEER_SERVICE_MAPPING` | `nil` | Defines remapping of `peer.service` tag across all instrumentation. Provide a list of `old_value1:new_value1, old_value2:new_value2, ...` |
| `tracing.contrib.global_default_service_name.enabled` | `DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED` | `false` | Changes the default value for `service_name` to the application service name across all instrumentation |
| `tracing.distributed_tracing.propagation_extract_first` | `DD_TRACE_PROPAGATION_EXTRACT_FIRST` | `false` | Exit immediately on the first valid propagation format detected. See [Distributed Tracing](#distributed-tracing) for more details. |
| `tracing.distributed_tracing.propagation_extract_style` | `DD_TRACE_PROPAGATION_STYLE_EXTRACT` | `['Datadog','tracecontext']` | Distributed tracing propagation formats to extract. Overrides `DD_TRACE_PROPAGATION_STYLE`. See [Distributed Tracing](#distributed-tracing) for more details. |
| `tracing.distributed_tracing.propagation_inject_style` | `DD_TRACE_PROPAGATION_STYLE_INJECT` | `['Datadog','tracecontext']` | Distributed tracing propagation formats to inject. Overrides `DD_TRACE_PROPAGATION_STYLE`. See [Distributed Tracing](#distributed-tracing) for more details. |
| `tracing.distributed_tracing.propagation_style` | `DD_TRACE_PROPAGATION_STYLE` | `nil` | Distributed tracing propagation formats to extract and inject. See [Distributed Tracing](#distributed-tracing) for more details. |
Expand Down
3 changes: 3 additions & 0 deletions lib/datadog/tracing/configuration/ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ module Distributed
# @see https://opentelemetry.io/docs/concepts/sdk-configuration/general-sdk-configuration/#get_otel__propagators
PROPAGATION_STYLE_NONE = 'none'

# Strictly stop at the first successfully serialized style.
EXTRACT_FIRST = 'DD_TRACE_PROPAGATION_EXTRACT_FIRST'

ENV_X_DATADOG_TAGS_MAX_LENGTH = 'DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH'
end

Expand Down
11 changes: 11 additions & 0 deletions lib/datadog/tracing/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,17 @@ def self.extended(base)
set_option(:propagation_inject_style, styles)
end
end

# Strictly stop at the first successfully serialized style.
# This prevents the tracer from enriching the extracted context with information from
# other valid propagations styles present in the request.
# @default `DD_TRACE_PROPAGATION_EXTRACT_FIRST` environment variable, otherwise `false`.
# @return [Boolean]
option :propagation_extract_first do |o|
o.env Tracing::Configuration::Ext::Distributed::EXTRACT_FIRST
o.default false
o.type :bool
end
end

# Enable trace collection and span generation.
Expand Down
28 changes: 24 additions & 4 deletions lib/datadog/tracing/distributed/propagation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,39 @@ def extract(data)

extracted_trace_digest = nil

::Datadog.configuration.tracing.distributed_tracing.propagation_extract_style.each do |style|
config = ::Datadog.configuration.tracing.distributed_tracing

config.propagation_extract_style.each do |style|
propagator = @propagation_styles[style]
next if propagator.nil?

begin
extracted_trace_digest = propagator.extract(data)
if extracted_trace_digest
# Return if we are only inspecting the first valid style.
next if config.propagation_extract_first

# Continue parsing styles to find the W3C `tracestate` header, if present.
# `tracestate` must always be propagated, as it might contain pass-through data that we don't control.
# @see https://www.w3.org/TR/2021/REC-trace-context-1-20211123/#mutating-the-tracestate-field
next if style != Configuration::Ext::Distributed::PROPAGATION_STYLE_TRACE_CONTEXT

if (tracecontext_digest = propagator.extract(data))
# Only parse if it represent the same trace as the successfully extracted one
next unless tracecontext_digest.trace_id == extracted_trace_digest.trace_id

extracted_trace_digest = extracted_trace_digest.merge(
trace_state: tracecontext_digest.trace_state,
trace_state_unknown_fields: tracecontext_digest.trace_state_unknown_fields
)
end
else
extracted_trace_digest = propagator.extract(data)
end
rescue => e
::Datadog.logger.error(
"Error extracting distributed trace data. Cause: #{e} Location: #{Array(e.backtrace).first}"
)
end

break if extracted_trace_digest
end

extracted_trace_digest
Expand Down
30 changes: 30 additions & 0 deletions lib/datadog/tracing/trace_digest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,36 @@ def initialize(

freeze
end

# Creates a copy of this object, modifying the provided fields.
# @param field_value_pairs [Hash<String>] the fields to be overwritten
# @return [TraceDigest] returns a copy of this object with the `field_value_pairs` modified
def merge(field_value_pairs)
# DEV: Because we want to sometimes freeze an argument, it's best to let `#initialize`
# DEV: decide how to handle each field, instead of duplicating that logic here.
TraceDigest.new(
span_id: span_id,
span_name: span_name,
span_resource: span_resource,
span_service: span_service,
span_type: span_type,
trace_distributed_tags: trace_distributed_tags,
trace_hostname: trace_hostname,
trace_id: trace_id,
trace_name: trace_name,
trace_origin: trace_origin,
trace_process_id: trace_process_id,
trace_resource: trace_resource,
trace_runtime_id: trace_runtime_id,
trace_sampling_priority: trace_sampling_priority,
trace_service: trace_service,
trace_distributed_id: trace_distributed_id,
trace_flags: trace_flags,
trace_state: trace_state,
trace_state_unknown_fields: trace_state_unknown_fields,
**field_value_pairs
)
end
end
end
end
37 changes: 37 additions & 0 deletions spec/datadog/tracing/configuration/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,43 @@ def propagation_inject_style
end
end
end

describe '#propagation_extract_first' do
subject(:propagation_extract_first) { settings.tracing.distributed_tracing.propagation_extract_first }

let(:var_value) { nil }
let(:var_name) { 'DD_TRACE_PROPAGATION_EXTRACT_FIRST' }
it { is_expected.to be false }

context 'when DD_TRACE_PROPAGATION_EXTRACT_FIRST' do
context 'is not defined' do
let(:var_value) { nil }

it { is_expected.to be false }
end

context 'is set to true' do
let(:var_value) { 'true' }

it { is_expected.to be true }
end

context 'is set to false' do
let(:var_value) { 'false' }

it { is_expected.to be false }
end
end

describe '#propagation_extract_first=' do
it 'updates the #propagation_extract_first setting' do
expect { settings.tracing.distributed_tracing.propagation_extract_first = true }
.to change { settings.tracing.distributed_tracing.propagation_extract_first }
.from(false)
.to(true)
end
end
end
end

describe '#enabled' do
Expand Down
107 changes: 75 additions & 32 deletions spec/datadog/tracing/distributed/propagation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,56 +228,99 @@
end

context 'datadog, and tracecontext header' do
let(:data) do
{
prepare_key['x-datadog-trace-id'] => '61185',
prepare_key['x-datadog-parent-id'] => '73456',
prepare_key['traceparent'] => '00-11111111111111110000000000000001-000000003ade68b1-01',
}
end

it do
expect(trace_digest).to be_a_kind_of(Datadog::Tracing::TraceDigest)
expect(trace_digest.span_id).to eq(73456)
expect(trace_digest.trace_id).to eq(61185)
expect(trace_digest.trace_sampling_priority).to be nil
end

context 'and sampling priority' do
context 'with trace_id not matching' do
let(:data) do
{
prepare_key['x-datadog-trace-id'] => '61185',
prepare_key['x-datadog-parent-id'] => '73456',
prepare_key['x-datadog-sampling-priority'] => '1',
prepare_key['traceparent'] => '00-00000000000000000000000000c0ffee-0000000000000bee-00',
prepare_key['traceparent'] => '00-11111111111111110000000000000001-000000003ade68b1-01',
}
end

it do
expect(trace_digest).to be_a_kind_of(Datadog::Tracing::TraceDigest)
expect(trace_digest.span_id).to eq(73456)
expect(trace_digest.trace_id).to eq(61185)
expect(trace_digest.trace_sampling_priority).to eq(1)
expect(trace_digest.trace_sampling_priority).to be nil
end

context 'and sampling priority' do
let(:data) do
{
prepare_key['x-datadog-trace-id'] => '61185',
prepare_key['x-datadog-parent-id'] => '73456',
prepare_key['x-datadog-sampling-priority'] => '1',
prepare_key['traceparent'] => '00-00000000000000000000000000c0ffee-0000000000000bee-00',
}
end

it do
expect(trace_digest).to be_a_kind_of(Datadog::Tracing::TraceDigest)
expect(trace_digest.span_id).to eq(73456)
expect(trace_digest.trace_id).to eq(61185)
expect(trace_digest.trace_sampling_priority).to eq(1)
end

context 'with a failing propagator (Datadog)' do
let(:error) { StandardError.new('test_err').tap { |e| e.set_backtrace('caller:1') } }

before do
allow_any_instance_of(::Datadog::Tracing::Distributed::Datadog).to receive(:extract).and_raise(error)
allow(Datadog.logger).to receive(:error)
end

it 'does not propagate error to caller' do
trace_digest
expect(Datadog.logger).to have_received(:error).with(/Cause: test_err Location: caller:1/)
end

it 'extracts values from non-failing propagator (tracecontext)' do
expect(trace_digest).to be_a_kind_of(Datadog::Tracing::TraceDigest)
expect(trace_digest.span_id).to eq(0xbee)
expect(trace_digest.trace_id).to eq(0xc0ffee)
expect(trace_digest.trace_sampling_priority).to eq(0)
end
end
end

context 'with a failing propagator (Datadog)' do
let(:error) { StandardError.new('test_err').tap { |e| e.set_backtrace('caller:1') } }
context 'and tracestate' do
let(:data) { super().merge(prepare_key['tracestate'] => 'dd=unknown_field;,other=vendor') }

before do
allow_any_instance_of(::Datadog::Tracing::Distributed::Datadog).to receive(:extract).and_raise(error)
allow(Datadog.logger).to receive(:error)
it 'does not preserve tracestate' do
expect(trace_digest.trace_state).to be nil
expect(trace_digest.trace_state_unknown_fields).to be nil
end
end
end

it 'does not propagate error to caller' do
trace_digest
expect(Datadog.logger).to have_received(:error).with(/Cause: test_err Location: caller:1/)
context 'with a matching trace_id' do
let(:data) do
{
prepare_key['x-datadog-trace-id'] => '61185',
prepare_key['x-datadog-parent-id'] => '73456',
prepare_key['traceparent'] => '00-0000000000000000000000000000ef01-0000000000011ef0-01',
}
end

it 'does not parse tracecontext sampling priority' do
expect(trace_digest.trace_sampling_priority).to be nil
end

context 'and tracestate' do
let(:data) { super().merge(prepare_key['tracestate'] => 'dd=unknown_field;,other=vendor') }

it 'preserves tracestate' do
expect(trace_digest.trace_state).to eq('other=vendor')
expect(trace_digest.trace_state_unknown_fields).to eq('unknown_field;')
end

it 'extracts values from non-failing propagator (tracecontext)' do
expect(trace_digest).to be_a_kind_of(Datadog::Tracing::TraceDigest)
expect(trace_digest.span_id).to eq(0xbee)
expect(trace_digest.trace_id).to eq(0xc0ffee)
expect(trace_digest.trace_sampling_priority).to eq(0)
context 'with propagation_extract_first true' do
before { Datadog.configure { |c| c.tracing.distributed_tracing.propagation_extract_first = true } }

it 'does not preserve tracestate' do
expect(trace_digest.trace_state).to be nil
expect(trace_digest.trace_state_unknown_fields).to be nil
end
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions spec/datadog/tracing/trace_digest_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,20 @@
end
end
end

describe '#merge' do
let(:merge) { trace_digest.merge(args) }
let(:args) { { span_name: 'new span name' } }
let(:options) { { trace_name: 'trace name' } }

it 'overrides the provided field' do
expect(merge.span_name).to be_a_frozen_copy_of('new span name')
end

it 'does not modify non provided fields' do
expect(merge.trace_name).to eq('trace name')
end

it { is_expected.to be_frozen }
end
end

0 comments on commit 3e047c6

Please sign in to comment.