-
Notifications
You must be signed in to change notification settings - Fork 159
Set DD_TRACE_OTEL_ENABLED=true by default for all integration tests
#8370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6329bfd
134b2f8
35db4c5
f1c3d36
67c86c3
f80ad4c
f96c00d
3e471f1
05774d8
42c7024
2e7cd1e
9fbe610
1a641f5
e8c744e
ed6038d
32b74a1
7390b47
ae698e9
a6524ae
c382254
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -212,8 +212,11 @@ public async Task TracingDisabled_DoesNotSubmitsTraces( | |
| headers.Should().NotContainKey(HttpHeaderNames.PropagatedTags); | ||
|
|
||
| // W3C trace context headers | ||
| headers.Should().NotContainKey(W3CTraceContextPropagator.TraceParentHeaderName); | ||
| headers.Should().NotContainKey(W3CTraceContextPropagator.TraceStateHeaderName); | ||
| // These are still injected when we have otel enabled, but they're not added by us | ||
| // TODO: Verify whether the injected values are actually _correct_ - they _should_ be, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a local test and this behavior only happens starting in .NET 7. Unfortunately, the span-id on the headers matches the span-id of the ignored activity. More details: The ActivitySource with name We then call In reality, this behavior will only affect cases where we want to disable tracing, which includes the export of traces or other telemetry data that the user would view as noise. We can do a follow-up PR to resolve this issue because it's not new, but we should fix this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, question, how should we fix this:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is the best approach. This isn't actually "integration disabled", rather "integration enabled but this http request has the do-not-trace header". This covers cases such as http requests to our own
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😬 I think that's conflating two separate issues, which is something we've flagged previously too 🙈 Because there's several different aspects here:
In practice, we really should be able to control span creation and propagation independently. And integration disablement is a whole other tangential thing 😬 So in short, I'm really not sure what the best fix is today, without exploring the full matrix of potential features further... |
||
| // because our IgnoreActivityHandler should fix them | ||
| // headers.Should().NotContainKey(W3CTraceContextPropagator.TraceParentHeaderName); | ||
| // headers.Should().NotContainKey(W3CTraceContextPropagator.TraceStateHeaderName); | ||
|
|
||
| // B3 trace context headers | ||
| headers.Should().NotContainKey(B3SingleHeaderContextPropagator.B3); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you remember why we chose to "listen and ignore" sources rather than "do not subscribe" to sources? I suspect we should be doing the latter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, the reason we don't do that is because we need to "fix" the IDs on these activities to match our own, so that if someone creates a child activity, or these IDs are embedded somewhere, everywhere uses the right values. Otherwise we can end up with weird duplication or parentage issues. Which basically means that we can never safely "not subscribe" (unfortunately)