Prevent telemetry requests from being traced #2961
Conversation
github-actions
bot
added
core
| @@ -74,45 +74,19 @@ | |||
| it { is_expected.to be false } | |||
| end | |||
| end | |||
| end | |||
|
|
|||
| context 'integration' do | |||
There was a problem hiding this comment.
This integration test was moved to spec/datadog/tracing/contrib/http/request_spec.rb and refactored.
| it { is_expected.to be true } | ||
| it { is_expected.to be false } |
There was a problem hiding this comment.
This diff is confusing because of the test removed: this PR did not change this assertion from true to false. See side-by-side for more clarity.
| 'Content-Type' => 'application/json', | ||
| 'DD-Telemetry-API-Version' => 'v1', | ||
| 'DD-Telemetry-Request-Type' => 'app-started', | ||
| 'DD-Internal-Untraced-Request' => '1', |
There was a problem hiding this comment.
Curious why you decided to remove the constants. Not opposed to the change, I just want to know your reasoning 😄
There was a problem hiding this comment.
When you use to the System Under Test (SUT) as test assertions, you can end up with test assertions that are couple to your SUT.
This means you could have wrong assertions, because they are simply reflecting something incorrect in your SUT, and regurgitating it back to you in a test case.
This doesn't mean we should replace every reference from the SUT from our test assertions, but when feasible, it's good to avoid it.
In a few cases, it can create unmaintainable tests, and it might be fine to use the SUT, but I tend to avoid it if possible.
Telemetry has its own custom transport, and it was missing a flag that lets the
Net::HTTPtracing integration know to skip instrumenting the telemetry request.This PR adds that flag, as an HTTP header, similar to how other
ddtracecomponents operate.There is also a bit of refactoring around this flag, to easy clarity during future use and simplify the circuit breaker implementation.