Skip to content

Commit

Permalink
Merge pull request #3322 from DataDog/tonycthsu/correlation-string
Browse files Browse the repository at this point in the history
Return string for `trace_id` and `span_id` from `Correlation::Identifier`
  • Loading branch information
TonyCTHsu authored Jan 26, 2024
2 parents 92e17f8 + 5e1a480 commit af8859f
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 100 deletions.
35 changes: 4 additions & 31 deletions lib/datadog/tracing/correlation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ module Datadog
module Tracing
# Contains behavior for managing correlations with tracing
# e.g. Retrieve a correlation to the current trace for logging, etc.
# This class is for usage with log correlation.
# To continue from a trace, users should use TraceDigest instead.
module Correlation
# Represents current trace state with key identifiers
# @public_api
Expand All @@ -21,42 +23,21 @@ class Identifier
:env,
:service,
:span_id,
:span_name,
:span_resource,
:span_service,
:span_type,
:trace_name,
:trace_resource,
:trace_service,
:version

# @!visibility private
def initialize(
env: nil,
service: nil,
span_id: nil,
span_name: nil,
span_resource: nil,
span_service: nil,
span_type: nil,
trace_id: nil,
trace_name: nil,
trace_resource: nil,
trace_service: nil,
version: nil
)
# Dup and freeze strings so they aren't modified by reference.
@env = Core::Utils::SafeDup.frozen_dup(env || Datadog.configuration.env)
@service = Core::Utils::SafeDup.frozen_dup(service || Datadog.configuration.service)
@span_id = span_id || 0
@span_name = Core::Utils::SafeDup.frozen_dup(span_name)
@span_resource = Core::Utils::SafeDup.frozen_dup(span_resource)
@span_service = Core::Utils::SafeDup.frozen_dup(span_service)
@span_type = Core::Utils::SafeDup.frozen_dup(span_type)
@span_id = (span_id || 0).to_s
@trace_id = trace_id || 0
@trace_name = Core::Utils::SafeDup.frozen_dup(trace_name)
@trace_resource = Core::Utils::SafeDup.frozen_dup(trace_resource)
@trace_service = Core::Utils::SafeDup.frozen_dup(trace_service)
@version = Core::Utils::SafeDup.frozen_dup(version || Datadog.configuration.version)
end

Expand Down Expand Up @@ -90,13 +71,12 @@ def to_log_format
end
end

# DEV-2.0: This public method was returning an Integer, but with 128 bit trace id it would return a String.
def trace_id
if Datadog.configuration.tracing.trace_id_128_bit_logging_enabled &&
!Tracing::Utils::TraceId.to_high_order(@trace_id).zero?
Kernel.format('%032x', @trace_id)
else
Tracing::Utils::TraceId.to_low_order(@trace_id)
Tracing::Utils::TraceId.to_low_order(@trace_id).to_s
end
end
end
Expand All @@ -113,14 +93,7 @@ def identifier_from_digest(digest)

Identifier.new(
span_id: digest.span_id,
span_name: digest.span_name,
span_resource: digest.span_resource,
span_service: digest.span_service,
span_type: digest.span_type,
trace_id: digest.trace_id,
trace_name: digest.trace_name,
trace_resource: digest.trace_resource,
trace_service: digest.trace_service
)
end
end
Expand Down
81 changes: 16 additions & 65 deletions spec/datadog/tracing/correlation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,8 @@
expect(identifier).to have_attributes(
env: default_env,
service: default_service,
span_id: 0,
span_name: nil,
span_resource: nil,
span_service: nil,
span_type: nil,
trace_id: 0,
trace_name: nil,
trace_resource: nil,
trace_service: nil,
span_id: '0',
trace_id: '0',
version: default_version
)
end
Expand Down Expand Up @@ -88,29 +81,15 @@
expect(identifier).to have_attributes(
env: default_env,
service: default_service,
span_id: span_id,
span_name: span_name,
span_resource: span_resource,
span_service: span_service,
span_type: span_type,
trace_id: low_order_trace_id(trace_id),
trace_name: trace_name,
trace_resource: trace_resource,
trace_service: trace_service,
span_id: span_id.to_s,
trace_id: low_order_trace_id(trace_id).to_s,
version: default_version
)
end

