Skip to content

refactor(otel-thread-ctx): add const offset assertions for ThreadContextRecord#2018

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit into
mainfrom
yannham/otel-thread-ctx-const-assertions
May 20, 2026
Merged

refactor(otel-thread-ctx): add const offset assertions for ThreadContextRecord#2018
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit into
mainfrom
yannham/otel-thread-ctx-const-assertions

Conversation

@yannham
Copy link
Copy Markdown
Contributor

@yannham yannham commented May 20, 2026

What does this PR do?

Follow-up to #2016.

Adds compile-time mem::offset_of! assertions for every field of ThreadContextRecord, catching accidental ABI breakage (field reordering, padding changes) at compile time rather than at runtime in a profiler.

The existing size-only assertion from inside new() is moved to a top-level const _: () = { ... } block next to the struct definition so it fires unconditionally regardless of code path.

Motivation

ThreadContextRecord has a fixed ABI read by out-of-process eBPF readers. The previous single size_of == 640 check would still pass if fields were reordered without changing the total size. Per-field offset assertions make this impossible.

Additional Notes

N/A

How to test the change?

N/A

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

Clippy Allow Annotation Report

Comparing clippy allow annotations between branches:

  • Base Branch: origin/main
  • PR Branch: origin/yannham/otel-thread-ctx-const-assertions

Summary by Rule

Rule Base Branch PR Branch Change

Annotation Counts by File

File Base Branch PR Branch Change

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 21 21 No change (0%)
datadog-live-debugger 6 6 No change (0%)
datadog-live-debugger-ffi 10 10 No change (0%)
datadog-profiling-replayer 4 4 No change (0%)
datadog-remote-config 3 3 No change (0%)
datadog-sidecar 57 57 No change (0%)
libdd-common 13 13 No change (0%)
libdd-common-ffi 12 12 No change (0%)
libdd-data-pipeline 5 5 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-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 15 15 No change (0%)
Total 198 198 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.

