-
Notifications
You must be signed in to change notification settings - Fork 420
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
feat(propagation): add DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT to handle x-org propagation #11631
Conversation
|
BenchmarksBenchmark execution time: 2025-01-14 23:25:34 Comparing candidate commit a9de823 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 394 metrics, 2 unstable metrics. |
5d2512b
to
ef045c8
Compare
…1723) This PR updates the `version_schema` in the `../pyproject.toml` file for the 2.18 release branch from `release-branch-semver` to `guess-next-dev`. This is to ensure that system tests work as intended with backports to this release branch. IMPORTANT: This PR needs to be merged before the first backport is created for 2.18.Otherwise, system tests will not work as expected. - [x] Reviewer check
…ncions [backport 2.18] (#11756) Backport a4f0e38 from #11752 to 2.18. This fix resolves an issue where AppSec was using a patched request and builtins functions, creating telemetry errors. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Alberto Vara <alberto.vara@datadoghq.com>
Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com>
e21c4a3
to
1c4b37e
Compare
e2911cd
to
1568543
Compare
0fb3045
to
e1b10df
Compare
Datadog ReportBranch report: ✅ 0 Failed, 130 Passed, 1468 Skipped, 4m 15.92s Total duration (29m 18.79s time saved) |
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
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.
Some questions for my own clarity. Otherwise LGTM from LP side
releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml
Outdated
Show resolved
Hide resolved
…5b.yaml Co-authored-by: erikayasuda <153395705+erikayasuda@users.noreply.github.com>
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.
Overall looks good to me. We need to confirm with @zacharycmontoya that the distributed tracing edge cases uncovered in this PR are documented somewhere. We should align one implementation across all tracers.
In terms of baggage, I think it looks good! |
6bc7ea8
to
0631002
Compare
0631002
to
a9de823
Compare
Configuration: DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT
Values:
Note: We do not need to implement this at the moment because the workaround we have in place DD_TRACE_PROPAGATION_STYLE_EXTRACT=none performs this exact function of completely ignoring incoming trace context headers and baggage headers. If we receive feedback, we can add this new option that we maintain.
Result
✅ Head services: Customers can configure these services with the restart configuration so that they are always the root span, regardless of incoming distributed tracing headers.
✅ Internal services: By default, internal services will remain unchanged and continue to report complete traces to Datadog. If a user has configured an internal service with the restart configuration, this will lead to partial traces. Thankfully with Span Links we can easily direct the customer to the upstream trace (if it was sampled).
❌ Sometimes-head services: This solution does not solve for services that are sometimes head services and sometimes not. The best solution for these services is to set the restart configuration so that a new trace can begin and sampling priority can be recalculated, and in the case that this was not a head service then the upstream service can be found with a span link.
❌ Reentrant systems: This solution does not improve this scenario no matter how the reentrant service is configured. Under the continue configuration, the reentrant service is part of the same trace as the original root span but it is orphaned. Under the restart configuration, a new trace will be started and the span will be decorated with a span link containing the original trace-id but the contained span-id would not point to a span tracked in their Datadog organization.
Note: This contains a fix, allowing only baggage to be propagated.
Checklist
Reviewer Checklist