it 'has frozen copies of strings' do
expect(identifier.env).to be_a_frozen_copy_of(default_env)
expect(identifier.service).to be_a_frozen_copy_of(default_service)
expect(identifier.span_name).to be_a_frozen_copy_of(span_name)
expect(identifier.span_resource).to be_a_frozen_copy_of(span_resource)
expect(identifier.span_service).to be_a_frozen_copy_of(span_service)
expect(identifier.span_type).to be_a_frozen_copy_of(span_type)
expect(identifier.trace_name).to be_a_frozen_copy_of(trace_name)
expect(identifier.trace_resource).to be_a_frozen_copy_of(trace_resource)
expect(identifier.trace_service).to be_a_frozen_copy_of(trace_service)
expect(identifier.version).to be_a_frozen_copy_of(default_version)
end
end
Expand All @@ -125,15 +104,8 @@
expect(identifier).to have_attributes(
env: default_env,
service: default_service,
span_id: 0,
span_name: nil,
span_resource: nil,
span_service: nil,
span_type: nil,
trace_id: 0,
trace_name: nil,
trace_resource: nil,
trace_service: nil,
span_id: '0',
trace_id: '0',
version: default_version
)
end
Expand All @@ -153,14 +125,7 @@
env: env,
service: service,
span_id: span_id,
span_name: span_name,
span_resource: span_resource,
span_service: span_service,
span_type: span_type,
trace_id: trace_id,
trace_name: trace_name,
trace_resource: trace_resource,
trace_service: trace_service,
version: version
)
end
Expand All @@ -169,29 +134,15 @@
expect(identifier).to have_attributes(
env: env,
service: service,
span_id: span_id,
span_name: span_name,
span_resource: span_resource,
span_service: span_service,
span_type: span_type,
trace_id: low_order_trace_id(trace_id),
trace_name: trace_name,
trace_resource: trace_resource,
trace_service: trace_service,
span_id: span_id.to_s,
trace_id: low_order_trace_id(trace_id).to_s,
version: version
)
end

it 'has frozen copies of strings' do
expect(identifier.env).to be_a_frozen_copy_of(env)
expect(identifier.service).to be_a_frozen_copy_of(service)
expect(identifier.span_name).to be_a_frozen_copy_of(span_name)
expect(identifier.span_resource).to be_a_frozen_copy_of(span_resource)
expect(identifier.span_service).to be_a_frozen_copy_of(span_service)
expect(identifier.span_type).to be_a_frozen_copy_of(span_type)
expect(identifier.trace_name).to be_a_frozen_copy_of(trace_name)
expect(identifier.trace_resource).to be_a_frozen_copy_of(trace_resource)
expect(identifier.trace_service).to be_a_frozen_copy_of(trace_service)
expect(identifier.version).to be_a_frozen_copy_of(version)
end
end
Expand Down Expand Up @@ -509,22 +460,22 @@ def have_attribute(attribute)
context 'when given 64 bit trace id' do
it 'returns to lower 64 bits of trace id' do
trace_id = 0xaaaaaaaaaaaaaaaa
expected_trace_id = 0xaaaaaaaaaaaaaaaa

identifier = described_class.new(trace_id: trace_id)

expect(identifier.trace_id).to eq(expected_trace_id)
# `0xaaaaaaaaaaaaaaaa.to_s` => '12297829382473034410'
expect(identifier.trace_id).to eq('12297829382473034410')
end
end

context 'when given 128 bit trace id' do
it 'returns to lower 64 bits of trace id' do
trace_id = 0xaaaaaaaaaaaaaaaaffffffffffffffff
expected_trace_id = 0xffffffffffffffff

identifier = described_class.new(trace_id: trace_id)

expect(identifier.trace_id).to eq(expected_trace_id)
# `0xffffffffffffffff.to_s` => '18446744073709551615'
expect(identifier.trace_id).to eq('18446744073709551615')
end
end
end
Expand All @@ -537,11 +488,11 @@ def have_attribute(attribute)
context 'when given 64 bit trace id' do
it 'returns lower 64 bits of trace id' do
trace_id = 0xaaaaaaaaaaaaaaaa
expected_trace_id = 0xaaaaaaaaaaaaaaaa

identifier = described_class.new(trace_id: trace_id)

expect(identifier.trace_id).to eq(expected_trace_id)
# `0xaaaaaaaaaaaaaaaa.to_s` => '12297829382473034410'
expect(identifier.trace_id).to eq('12297829382473034410')
end
end

Expand All @@ -559,11 +510,11 @@ def have_attribute(attribute)
context 'when given > 64 bit trace id but high order is 0' do
it 'returns to lower 64 bits of trace id' do
trace_id = 0x00000000000000000aaaaaaaaaaaaaaaa
expected_trace_id = 0xaaaaaaaaaaaaaaaa

identifier = described_class.new(trace_id: trace_id)

expect(identifier.trace_id).to eq(expected_trace_id)
# `0xaaaaaaaaaaaaaaaa.to_s` => '12297829382473034410'
expect(identifier.trace_id).to eq('12297829382473034410')
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/datadog/tracing/tracer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -739,16 +739,16 @@
it 'produces an Identifier with data' do
is_expected.to be_a_kind_of(Datadog::Tracing::Correlation::Identifier)
expect(active_correlation.trace_id)
.to eq(low_order_trace_id(span.trace_id))
expect(active_correlation.span_id).to eq(span.id)
.to eq(low_order_trace_id(span.trace_id).to_s)
expect(active_correlation.span_id).to eq(span.id.to_s)
end
end

context 'when no trace is active' do
it 'produces an empty Identifier' do
is_expected.to be_a_kind_of(Datadog::Tracing::Correlation::Identifier)
expect(active_correlation.trace_id).to eq 0
expect(active_correlation.span_id).to eq 0
expect(active_correlation.trace_id).to eq '0'
expect(active_correlation.span_id).to eq '0'
end
end

Expand Down

0 comments on commit af8859f

Please sign in to comment.