Skip to content

refactor(sidecar)!: Avoid a dedicated socket for crashtracker - #2179

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
bob/sidecar-crashtracker-ipc
Jul 7, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
bob/sidecar-crashtracker-ipc

Conversation

@bwoebi

@bwoebi bwoebi commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This allows us doing an unified authentication of the socket on the sidecar side, before handing off to crashtracker. Usually the parent process would be the one connecting to the crashtracker, here it is not. This will allow custom handling, and one less endpoint to secure.

The strategy chosen is upgrading to the crashtracker once an IPC socket is received. Currently we recreate a new socket, but we might also decide to reuse the existing IPC connection in future.

The changes to the crashtracker crate itself are kept minimal, just adding a new config for a custom socket connector, and using the existing one as default.

There's also a bit of refactoring on the sidecar side, to make it easier to take ownership of the asyncfd.

@bwoebi
bwoebi requested review from a team as code owners June 29, 2026 17:46
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 55.84%
Overall Coverage: 74.34% (-0.03%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 165efb2 | Docs | Datadog PR Page | Give us feedback!

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Clippy Allow Annotation Report

Comparing clippy allow annotations between branches:

  • Base Branch: origin/main
  • PR Branch: origin/bob/sidecar-crashtracker-ipc

Summary by Rule

Rule Base Branch PR Branch Change
expect_used 2 2 No change (0%)
unwrap_used 12 12 No change (0%)
Total 14 14 No change (0%)

Annotation Counts by File

File Base Branch PR Branch Change
datadog-ipc/src/example_interface.rs 1 1 No change (0%)
datadog-sidecar/src/config.rs 2 2 No change (0%)
datadog-sidecar/src/service/blocking.rs 1 1 No change (0%)
datadog-sidecar/src/service/sidecar_server.rs 6 6 No change (0%)
datadog-sidecar/src/unix.rs 4 4 No change (0%)

Annotation Stats by Crate

Crate Base Branch PR Branch Change
clippy-annotation-reporter 5 5 No change (0%)
datadog-ffe-ffi 1 1 No change (0%)
datadog-ipc 22 22 No change (0%)
datadog-live-debugger 4 4 No change (0%)
datadog-live-debugger-ffi 10 10 No change (0%)
datadog-profiling-replayer 4 4 No change (0%)
datadog-sidecar 45 45 No change (0%)
libdd-common 13 13 No change (0%)
libdd-common-ffi 12 12 No change (0%)
libdd-data-pipeline 6 6 No change (0%)
libdd-ddsketch 2 2 No change (0%)
libdd-dogstatsd-client 1 1 No change (0%)
libdd-profiling 13 13 No change (0%)
libdd-remote-config 3 3 No change (0%)
libdd-telemetry 20 20 No change (0%)
libdd-tinybytes 4 4 No change (0%)
libdd-trace-normalization 2 2 No change (0%)
libdd-trace-obfuscation 3 3 No change (0%)
libdd-trace-stats 1 1 No change (0%)
libdd-trace-utils 11 11 No change (0%)
Total 182 182 No change (0%)

About This Report

This 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.

@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch from 78bf47c to f4d9ba8 Compare June 29, 2026 17:58

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78bf47c88d

ℹ️ 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".

Comment thread datadog-sidecar/src/crashtracker.rs
Comment thread datadog-sidecar/src/crashtracker.rs
Comment thread datadog-ipc/src/ipc_server.rs
Comment thread datadog-sidecar/src/crashtracker.rs
Comment thread datadog-sidecar-ffi/Cargo.toml Outdated
Comment thread datadog-sidecar/src/service/sidecar_server.rs
@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch 2 times, most recently from eaf749d to 15ae3e8 Compare June 30, 2026 13:26
@bwoebi bwoebi changed the title Avoid a dedicated socket for crashtracker refactor(sidecar)!: Avoid a dedicated socket for crashtracker Jun 30, 2026
@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch 4 times, most recently from d3bc905 to 99cefc7 Compare July 1, 2026 12:58

impl ConnectionSidecarHandler {
fn new(server: SidecarServer) -> Self {
fn new(server: SidecarServer, connection: OwnedServerConn) -> Option<Self> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always returns Some(..), right? Is the Option misleading then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, that code went through two iterations :-D

};

// One recv() returns one datagram; a max-sized buffer avoids truncation.
let read_result = guard.try_io(|inner| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every single poll read allocates and deallocs 4MiB. Is this okay?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really a big problem for crashtracker, but yeah, I should move it out of the loop to save some syscalls.

use tokio::io::unix::AsyncFd;
use tokio::io::{AsyncReadExt, BufReader};

fn dgram_pair() -> (OwnedFd, OwnedFd) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test use SOCK_DGRAM but production is SOCK_SEQPACKET. Can we make tests consistent with our prod code or is this intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intentional, yes, or I would have to rewrite the test for macos. In any case, both have the same semantics needed with respect to crashtracker.

@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch 3 times, most recently from 8e41485 to 449b928 Compare July 4, 2026 01:19
@bwoebi

bwoebi commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jul 4, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-04 01:32:20 UTC ℹ️ Start processing command /merge


2026-07-04 01:32:27 UTC ℹ️ MergeQueue: waiting for PR to be ready

This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals. View in MergeQueue UI.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2026-07-04 06:26:11 UTC ⚠️ MergeQueue: This merge request was unqueued

devflow unqueued this merge request: It did not become mergeable within the expected time

@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch from 2c81f3c to 791caa0 Compare July 6, 2026 17:25
bwoebi added 4 commits July 6, 2026 19:54
This allows us doing an unified authentication of the socket on the sidecar side, before handing off to crashtracker.
Usually the parent process would be the one connecting to the crashtracker, here it is not. This will allow custom handling, and one less endpoint to secure.

The strategy chosen is upgrading to the crashtracker once an IPC socket is received. Currently we recreate a new socket, but we might also decide to reuse the existing IPC connection in future.

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

# Conflicts:
#	libdd-crashtracker/src/collector/receiver_manager.rs
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@bwoebi
bwoebi force-pushed the bob/sidecar-crashtracker-ipc branch from 791caa0 to 165efb2 Compare July 6, 2026 17:57
@dd-octo-sts

dd-octo-sts Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Artifact Size Benchmark Report

aarch64-alpine-linux-musl
Artifact Baseline Commit Change
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.so 7.88 MB 7.88 MB 0% (0 B) 👌
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.a 85.91 MB 85.91 MB +0% (+1.86 KB) 👌
aarch64-unknown-linux-gnu
Artifact Baseline Commit Change
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.61 MB 10.61 MB +0% (+144 B) 👌
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.a 97.11 MB 97.11 MB +0% (+1.65 KB) 👌
libdatadog-x64-windows
Artifact Baseline Commit Change
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.dll 25.46 MB 25.46 MB 0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.lib 88.44 KB 88.44 KB 0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.pdb 184.60 MB 184.60 MB 0% (0 B) 👌
/libdatadog-x64-windows/debug/static/datadog_profiling_ffi.lib 946.40 MB 946.40 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.dll 8.32 MB 8.32 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.lib 88.44 KB 88.44 KB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.pdb 24.62 MB 24.62 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/static/datadog_profiling_ffi.lib 49.04 MB 49.04 MB 0% (0 B) 👌
libdatadog-x86-windows
Artifact Baseline Commit Change
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.dll 22.06 MB 22.06 MB 0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.lib 89.82 KB 89.82 KB 0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.pdb 188.64 MB 188.62 MB --.01% (-24.00 KB) 💪
/libdatadog-x86-windows/debug/static/datadog_profiling_ffi.lib 935.37 MB 935.37 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.dll 6.43 MB 6.43 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.lib 89.82 KB 89.82 KB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.pdb 26.43 MB 26.43 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/static/datadog_profiling_ffi.lib 46.65 MB 46.65 MB 0% (0 B) 👌
x86_64-alpine-linux-musl
Artifact Baseline Commit Change
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.a 76.59 MB 76.59 MB +0% (+408 B) 👌
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.so 8.78 MB 8.78 MB 0% (0 B) 👌
x86_64-unknown-linux-gnu
Artifact Baseline Commit Change
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.a 92.11 MB 92.11 MB +0% (+328 B) 👌
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.69 MB 10.69 MB +0% (+120 B) 👌

let stream = UnixStream::connect(unix_socket_path);
match stream {
Ok(s) => s.into_raw_fd(),
Err(_) => -1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we document that this returns -1 on error?

use_alt_stack: bool,
}

impl PartialEq for CrashtrackerConfiguration {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just derive here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

miri will fail with fn() pointers.

timeout: Duration,
unix_socket_path: Option<String>,
#[serde(skip, default = "default_unix_socket_connector_value")]
unix_socket_connector: fn(&str) -> std::os::fd::RawFd,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sent over the socket to the receiver as a json string. What does it mean to send a function over the wire?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you've missed the #[serde(skip)].

@bwoebi

bwoebi commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Jul 7, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-07-07 12:06:44 UTC ℹ️ Start processing command /merge


2026-07-07 12:06:48 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1h (p90).


2026-07-07 12:47:37 UTC ℹ️ MergeQueue: This merge request was merged

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit e91210e into main Jul 7, 2026
99 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the bob/sidecar-crashtracker-ipc branch July 7, 2026 12:47
Eldolfin pushed a commit that referenced this pull request Jul 8, 2026
This allows us doing an unified authentication of the socket on the sidecar side, before handing off to crashtracker. Usually the parent process would be the one connecting to the crashtracker, here it is not. This will allow custom handling, and one less endpoint to secure.

The strategy chosen is upgrading to the crashtracker once an IPC socket is received. Currently we recreate a new socket, but we might also decide to reuse the existing IPC connection in future.


The changes to the crashtracker crate itself are kept minimal, just adding a new config for a custom socket connector, and using the existing one as default.

There's also a bit of refactoring on the sidecar side, to make it easier to take ownership of the asyncfd.

Co-authored-by: bob.weinand <bob.weinand@datadoghq.com>
taegyunkim pushed a commit that referenced this pull request Jul 8, 2026
This allows us doing an unified authentication of the socket on the sidecar side, before handing off to crashtracker. Usually the parent process would be the one connecting to the crashtracker, here it is not. This will allow custom handling, and one less endpoint to secure.

The strategy chosen is upgrading to the crashtracker once an IPC socket is received. Currently we recreate a new socket, but we might also decide to reuse the existing IPC connection in future.

The changes to the crashtracker crate itself are kept minimal, just adding a new config for a custom socket connector, and using the existing one as default.

There's also a bit of refactoring on the sidecar side, to make it easier to take ownership of the asyncfd.

Co-authored-by: bob.weinand <bob.weinand@datadoghq.com>
Signed-off-by: Taegyun Kim <taegyun.kim@datadoghq.com>
hoolioh added a commit that referenced this pull request Aug 19, 2026
# Release proposal for libdd-crashtracker, libdd-gotter and their
dependencies

This PR contains version bumps based on public API changes and commits
since last release.

## libdd-gotter
**Next version:** `1.0.0`
**Semver bump:** `major`
**Tag:** `libdd-gotter-v1.0.0`

**Warning:** this is an initial release. Please verify that the version
and commits included are correct.


## libdd-crashtracker
**Next version:** `2.0.0`
**Semver bump:** `major`
**Tag:** `libdd-crashtracker-v2.0.0`

### ⚠️ major bump forced due to:

- `libdd-common`: ^1.0.0 → ^5.2.0
- `libdd-telemetry`: ^1.0.0 → ^7.0.0

### Commits

- feat(crashtracking)!: retrieve c assert message for linux when
`__assert_fail` is dynamically loaded (#2268)
- test: skip/shorten slow miri jobs (#2331)
- feat(crashtracking): send debug log when no data is received at all
(#2321)
- fix(crashtracking): check fields and exclude uuid for `has_data`
(#2322)
- refactor: migrate to workspace dependencies, phase 4 (#2296)
- refactor: migrate to workspace dependencies, phase 3 (#2283)
- chore: moving to workspace-level dependencies, phase 2 (#2270)
- refactor: consolidate core dependencies at workspace level (phase 1)
(#2253)
- test: stabilize flaky tests (#2256)
- feat(telemetry)!: make telemetry worker wasm-compatible for the
TraceExporter (#2172)
- chore(crashtracking): harden multi thread ptrace collection (#2216)
- fix(sidecar): don't double-encode file:// telemetry endpoints (#2230)
- fix: new clippy lints (#2219)
- refactor(sidecar)!: Avoid a dedicated socket for crashtracker (#2179)
- refactor(clippy): prefer core and alloc imports (#2196)
- refactor(libdd-telemetry)!: avoid leaking libdd-common types in the
public API (#2152)
- feat(crashtracking): collect all native stacks for unhandled exception
and also crashing thread (#2155)
- fix(crashtracking): sanitize type and message for unhandled exceptions
(#2148)
- fix(crashtracking): multi thread collection centos flakes harden
(#2113)
- chore(crashtracking)!: remove frame count experimental field (#2114)
- fix(crashtracking): authenticate peer granted socket ptrace access
(#2098)
- feat(crashtracking)!: add experimental frame count field (#2099)
- fix(crashtracker): increase test_waitall_nohang timeout to 500ms
(#2097)
- fix(crashtracker): support socket based receiver for all thread
collection (#2080)
- fix(crashtracker): set failed thread stack collection as incomplete
empty stack (#2079)
- fix(crashtracking)!: flatten all threads object into a list of
`ThreadData` (#2054)
- refactor(crashtracker): update imports to linux only (#2036)
- fix(crashtracker): move preload logger marking after recursive guard
(#2023)
- fix(libdd-telemetry): restore previous Cargo.toml version (#1993)
- feat!: integrate obfuscation to the stats exporter [APMSP-2764]
(#1819)
- chore(crashtracking)!: create errorsintake crash ping directly from
telemetry (#1963)
- feat(telemetry)!: include dependencies and integrations in
app-extended-heartbeat (#1962)
- feat(crashtracking)!: collect all threads (#1878)
- chore(crashtracking): bump libdatadog-libunwind to v1.0.2 (#1942)
- chore(crashtracking): default errors intake crash report upload to be
on (#1902)
- chore(libdd-crashtracker): remove path reference for
libdd-libunwind-sys (#1877)
- fix(crashtracking): handle new lines in client submitted exception
message (#1836)
- fix(crashtracker): Fix bin_tests in gitlab (#1832)
- feat(crashtracking)!: improve parity between errors intake payload and
telemetry intake payload (#1823)
- feat(crashtracking)!: emit ucontext registers as structured data
(#1787)
- fix(ci): use single threaded for all tests that mutate signal state
and use non-fatal signal (#1812)
- fix(crashtracking): fix SIGCHLD signal guarding while in CT signal
handler (#1807)
- fix(ci): use single threaded to avoid race conditions for sa guard
tests (#1800)
- fix(crashtracking): guard sigchld and sigpipe during crashtracker
signal handler execution (#1771)
- chore(crashtracking): preserve errno for crashtracker (#1767)
- chore(crashtracker): use weaker mem ordering for OP_COUNTERS (#1744)
- chore(crashtracking): rename target triple to runtime platform (#1747)
- chore(ci): give libdd-libunwind-sys its own version (#1743)
- chore(crashtracking): add tag for target triple (#1741)
- refactor(libdd-crashtracker)!: avoid leaking Endpoint through the
public API (#1705)
- chore(cargo): bump to 29.0.0 (#1702)
- fix(crashtracking): use libunwind to unwind frames (#1663)
- chore(deps): bump blazesym to 0.2.3 and blazesym-c to 0.1.7 (#1654)
- chore(ci): fix crashtracker receiver binary rpath setting (#1652)
- chore(crashtracking): emit a best effort stacktrace for Mac (#1645)
- chore(crashtracker): use default-features=false for aws-lc-sys (#1625)
- feat(crashtracking): report unhandled exceptions (#1596)
- refactor(ddcommon)!: remove direct dependency on hyper client
everywhere in common (#1604)
- feat(crashtracking): include `Kind` in crash ping and clarify
requirements (#1595)
- fix(crashtracking): add process_tags to application field (#1576)
- ci: update nightly in CI to 2026-02-08 (#1539)
- fix(telemetry)!: fix logs payload format [APMSP-2590] (#1498)
- chore(crashtracking): bump os_info crate to 3.14 (#1507)
- Don't bail (#1494)
- feat(crashtracking)!: emit crashing thread name in crash report for
linux crashes (#1485)
- test(crashtracking): add minimal LD preload test for crashtracker
collector (#1428)
- chore: release libddcommon-v1.1.0 (#1456)
- chore: prepare libdd-telemetry-v2.0.0 (#1457)
- [crashtracker] Retrieve panic message when crashing (#1361)
- fix(sidecar): AWS lambda also can return EACCESS for shm_open (#1446)
- chore(crashtracking): add `is_crash_debug` tag to crashtracker
receiver debug logs (#1445)
- [crashtracker] Log errors in crashtracker receiver (#1395)
- chore: add changelog for every published crate (#1396)
- Fix CI (#1389)
- [crashtracker] Set OS info in the crash info builder when receiving
report (#1388)
- crashtracker: support cxx bindings for crashinfo (#1379)


[APMSP-2764]:
https://datadoghq.atlassian.net/browse/APMSP-2764?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: hoolioh <107922352+hoolioh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants