fix: handle fork in otel process ctx#1650
fix: handle fork in otel process ctx#1650gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit intomainfrom
Conversation
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
BenchmarksComparisonBenchmark execution time: 2026-03-05 12:13:42 Comparing candidate commit 9e85c64 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 56 metrics, 2 unstable metrics. scenario:profile_add_sample2_frames_x1000
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
Group 14
Group 15
Group 16
Group 17
Group 18
Group 19
BaselineOmitted due to size. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1650 +/- ##
==========================================
+ Coverage 71.22% 71.25% +0.02%
==========================================
Files 425 425
Lines 62695 62704 +9
==========================================
+ Hits 44657 44677 +20
+ Misses 18038 18027 -11
🚀 New features to boost your workflow:
|
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
dffff99 to
16cc9c4
Compare
ivoanjo
left a comment
There was a problem hiding this comment.
However, two follow-up PRs are coming with an FFI and libdatadog-side protobuf encoding of the payload, which will make it possible to test the fork behavior e.g. from Ruby, once they land.
👍 Agreed on this one -- no need to boil the ocean as we can test this trivially on the Ruby side
de3ea26 to
2f163e6
Compare
genesor
left a comment
There was a problem hiding this comment.
My main tracer (go) is not a client a libdatadog, I wouldn't really know what to look for here.
The changes make sense, I was able to understand everything thanks to the extensive comments.
I will share the PR in my team channel if a user of libdatadog wants to get another pair of eyes on it.
2f163e6 to
9e85c64
Compare
Depends #1650 # What does this PR do? Follow-up of #1585 #1640 #1650 Adds the protobuf definition for the OTel process context and associated messages, and make the interface of the publisher higher-level by taking the new structured `ProcessContext` value instead of a raw bytes payload. # Motivation Since libdatadog is already taking care of some protobuf encoding, and it's supposedly faster and simpler to do here rather than on the side of each language runtime, it makes sense to offer an interface with a struct that is encoded by libdatadog. The process context will be a `ProcessContext`-based opaque pointer on the FFI side, with proper setters/getters. # Additional Notes I was not sure where to put the protobuf definition, as there are a bunch of protobuf-dedicated crates in libdatadog. I feel like the OTel process context is about tracing metadata, but I'm happy to move it elsewhere if it makes more sense. # How to test the change? Once the FFI lands, we'll be able to publish the context from a language runtime and check the whole process end-to-end. For now, I feel like an additional test would mostly test `prost`, which isn't very valuable. Co-authored-by: yann.hamdaoui <[email protected]>
# Release proposal for libdd-library-config and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-trace-protobuf **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-protobuf-v2.0.0` ### Commits - feat(trace-protobuf)!: Add two fields to ClientGroupedStats [SVLS-8627] (#1630) - feat: otel process ctxt protobuf encoding (#1651) ## libdd-library-config **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-library-config-v1.1.0` ### Commits - chore: update otel process ctx protocol (#1713) - feat: publish tracer metadata as OTel process ctx (#1658) - feat: otel process ctxt protobuf encoding (#1651) - fix: handle fork in otel process ctx (#1650) - chore: implement otel process ctx update (#1640) - feat: process context publication (#1585) - ci: update nightly in CI to 2026-02-08 (#1539) - fix(stable_config): [APMAPI-1690] add >100mb check for stable config files (#1432) - chore: add changelog for every published crate (#1396) [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1690]: https://datadoghq.atlassian.net/browse/APMAPI-1690?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
# Release proposal for libdd-library-config and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-trace-protobuf **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-protobuf-v2.0.0` ### Commits - feat(trace-protobuf)!: Add two fields to ClientGroupedStats [SVLS-8627] (#1630) - feat: otel process ctxt protobuf encoding (#1651) ## libdd-library-config **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-library-config-v1.1.0` ### Commits - chore: update otel process ctx protocol (#1713) - feat: publish tracer metadata as OTel process ctx (#1658) - feat: otel process ctxt protobuf encoding (#1651) - fix: handle fork in otel process ctx (#1650) - chore: implement otel process ctx update (#1640) - feat: process context publication (#1585) - ci: update nightly in CI to 2026-02-08 (#1539) - fix(stable_config): [APMAPI-1690] add >100mb check for stable config files (#1432) - chore: add changelog for every published crate (#1396) [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1690]: https://datadoghq.atlassian.net/browse/APMAPI-1690?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
# Release proposal for libdd-library-config and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-trace-protobuf **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-protobuf-v2.0.0` ### Commits - feat(trace-protobuf)!: Add two fields to ClientGroupedStats [SVLS-8627] (#1630) - feat: otel process ctxt protobuf encoding (#1651) ## libdd-library-config **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-library-config-v1.1.0` ### Commits - chore: update otel process ctx protocol (#1713) - feat: publish tracer metadata as OTel process ctx (#1658) - feat: otel process ctxt protobuf encoding (#1651) - fix: handle fork in otel process ctx (#1650) - chore: implement otel process ctx update (#1640) - feat: process context publication (#1585) - ci: update nightly in CI to 2026-02-08 (#1539) - fix(stable_config): [APMAPI-1690] add >100mb check for stable config files (#1432) - chore: add changelog for every published crate (#1396) [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1690]: https://datadoghq.atlassian.net/browse/APMAPI-1690?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- # What does this PR do? A brief description of the change being made with this pull request. # Motivation What inspired you to submit this pull request? # Additional Notes Anything else we should know when reviewing? # How to test the change? Describe here in detail how the change can be validated. [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1690]: https://datadoghq.atlassian.net/browse/APMAPI-1690?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
# Release proposal for libdd-library-config and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-trace-protobuf **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-protobuf-v2.0.0` ### Commits - feat(trace-protobuf)!: Add two fields to ClientGroupedStats [SVLS-8627] (#1630) - feat: otel process ctxt protobuf encoding (#1651) ## libdd-library-config **Next version:** `1.1.0` **Semver bump:** `minor` **Tag:** `libdd-library-config-v1.1.0` ### Commits - chore: update otel process ctx protocol (#1713) - feat: publish tracer metadata as OTel process ctx (#1658) - feat: otel process ctxt protobuf encoding (#1651) - fix: handle fork in otel process ctx (#1650) - chore: implement otel process ctx update (#1640) - feat: process context publication (#1585) - ci: update nightly in CI to 2026-02-08 (#1539) - fix(stable_config): [APMAPI-1690] add >100mb check for stable config files (#1432) - chore: add changelog for every published crate (#1396) [SVLS-8627]: https://datadoghq.atlassian.net/browse/SVLS-8627?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1690]: https://datadoghq.atlassian.net/browse/APMAPI-1690?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: dd-octo-sts[bot] <200755185+dd-octo-sts[bot]@users.noreply.github.com>
Depends #1640
What does this PR do?
This PR is a follow up of #1585 and #1640 which implement OTel process context publication. It adds a proper handling of the case of publishing a new context after a
fork.Motivation
Some language runtimes (e.g. Python or Ruby) resorts to
fork. In this case, the OTel process context of the child must be re-published (the process context of the parent is explicitly NOT inherited throughMADVISE_DONTFORK). However, since they share the same copy of the static handler, prior to this PR, the publication would try to access the non-inherited mapping, potentially causing a crash.This PR properly handles this case by storing the PID of the publisher, so that upon update, we can detect if there's been a fork since (and we are a child), in which case we can re-create a mapping from scratch.
How to test the change?
There is currently no test, because having tests that
forkis not trivial to set up in the current Rust test framework (it is admittedly possible, but is a larger question that should be treated separately IMHO). However, two follow-up PRs are coming with an FFI and libdatadog-side protobuf encoding of the payload, which will make it possible to test the fork behavior e.g. from Ruby, once they land.