feat: publish Linux OTel contexts - #4077
Conversation
|
Benchmarks [ tracer ]Benchmark execution time: 2026-08-14 21:05:26 Comparing candidate commit cbd2d75 in PR branch Found 9 performance improvements and 4 performance regressions! Performance is the same for 181 metrics, 0 unstable metrics.
|
Benchmarks [ appsec ]Benchmark execution time: 2026-08-14 20:31:23 Comparing candidate commit cbd2d75 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics.
|
Publish standard OTel process and thread contexts from the tracer and make the profiler consume them for runtime identity and effective service metadata. Handle span, stack, Fiber, configuration, and fork lifecycle changes while retaining the legacy non-Linux path.
This reverts commit 6b3710c.
c7032dc to
29841a6
Compare
Benchmarks [ profiler ]Benchmark execution time: 2026-08-14 20:03:57 Comparing candidate commit cbd2d75 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 27 metrics, 7 unstable metrics.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d01de4429
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Service, environment, version, and configured hostname can vary by request. Omit them from the process-wide context while retaining their thread-context key mapping and active values.
morrisonlevi
changed the title
# Conflicts: # libdatadog
ivoanjo
left a comment
ivoanjo
left a comment
There was a problem hiding this comment.
I'm a big n00b on the codebase, yet I gave a pass on the OTel thread context bits, hopefully this helps!
| static void ddtrace_otel_record_end_update(datadog_otel_thr_ctx_rec *record) { | ||
| atomic_signal_fence(memory_order_release); | ||
| atomic_store_explicit(&record->valid, 1, memory_order_relaxed); | ||
| } |
There was a problem hiding this comment.
Same note as for ddtrace_otel_detach -- the signal fence should come last for similar reasons
There was a problem hiding this comment.
I intentionally kept the release signal fence before the pointer store. This matches OTEP 4947 and libdatadog’s ThreadContext::attach(), which performs compiler_fence(Ordering::Release) followed by a relaxed TLS swap.
| _Atomic(uint64_t) span_id; | ||
| _Atomic(uint8_t) valid; | ||
| _Atomic(uint8_t) trace_flags; |
There was a problem hiding this comment.
Minor: Maybe this is useful for the PHP profiler, so it's probably ok to keep; just wanted to point out that span_id and trace_flags don't actually need to be atomic, (e.g. similar to how trace_id and the others also don't need).
valid is the only one that needs to be treated specially, since in practice it's kind of a crappy lock -- 0 means lock is taken by writer, 1 means it's free for reading ;)
There was a problem hiding this comment.
These remain atomic because sometimes all we do is flip a single field. Since they are small enough, they will not tear in practice. Cheaper/faster when switching fibers which happens often for code that does this, or for trace propagation, etc.
# What does this PR do? If these items are not set, they should be missing, rather than sent with no value. # Motivation In PHP, these will not be set at all because they live in the OTel thread context instead of the process context. # Additional Notes Needed for DataDog/dd-trace-php#4077. # How to test the change? Tests the same, there just won't be "empty" items and they'll be missing instead. Co-authored-by: bob.weinand <bob.weinand@datadoghq.com>
| #ifdef __linux__ | ||
| if (stack == DDTRACE_G(active_stack) && stack->active && SPANDATA(stack->active)->stack == stack) { | ||
| ddtrace_update_otel_thread_context_span_id(SPANDATA(stack->active)); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
I think the stack == DDTRACE_G(active_stack) is wrong, because all it does is updating the spans id. If there's a later swap to this closed spans stack, it'll be stale.
And let's mirror it like dd_drop_span, i.e.
if (!stack->active || SPANDATA(stack->active)->stack != stack) {
dd_close_entry_span_of_stack(stack);
#ifdef __linux__
} else {
ddtrace_update_otel_thread_context_span_id(SPANDATA(stack->active));
#endif
}
There was a problem hiding this comment.
I believe I've fixed this, check the current code and refer to our conversation in Slack.
bwoebi
left a comment
bwoebi
left a comment
There was a problem hiding this comment.
Logic looks right to me now.
4 participants
PROF-15487
Description
Publish standard OTel process and thread contexts from the tracer and make the profiler consume them for runtime identity and effective service metadata. Handle span, stack, Fiber, configuration, and fork lifecycle changes while retaining the legacy non-Linux path.
WIP for appsec.
Reviewer checklist