chore: add and agents.md#1956
Conversation
![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: e583d45526
ℹ️ 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".
| # Hyper backend | ||
| cargo nextest run -p libdd-http-client --no-default-features --features hyper-backend,https | ||
| ``` | ||
| - **test_spawn_from_lib**: `cargo nextest run --package test_spawn_from_lib --features prefer-dynamic`. |
There was a problem hiding this comment.
For test_spawn_from_lib on Unix/Linux, enabling the prefer-dynamic Cargo feature is not enough: the crate's own Cargo.toml notes these tests require prefer-dynamic, and the CI step in .github/workflows/test.yml sets RUSTFLAGS="-C prefer-dynamic" when running this package. Agents following this documented command will run a different validation path from CI and can get misleading failures when the trampoline test binary is not built dynamically.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1956 +/- ##
==========================================
- Coverage 72.83% 72.80% -0.03%
==========================================
Files 459 459
Lines 76134 76134
==========================================
- Hits 55455 55433 -22
- Misses 20679 20701 +22
🚀 New features to boost your workflow:
|
|
🎯 Code Coverage (details) 🔗 Commit SHA: beaa018 | Docs | Datadog PR Page | Give us feedback! |
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
|
| ### Licenses | ||
| All source files must have Apache 2.0 license headers (except `symbolizer-ffi`). The third-party license CSV (`LICENSE-3rdparty.csv`) is validated in CI. To regenerate: | ||
| ```bash | ||
| ./scripts/update_license_3rdparty.sh |
There was a problem hiding this comment.
this probably would work better as pre-commit hook :)
| - Avoid `unwrap`/`panic!` outside of tests; bubble errors up instead. | ||
| - Bubble errors up to the library caller with detail — prefer structured error enums (e.g. `thiserror`) over opaque strings. |
There was a problem hiding this comment.
L127-128 should probably could be merged together
| - **Rust**: MSRV `1.84.1` (set in workspace `Cargo.toml`); use stable for build/clippy. | ||
| - **Nightly rustfmt**: `nightly-2026-02-08` — `rustfmt.toml` uses nightly-only features. | ||
| - **cargo-nextest**: `0.9.96` (required for running tests). Install with `cargo install --locked 'cargo-nextest@0.9.96'`. | ||
| - **cbindgen**: `0.29` (for FFI header generation). | ||
| - **System tools**: `cmake` and `protoc`. |
There was a problem hiding this comment.
this should all probably get inferred from metadada
There was a problem hiding this comment.
Agreed, otherwise this is going to be drifting hard. Also, for most of them the version isn't that relevant (e.g. cargo nextest), though it is for some others.
| cargo check -p <crate> # fast iteration on a single crate | ||
| cargo build --workspace --exclude builder # full build | ||
| ``` | ||
| 2. **Format and lint** — always run on every crate that was touched, before finishing: |
There was a problem hiding this comment.
I don't think it's a bad think if some instructions are both in the pre-commit hook and AGENT.md given that not everyone will enable pre-commit hooks, for example it might not work properly out of the box for some people or some might find them too slow (ie me when touching one line in datadog-agent and it taking 10mins to run 🥲 )
| ```bash | ||
| cargo ffi-test | ||
| ``` | ||
| 5. **If `tracing_integration_tests::` tests fail**, they require Docker. Prompt the user to start Docker and retry; to skip them locally use: |
There was a problem hiding this comment.
this is great example of filling the knowledge cracks which is not easly deduced
| - **libdd-shared-runtime** / **libdd-shared-runtime-ffi** — shared Tokio runtime with fork-safe worker management | ||
| - **libdd-capabilities** / **libdd-capabilities-impl** — portable capability traits and native implementations for cross-platform libdatadog | ||
| - **libdd-http-client** — HTTP client abstraction with `reqwest-backend` (default) and `hyper-backend` features | ||
| - **libdd-dogstatsd-client** — DogStatsD metrics client |
There was a problem hiding this comment.
it will infer this information, overall I suspect most of this section is redundant, but I haven't used this AGENTS.md yet, so I'm not 100% sure
There was a problem hiding this comment.
I agree. I'm not sure it's useful/worth the trouble putting this here. Domain-specific knowledge or datadog-specific constraints that can't be easily inferred, like the test section above, seems to be more valuable and less inclined to drift.
| - **Rust**: MSRV `1.84.1` (set in workspace `Cargo.toml`); use stable for build/clippy. | ||
| - **Nightly rustfmt**: `nightly-2026-02-08` — `rustfmt.toml` uses nightly-only features. | ||
| - **cargo-nextest**: `0.9.96` (required for running tests). Install with `cargo install --locked 'cargo-nextest@0.9.96'`. | ||
| - **cbindgen**: `0.29` (for FFI header generation). | ||
| - **System tools**: `cmake` and `protoc`. |
There was a problem hiding this comment.
Agreed, otherwise this is going to be drifting hard. Also, for most of them the version isn't that relevant (e.g. cargo nextest), though it is for some others.
| 2. **Format and lint** — always run on every crate that was touched, before finishing: | ||
| ```bash | ||
| cargo +nightly-2026-02-08 fmt --all -- --check | ||
| cargo +stable clippy --workspace --all-targets --all-features -- -D warnings -A clippy::manual_is_multiple_of |
There was a problem hiding this comment.
Where is this accept coming from? If there's some config clippy file, better link to it rather than hardcoding allowlisted lints.
There was a problem hiding this comment.
From the CI. The reason for it is that satisfying this lint requires rust 1.85, which is over the MSRV.
I could add a lint skip, with a comment to remove it when we update the MSRV
| - **libdd-shared-runtime** / **libdd-shared-runtime-ffi** — shared Tokio runtime with fork-safe worker management | ||
| - **libdd-capabilities** / **libdd-capabilities-impl** — portable capability traits and native implementations for cross-platform libdatadog | ||
| - **libdd-http-client** — HTTP client abstraction with `reqwest-backend` (default) and `hyper-backend` features | ||
| - **libdd-dogstatsd-client** — DogStatsD metrics client |
There was a problem hiding this comment.
I agree. I'm not sure it's useful/worth the trouble putting this here. Domain-specific knowledge or datadog-specific constraints that can't be easily inferred, like the test section above, seems to be more valuable and less inclined to drift.
| - Avoid `unwrap`/`panic!` outside of tests; bubble errors up instead. | ||
| - Bubble errors up to the library caller with detail — prefer structured error enums (e.g. `thiserror`) over opaque strings. | ||
| - Stay free of global effects unless a feature requires them: no spawning threads, no globals, no reading environment variables behind the caller's back. | ||
| - Care about performance, especially memory allocations on hot paths. |
There was a problem hiding this comment.
Should we add something about being extra-careful regarding UB, given FFI and/or unsafe code?
There was a problem hiding this comment.
Added a section about FFI
| - Care about performance, especially memory allocations on hot paths. | ||
|
|
||
| ### Cryptography | ||
| - Non-FIPS builds: ring as TLS crypto provider |
There was a problem hiding this comment.
I don't know about AI, but as a human who comes to this codebase, I would find this section inscrutable because acronyms aren't explained. Maybe should we quickly explain (one sentence) what FIPS is and why someone working on libdatadog should care?
| ### Release profiles | ||
| - `dev`: full debug info | ||
| - `release`: size-optimized (`opt-level = "s"`), LTO, single codegen unit | ||
| - `bench`: `opt-level = 3` |
There was a problem hiding this comment.
This is readable from the (rather small) Cargo.toml and subject to drift. Not sure it's worth repeating here.
| ``` | ||
| - **test_spawn_from_lib**: `cargo nextest run --package test_spawn_from_lib --features prefer-dynamic`. | ||
|
|
||
| ## Architecture |
There was a problem hiding this comment.
Lately we've been adding , removing and splitting crates. Therefore I guess this section will become outdated in no time.
There was a problem hiding this comment.
Deleted the crates section. We can add it back latter if it seems useful
|
I removed the sections that are the most likely to drift, we can add them back latter if needed |
| - Care about performance, especially memory allocations on hot paths. | ||
|
|
||
| ### Cryptography | ||
| - Non-FIPS builds: ring as TLS crypto provider |
| - Windows FIPS requires env var: `AWS_LC_FIPS_SYS_NO_ASM=1` | ||
|
|
||
| ### Commit Messages | ||
| PR titles and commits must follow **Conventional Commits**: `<type>[scope]: <description>` |
There was a problem hiding this comment.
? Not sure it's the right way to write it. Maybe an example would work better, but to indicate there are parentheses.
| PR titles and commits must follow **Conventional Commits**: `<type>[scope]: <description>` | |
| PR titles and commits must follow **Conventional Commits**: `<type>([scope]): <description>` |
|
/merge |
|
View all feedbacks in Devflow UI.
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.
The expected merge time in
This pull request was merged directly. |
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: waiting
and removed
mergequeue-status: waiting
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
6 participants
What does this PR do?
Add an AGENTS.md file
It is more or less structured like the datadog-agent repo agents.md which I found to work well
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.