Use a webserver-wide session id instead of per fork#3828
Use a webserver-wide session id instead of per fork#3828
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b10ad2605
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (dd_main_pid != getpid()) { // equal to session id if not a fork | ||
| ddtrace_generate_runtime_id(); |
There was a problem hiding this comment.
Generate runtime ID in non-forked processes
This guard skips ddtrace_generate_runtime_id() when the process that ran MINIT also runs RINIT (e.g., CLI, single-process SAPIs, and Windows). In that case ddtrace_runtime_id stays at its zero-initialized value, so emitted runtime-id values become 00000000-0000-0000-0000-000000000000 instead of a unique UUID, which breaks per-runtime correlation in traces/telemetry.
Useful? React with 👍 / 👎.
| ddtrace_sidecar_push_tag(tags, DDOG_CHARSLICE_C("language"), DDOG_CHARSLICE_C("php")); | ||
| ddtrace_sidecar_push_tag(tags, DDOG_CHARSLICE_C("runtime"), DDOG_CHARSLICE_C("php")); | ||
| ddtrace_sidecar_push_tag(tags, DDOG_CHARSLICE_C("runtime-id"), (ddog_CharSlice) {.ptr = (char *) dd_sidecar_formatted_session_id, .len = sizeof(dd_sidecar_formatted_session_id)}); | ||
| ddtrace_sidecar_push_tag(tags, DDOG_CHARSLICE_C("runtime-id"), (ddog_CharSlice) {.ptr = (char *) ddtrace_formatted_session_id, .len = sizeof(ddtrace_formatted_session_id)}); |
There was a problem hiding this comment.
Keep crash metadata runtime-id tied to runtime UUID
The runtime-id crash tag is now populated from ddtrace_formatted_session_id (webserver-wide) rather than the per-process runtime UUID. After forks, multiple workers will report the same runtime-id, reducing crash attribution granularity and making crash/runtime correlation inconsistent with normal span metadata that still uses the runtime UUID.
Useful? React with 👍 / 👎.
4b10ad2 to
4577655
Compare
4577655 to
b10a16b
Compare
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: b10a16b | Docs | Datadog PR Page | Give us feedback! |
Benchmarks [ tracer ]Benchmark execution time: 2026-04-24 18:32:06 Comparing candidate commit b10a16b in PR branch Found 1 performance improvements and 4 performance regressions! Performance is the same for 189 metrics, 0 unstable metrics. scenario:SamplingRuleMatchingBench/benchRegexMatching1
scenario:SamplingRuleMatchingBench/benchRegexMatching2
scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching3
scenario:SamplingRuleMatchingBench/benchRegexMatching4
|
This is what session_id supposedly is actually meant to mean.