@yannham yannham marked this pull request as ready for review May 20, 2026 13:41
@yannham yannham requested a review from a team as a code owner May 20, 2026 13:41
@yannham yannham requested review from ivoanjo and scottgerring May 20, 2026 13:41
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.88%. Comparing base (d96d160) to head (cc8f8fd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2018      +/-   ##
==========================================
- Coverage   72.90%   72.88%   -0.02%     
==========================================
  Files         457      457              
  Lines       75769    75769              
==========================================
- Hits        55236    55226      -10     
- Misses      20533    20543      +10     
Components Coverage Δ
libdd-crashtracker 65.21% <ø> (-0.02%) ⬇️
libdd-crashtracker-ffi 36.82% <ø> (ø)
libdd-alloc 98.77% <ø> (ø)
libdd-data-pipeline 86.69% <ø> (ø)
libdd-data-pipeline-ffi 78.63% <ø> (ø)
libdd-common 79.81% <ø> (ø)
libdd-common-ffi 74.41% <ø> (ø)
libdd-telemetry 73.34% <ø> (ø)
libdd-telemetry-ffi 31.36% <ø> (ø)
libdd-dogstatsd-client 82.64% <ø> (ø)
datadog-ipc 76.22% <ø> (ø)
libdd-profiling 81.69% <ø> (ø)
libdd-profiling-ffi 64.79% <ø> (ø)
libdd-sampling 97.46% <ø> (ø)
datadog-sidecar 29.35% <ø> (ø)
datdog-sidecar-ffi 10.93% <ø> (ø)
spawn-worker 48.86% <ø> (ø)
libdd-tinybytes 93.16% <ø> (ø)
libdd-trace-normalization 81.71% <ø> (ø)
libdd-trace-obfuscation 87.30% <ø> (ø)
libdd-trace-protobuf 68.25% <ø> (ø)
libdd-trace-utils 88.86% <ø> (ø)
libdd-tracer-flare 86.88% <ø> (ø)
libdd-log 74.83% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@datadog-datadog-prod-us1
Copy link
Copy Markdown
Contributor

datadog-datadog-prod-us1 Bot commented May 20, 2026

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 72.89% (-0.01%)

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

@dd-octo-sts
Copy link
Copy Markdown
Contributor

dd-octo-sts Bot commented May 20, 2026

Artifact Size Benchmark Report

aarch64-alpine-linux-musl
Artifact Baseline Commit Change
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.so 7.57 MB 7.57 MB 0% (0 B) 👌
/aarch64-alpine-linux-musl/lib/libdatadog_profiling.a 82.01 MB 82.01 MB 0% (0 B) 👌
aarch64-unknown-linux-gnu
Artifact Baseline Commit Change
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.a 98.27 MB 98.27 MB 0% (0 B) 👌
/aarch64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.02 MB 10.02 MB 0% (0 B) 👌
libdatadog-x64-windows
Artifact Baseline Commit Change
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.dll 24.52 MB 24.52 MB 0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.lib 79.87 KB 79.87 KB 0% (0 B) 👌
/libdatadog-x64-windows/debug/dynamic/datadog_profiling_ffi.pdb 180.30 MB 180.31 MB +0% (+8.00 KB) 👌
/libdatadog-x64-windows/debug/static/datadog_profiling_ffi.lib 914.19 MB 914.19 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.dll 7.75 MB 7.75 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.lib 79.87 KB 79.87 KB 0% (0 B) 👌
/libdatadog-x64-windows/release/dynamic/datadog_profiling_ffi.pdb 23.23 MB 23.23 MB 0% (0 B) 👌
/libdatadog-x64-windows/release/static/datadog_profiling_ffi.lib 45.47 MB 45.47 MB 0% (0 B) 👌
libdatadog-x86-windows
Artifact Baseline Commit Change
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.dll 21.14 MB 21.14 MB 0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.lib 81.11 KB 81.11 KB 0% (0 B) 👌
/libdatadog-x86-windows/debug/dynamic/datadog_profiling_ffi.pdb 184.53 MB 184.53 MB 0% (0 B) 👌
/libdatadog-x86-windows/debug/static/datadog_profiling_ffi.lib 899.90 MB 899.90 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.dll 6.01 MB 6.01 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.lib 81.11 KB 81.11 KB 0% (0 B) 👌
/libdatadog-x86-windows/release/dynamic/datadog_profiling_ffi.pdb 24.88 MB 24.88 MB 0% (0 B) 👌
/libdatadog-x86-windows/release/static/datadog_profiling_ffi.lib 42.97 MB 42.97 MB 0% (0 B) 👌
x86_64-alpine-linux-musl
Artifact Baseline Commit Change
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.a 73.11 MB 73.11 MB 0% (0 B) 👌
/x86_64-alpine-linux-musl/lib/libdatadog_profiling.so 8.45 MB 8.45 MB 0% (0 B) 👌
x86_64-unknown-linux-gnu
Artifact Baseline Commit Change
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.a 90.92 MB 90.92 MB 0% (0 B) 👌
/x86_64-unknown-linux-gnu/lib/libdatadog_profiling.so 10.08 MB 10.08 MB 0% (0 B) 👌

Copy link
Copy Markdown
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

👍 LGTM Really like this one!

Base automatically changed from yannham/otel-thread-ctx-polarsignals-review to main May 20, 2026 16:58
…extRecord

Add compile-time offset_of assertions for every field of the
ThreadContextRecord struct, guarding against silent ABI breakage if
someone reorders or resizes fields in the future.

Move the existing size assertion out of `new()` into a top-level
`const _: () = { ... }` block next to the struct definition, so it
runs unconditionally at compile time regardless of code path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@yannham yannham force-pushed the yannham/otel-thread-ctx-const-assertions branch from 4c6affb to cc8f8fd Compare May 20, 2026 17:10
@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d Bot merged commit 0f83957 into main May 20, 2026
113 checks passed
@gh-worker-dd-mergequeue-cf854d gh-worker-dd-mergequeue-cf854d Bot deleted the yannham/otel-thread-ctx-const-assertions branch May 20, 2026 18:35
gh-worker-dd-mergequeue-cf854d Bot pushed a commit that referenced this pull request May 21, 2026
#2019)

# What does this PR do?

Follow-up to #2018. Minor doc and style improvements in `libdd-otel-thread-ctx`.

- Add a safety comment for the `unsafe` dereference in `ThreadContext::update()`
- Fix `set_attrs` doc comment referencing `record.attrs_data` instead of `self.attrs_data`
- Use `u8::try_from(val_len).unwrap_or_else(...)` for value length capping in `set_attrs`, making the `u8` invariant statically guaranteed instead of relying on a manual `if` + a comment

# Motivation

The `unsafe` block in `update()` lacked a safety justification. The `try_from` pattern eliminates the `as u8` cast and the stale comment that referenced a `min()` that didn't exist.

# Additional Notes

N/A

# How to test the change?

```
cargo test -p libdd-otel-thread-ctx
```

All 8 existing tests pass unchanged.

Co-authored-by: yann.hamdaoui <yann.hamdaoui@datadoghq.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.

4 participants