W3C Phase 3 Extract p Regardless of Order
#2385
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mabdinur
reviewed
Apr 24, 2024
bouwkast
reviewed
May 1, 2024
There was a problem hiding this comment.
I'm a bit confused with these tests as looking at the RFC it still has two checks for merging headers based on Trace IDs:
- If 128-bit trace IDs are provided by both headers compare those, merge if equal
- If it is just 64-bit trace IDs in the headers, compare those, merge if equal
mabdinur
force-pushed
the
maximo/w3c-phase-3
branch
from
67312a2
to
e8fefee
Compare
mabdinur
approved these changes
May 14, 2024
mabdinur
reviewed
May 14, 2024
mabdinur
reviewed
May 14, 2024
mabdinur
reviewed
May 14, 2024
This was referenced May 14, 2024
mabdinur
force-pushed
the
maximo/w3c-phase-3
branch
from
d2fb81a
to
324a172
Compare
mabdinur
force-pushed
the
maximo/w3c-phase-3
branch
from
324a172
to
a7c369d
Compare
robertomonteromiguel
approved these changes
May 15, 2024
This reverts commit a7c369d.
2 tasks
brettlangdon
added a commit
to DataDog/dd-trace-py
that referenced
this pull request
May 22, 2024
…9254) ## Description Ensures when trace headers with the same trace_id but different span_ids are received by an application the tracecontext headers are treated as the source of truth. - The traceparent header will be used to set the parent_id - If the conflicting trace information was extracted from datadog headers, the span_id from the datadog headers will be stored in the _dd.parent_id parent_id tag. - All other propagation styles are third class citizens. If they conflict with tracecontext headers their span_ids will be overwritten. This behavior will be validated by the following system-tests: DataDog/system-tests#2385 ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] 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: Brett Langdon <brett.langdon@datadoghq.com>
link04
added a commit
to DataDog/dd-trace-dotnet
that referenced
this pull request
Jul 1, 2024
…ith The W3C Headers (#5721) ## Summary of changes Making sure that the Parent Id tag is set as expected based on the confirmation that the TraceIds match but the SpanIds don't when using the W3C headers, also setting the spanId based on that disconnected to keep try fixing the parent relation ship. RFC: https://docs.google.com/document/d/1Zc7uAQTJ2G5vHEbmx8k4vMnYdQ-BnqZxewXk0ohgd3g/edit ## Reason for change To improve our interoperability with the `tracecontext` headers. ## Implementation details Added the following to the SpanContextProgator.cs: ``` if (localSpanContext.RawSpanId != spanContext.RawSpanId) { if (!string.IsNullOrEmpty(spanContext.LastParentId) && spanContext.LastParentId != W3CTraceContextPropagator.ZeroLastParent) { localSpanContext.LastParentId = spanContext.LastParentId; } else { localSpanContext.LastParentId = $"{localSpanContext.SpanId:x16}"; } localSpanContext.SpanId = spanContext.SpanId; } ``` ## Test coverage Added new test case based out of the system-tests cases for this feature: DataDog/system-tests#2385
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
To make sure the traces are following the expected behavior when it comes to extracting the
ptag to the spans under the current conditions based on the RFC.https://docs.google.com/document/d/1Zc7uAQTJ2G5vHEbmx8k4vMnYdQ-BnqZxewXk0ohgd3g/edit#heading=h.53cixy7xk9rp
Changes
Added two new tests to check the created tags when the trace & span Id match and the outcome in scenarios where it doesn't or the tag shouldn't be added.
Workflow
codeownersfile quickly.🚀 Once your PR is reviewed, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
run-parametric-scenario,run-profiling-scenario...) are presents[<language>], double-check that only<language>is impacted by the changebuild-XXX-imagelabel is present