chore(config): migrate Datadog Logs to typed config#1982
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Datadog Logs encoder configuration away from raw GenericConfiguration deserialization and instead builds it from the shared typed compression configuration exposed by agent_data_plane_config.
Changes:
- Replaced
DatadogLogsConfiguration’s serde-based config deserialization with a constructor that maps fromagent_data_plane_config::shared::Compression. - Updated the agent-data-plane topology wiring to pass
saluki.shared.endpoints.compressioninto the logs pipeline builder. - Replaced the config smoke test with a unit test validating the new mapping behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/saluki-components/src/encoders/datadog/logs/mod.rs | Switch logs encoder config to be constructed from shared typed compression settings; adjust tests accordingly. |
| bin/agent-data-plane/src/cli/run.rs | Wire the logs pipeline to use typed shared compression config instead of generic config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Creates a new `DatadogLogsConfiguration` from the shared compression settings. | ||
| pub fn from_configuration(compression: &Compression) -> Result<Self, GenericError> { | ||
| Ok(Self { | ||
| compressor_kind: compression.compressor_kind.clone(), | ||
| zstd_compressor_level: compression.zstd_compressor_level, | ||
| }) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a66a87c9
ℹ️ 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".
| pub fn from_configuration(compression: &Compression) -> Result<Self, GenericError> { | ||
| Ok(Self { | ||
| compressor_kind: compression.compressor_kind.clone(), | ||
| zstd_compressor_level: compression.zstd_compressor_level, |
There was a problem hiding this comment.
Preserve ADP's zstd default for logs
When serializer_zstd_compressor_level is omitted, this now copies the shared typed value populated from the generated Datadog config default of 1 (lib/datadog-agent/config/src/generated/datadog_configuration.rs:354), but the overlay explicitly documents that ADP intentionally defaults this key to level 3 while the Agent defaults to 1, and the removed logs config used default_zstd_compressor_level() -> 3. Default log pipelines therefore silently switch to weaker zstd compression while the other encoders still use level 3, increasing log payload sizes for users who do not set the key.
Useful? React with 👍 / 👎.
Binary Size Analysis (Agent Data Plane)Baseline: 7b98c6f · Comparison: aa34a87 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (3)Experiments configured
Bounds Checks: ✅ Passed (3)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
webern
left a comment
There was a problem hiding this comment.
Review by Claude Code, operating on behalf of @webern.
Summary
The mechanical cutover is clean: the encoder now builds from shared.endpoints.compression, the two keys (serializer_compressor_kind, serializer_zstd_compressor_level) remain reachable through the schema → translator path, and no input validation/coercion was dropped (nothing existed at the component layer before; the values still flow straight into CompressionScheme::new). CI, regression detector, and bounds checks are all green.
There is one real regression, however, which Copilot and Codex both also caught.
Default zstd compressor level regresses 3 → 1
The removed component code defaulted serializer_zstd_compressor_level to 3:
const fn default_zstd_compressor_level() -> i32 { 3 }After this PR the value comes from shared.endpoints.compression.zstd_compressor_level, which the translator populates from the generated Datadog config default of 1 (lib/datadog-agent/config/src/generated/datadog_configuration.rs, serializer_zstd_compressor_level: defaults::default_u64::<i64, 1>()). Nothing seeds an ADP default of 3 at the shared/typed layer, so when the key is unset the effective default silently drops from 3 to 1.
This directly contradicts the documented intent in schema_overlay.yaml:
"ADP defaults to level 3 vs Agent level 1. ADP intentionally compresses better. Key is recognized by both; default divergence is intentional."
The overlay documents the divergence in prose but never actually sets default: 3, so the generated default is 1. Net effect: default log pipelines get weaker zstd compression (larger payloads), and Logs now diverges from the sibling Datadog encoders (metrics/events/service-checks/traces) that still default to 3 until they're migrated.
Suggested fix
Per the "defaults live in the config layer" principle, establish ADP's intended default of 3 at the typed/shared layer rather than in the component — e.g. give serializer_zstd_compressor_level a default: 3 override in schema_overlay.yaml so the generated default matches the documented intent. Doing it at the shared layer fixes it uniformly for every Datadog encoder that reads shared.endpoints.compression, not just Logs.
serializer_compressor_kind is unaffected — both the old component default and the schema default are zstd.
Tests
Dropping the old config_smoke test is fine — it was purely a deserialization/remapping check. The replacement only verifies a trivial field copy, so nothing now covers the effective default value. A translator-level assertion that shared.endpoints.compression.zstd_compressor_level defaults to the intended value would have caught this regression and is worth adding alongside the fix.
Build the Logs encoder from shared typed compression settings and retire its raw configuration deserialization.
webern
left a comment
There was a problem hiding this comment.
Follow-up by Claude Code, operating on behalf of @webern.
The zstd-default regression I flagged earlier is resolved by the rebase. The fix now lives in the config layer, where it belongs:
- New constant
ZSTD_DEFAULT_OVERRIDE = 3inagent-data-plane-config/src/shared.rs. consume_serializer_zstd_compressor_levelcompares the incoming level to the Agent's schema default and substitutes ADP's intended level 3 when they match, leaving any explicit operator value untouched.
shared.endpoints.compression.zstd_compressor_level now defaults to 3, matching the documented ADP intent and the old component behavior, and it applies uniformly to every Datadog encoder rather than just Logs. The trivial 1→3 substitution doesn't warrant a dedicated test.
The documented caveat stands — an operator who explicitly sets level 1 is indistinguishable from the Agent default and is also bumped to 3 — but that's noted in-code as follow-up work.
No remaining concerns. This is ready to merge.
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs`
Human Summary
TODO: human writes here
AI Summary
Build the Datadog Logs encoder from the shared typed compression configuration and remove its raw configuration deserialization.
Change Type
How did you test this PR?
make build-schema-overlay && make fmtmake check-allmake testmake check-docsReferences