Skip to content

Commit

Permalink
[core] Only require trace id for distributed tracing headers (#708)
Browse files Browse the repository at this point in the history
* [core] Only require trace id for distributed tracing headers

* Ignore parent_id if origin is 'synthetics'

* Update lib/ddtrace/propagation/distributed_headers.rb

* missing expected value

* ignore method length of test
  • Loading branch information
brettlangdon authored Mar 6, 2019
1 parent ca87476 commit fe4f7f7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
4 changes: 4 additions & 0 deletions lib/ddtrace/propagation/distributed_headers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ def initialize(env)
end

def valid?
# Synthetics sends us `X-Datadog-Parent-Id: 0` which normally we would want
# to filter out, but is ok in this context since there is no parent from Synthetics
return true if origin == 'synthetics' && trace_id

# Sampling priority and origin are optional.
trace_id && parent_id
end
Expand Down
19 changes: 14 additions & 5 deletions test/propagation/distributed_headers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'ddtrace/propagation/distributed_headers'

class DistributedHeadersTest < Minitest::Test
def test_valid_without_sampling_priority
def test_valid_without_sampling_priority # rubocop:disable Metrics/MethodLength
test_cases = {
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_ID' => '456' } => true,
Expand All @@ -20,9 +20,6 @@ def test_valid_without_sampling_priority
{ 'HTTP_X_DATADOG_TRACE_ID' => 'a',
'HTTP_X_DATADOG_PARENT_ID' => '456',
'HTTP_X_DATADOG_SAMPLING_PRIORITY' => '0' } => false,
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_ID' => 'b',
'HTTP_X_DATADOG_SAMPLING_PRIORITY' => '0' } => false,
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_ID' => '456',
'HTTP_X_DATADOG_SAMPLING_PRIORITY' => 'ooops' } => true, # corner case, 0 is valid for a sampling priority
Expand All @@ -31,7 +28,17 @@ def test_valid_without_sampling_priority
{ 'HTTP_X_DATADOG_TRACE_TYPO' => '123',
'HTTP_X_DATADOG_PARENT_ID' => '456' } => false,
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_TYPO' => '456' } => false
'HTTP_X_DATADOG_PARENT_ID' => 'b',
'HTTP_X_DATADOG_SAMPLING_PRIORITY' => '0' } => false,
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_TYPO' => '456' } => false,
# Parent id is not required when origin is synthetics
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_ID' => '0',
'HTTP_X_DATADOG_ORIGIN' => 'not-synthetics' } => false,
{ 'HTTP_X_DATADOG_TRACE_ID' => '123',
'HTTP_X_DATADOG_PARENT_ID' => '0',
'HTTP_X_DATADOG_ORIGIN' => 'synthetics' } => true
}

test_cases.each do |env, expected|
Expand Down Expand Up @@ -70,6 +77,8 @@ def test_parent_id
test_cases = {
{ 'HTTP_X_DATADOG_PARENT_ID' => '123' } => 123,
{ 'HTTP_X_DATADOG_PARENT_ID' => '0' } => nil,
{ 'HTTP_X_DATADOG_PARENT_ID' => 'a' } => nil,
{ 'HTTP_X_DATADOG_PARENT_ID' => '' } => nil,
{ 'HTTP_X_DATADOG_PARENT_ID' => '-1' } => 18446744073709551615,
{ 'HTTP_X_DATADOG_PARENT_ID' => '-8809075535603237910' } => 9637668538106313706,
{ 'HTTP_X_DATADOG_PARENT_ID' => 'ooops' } => nil,
Expand Down

0 comments on commit fe4f7f7

Please sign in to comment.