Repository navigation
ci: add a cargo-machete gate for unused dependencies - #1471
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Keep the #1467 dependency pins without ddsketch-agent, regenerate Cargo.lock and LICENSE-3rdparty.csv, and list libddwaf-sys in the cargo-machete ignore list: it is a dependency only to enable its aws-lc-rs feature.
…/cargo-machete/feature
litianningdatadog
approved these changes
Nov 2, 2026
duncanista
added a commit
that referenced
this pull request
Nov 2, 2026
**TL;DR:** Removes 6 dependencies that no code uses: indexmap, serde-aux, lazy_static, log, ddsketch-agent and proptest. **Why:** Less to compile, audit and keep up to date. The next PR (#1471) adds a CI check so that unused dependencies do not come back. ## Overview Jira: [SVLS-9975](https://datadoghq.atlassian.net/browse/SVLS-9975) Based on `main` (#1469 is merged). - Remove 6 dependencies that no code uses: `indexmap`, `serde-aux`, `lazy_static`, `log`, `ddsketch-agent` and the dev-dependency `proptest`. The lock drops the 4 packages that only `proptest` pulled in: `bit-set` 0.8, `bit-vec` 0.8, `rusty-fork` and `wait-timeout`. Older `bit-set` (0.5) and `bit-vec` (0.6) stay in the lock, through the `httpmock` dev dependency. `LICENSE-3rdparty.csv` drops the same 4 rows, so the required "Valid LICENSE-3rdparty.csv" check passes. - `[package.metadata.cargo-machete] ignored` lists the 5 crates that stay on purpose, with the reason in a comment: version bounds on transitive crates, `aws-lc-rs` reached through `rustls::crypto`, `serde_bytes` used in a `#[serde(with)]` string, and `libddwaf-sys`, which is listed only to enable its `aws-lc-rs` feature (#1467). The next PR (#1471) adds the CI job that reads it. Size: about 16 core lines; the rest is `Cargo.lock` and `LICENSE-3rdparty.csv`. ## Testing - applies on the previous PR of the stack: pass - `cargo metadata --locked`: pass - `cargo deny --locked check`: pass - `cargo machete`: pass - `dd-rust-license-tool check`: pass - `cargo clippy --workspace --all-targets --features default -- -D warnings`: pass - `cargo clippy --workspace --all-targets --features default,test-mode -- -D warnings`: pass - Before this stack, the FIPS flavor compiled in Docker with the removal (`compile_bottlecap.sh`, arm64, FIPS=1). [SVLS-9975]: https://datadoghq.atlassian.net/browse/SVLS-9975?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Base automatically changed from
jordan.gonzalez/remove-unused-deps/feature
to
main
November 2, 2026 21:10
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the added unused-dependency CI check.
No open issues 🎉
What changed in this PR
Adds a CI check to catch unused Rust dependencies after the cleanup in #1470, helping prevent them from accumulating again.
Changes:
- Adds a non-required
Unused Dependenciesjob with read-only repository permissions. - Pins the installer action and cargo-machete version.
- Runs static dependency analysis in
bottlecapwithout building the project.
| File | Description |
|---|---|
.github/workflows/rs_ci.yml |
Adds the pinned cargo-machete job to detect unused dependencies. |
Review effort: Balanced
duncanista
added a commit
that referenced
this pull request
Nov 5, 2026
**TL;DR:** One shared set of generated test inputs: Telemetry API batches, Lambda log lines, traces, DogStatsD packets and trigger events. **Why:** The fault-injection harness (#1473) and the benchmarks (#1474) need the same realistic inputs. Today each test builds its own by hand. The extension does not change. ## Overview Jira: [SVLS-9977](https://datadoghq.atlassian.net/browse/SVLS-9977) Stacked on #1471 (`jordan.gonzalez/cargo-machete/feature`). This PR does not need the CI PRs below it. It sits on this stack so that the bench PR can build on both. ### Why this PR We are adding test tools for the extension: - criterion benchmarks for the hot paths (#1474) - a fault-injection harness that runs the real `bottlecap` binary against a fake Lambda API and an intake that fails on demand (#1473) - later, a local Lambda simulator All of them need the same realistic inputs: Telemetry API batches, Lambda log lines, trace payloads, DogStatsD packets and trigger events. Today each test builds its own inputs by hand. This PR adds one small set of generators for those inputs, so every tool tests and measures the same shapes. Large inputs, such as a 220 KB SQS batch, are generated at test time instead of being checked into git. On its own, this PR only adds test code. The extension does not change. The only non-test change is 9 lines that make the existing `stitch` module `#[doc(hidden)] pub`, so integration tests and benches can call it. What to check here: that the generated shapes match what Lambda sends (for example the split-record format that the stitcher reassembles since #1415), and the self-test in `tests/fixtures_corpus.rs`. ### What is in it `bottlecap/tests/fixtures/` generates the inputs that the benches, the fault-injection harness and the Lambda simulator share, so no large blob is in git: - `telemetry`: Telemetry API batches (platform events, function logs in 3 formats, split records for the stitcher, `runtimeDone` with or without metrics, `logsDropped`, unknown events); - `logs`: Lambda text and JSON lines, one OOM line per runtime; - `traces`: synthetic traces and the 2000-span v0.4 msgpack payload, at a fixed base time; - `dogstatsd`: datagrams for 50 metric names x 100 endpoints; - `events`: the checked-in trigger payloads plus a 220 KB SQS and a 100-record DynamoDB event. `tests/fixtures_corpus.rs` checks the generators (for example, a split record stitches back). It needs `extension::telemetry::stitch` from the integration tests, so the module and `FragmentBuffer` become `#[doc(hidden)] pub`. No behavior change. Size: 13 core lines, 1058 lines in total. ## Testing - applies on the previous PR of the stack: pass - `cargo fmt --all -- --check`: pass - `cargo clippy --workspace --all-targets --features default -- -D warnings`: pass - `cargo clippy --workspace --all-targets --features default,test-mode -- -D warnings`: pass - `cargo nextest run --workspace`: pass [SVLS-9977]: https://datadoghq.atlassian.net/browse/SVLS-9977?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR: Adds a CI job (cargo-machete) that fails when
Cargo.tomllists a dependency that the code does not use.Why: The 6 unused dependencies that #1470 removes built up over time, because nothing caught them.
Overview
Jira: SVLS-9975
Stacked on #1470 (
jordan.gonzalez/remove-unused-deps/feature).Add the
Unused Dependenciesjob:cargo-machete0.9.2 throughtaiki-e/install-actionv2.87.28. It is static analysis, with no build, so it is fast and deterministic. A crate used
only through a string path or kept as a version bound goes in
[package.metadata.cargo-machete] ignored(previous PR). It is not a required check.Size: 17 core lines, 17 lines in total.
Testing
cargo machete: pass