chore(config): migrate DatadogEventsConfiguration to typed config#1981
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Refactors the Datadog events encoder configuration flow so DatadogEventsConfiguration is constructed from the typed shared configuration model (agent_data_plane_config) instead of deserializing from the generic config map, and updates the baseline events pipeline wiring accordingly.
Changes:
- Build
DatadogEventsConfigurationfrom typed shared config (MetricsEncoding+Compression) rather thanGenericConfiguration. - Remove component-local serde defaults / deserialization path for the events encoder config.
- Replace the prior config smoke test with a focused unit test for the new typed-config constructor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/saluki-components/src/encoders/datadog/events/mod.rs | Switches events encoder config construction to typed shared config and replaces config smoke testing with a unit test. |
| bin/agent-data-plane/src/cli/run.rs | Updates baseline events pipeline setup to use ConfigurationSystem (typed config) when creating the events encoder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Maximum compressed size, in bytes, of an events payload. | ||
| /// | ||
| /// This uses the same generic event payload setting as the Datadog Agent. ADP sends events to | ||
| /// `/api/v1/events_batch`, so the effective value is clamped to that endpoint's global intake limit of 3,200,000 | ||
| /// bytes. If set to `0`, every non-empty compressed payload exceeds the limit and is dropped during flush. |
| /// Maximum uncompressed size, in bytes, of an events payload. | ||
| /// | ||
| /// This uses the same generic event payload setting as the Datadog Agent. ADP sends events to | ||
| /// `/api/v1/events_batch`, so the effective value is clamped to that endpoint's global intake limit of 62,914,560 | ||
| /// bytes. Values smaller than the minimum endpoint framing size prevent the request builder from starting. |
| /// Compression kind to use for the request payloads. | ||
| /// | ||
| /// Defaults to `zstd`. | ||
| #[serde( | ||
| rename = "serializer_compressor_kind", | ||
| default = "default_serializer_compressor_kind" | ||
| )] | ||
| compressor_kind: String, | ||
|
|
||
| /// Compressor level to use when the compressor kind is `zstd`. | ||
| /// | ||
| /// Defaults to 3. | ||
| #[serde( | ||
| rename = "serializer_zstd_compressor_level", | ||
| default = "default_zstd_compressor_level" | ||
| )] | ||
| zstd_compressor_level: i32, | ||
|
|
| /// Whether to log event payload contents before encoding. | ||
| /// | ||
| /// This logs decoded event objects, not the encoded HTTP body. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b9a232e9
ℹ️ 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".
| max_payload_size: metrics_encoding.max_payload_size, | ||
| max_uncompressed_payload_size: metrics_encoding.max_uncompressed_payload_size, | ||
| compressor_kind: compression.compressor_kind.clone(), | ||
| zstd_compressor_level: compression.zstd_compressor_level, |
There was a problem hiding this comment.
Preserve ADP zstd level default for events
When the events pipeline is enabled and serializer_zstd_compressor_level is omitted, this now copies the typed Datadog default of 1; the project overlay explicitly documents ADP's intentional default divergence as level 3 (lib/datadog-agent/config/schema/schema_overlay.yaml:2253-2257), and the removed component default also returned 3. That makes this refactor change default event compression behavior, producing larger payloads/more splits and potentially dropping events near the compressed limit, so the migration needs to preserve the ADP default when the key is unset.
Useful? React with 👍 / 👎.
Binary Size Analysis (Agent Data Plane)Baseline: 7b98c6f · Comparison: f41a555 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Build the events encoder from the shared SalukiConfiguration model and remove source-map deserialization from the component.
|
Clean-room review — Claude Code operating on behalf of @webern. Blocking:
|
|
Correction to my earlier comment — Claude Code operating on behalf of @webern. I need to retract the "blocking" framing on the In the default deployment ( That means the old component code path already saw The only case that changes is standalone mode (no Agent config stream): there the config is just the local file + env, so an unset key was previously absent and fell back to the component default Net: this cutover is behavior-preserving where it matters. The one real follow-up is documentation, not code — the schema overlay's note for Apologies for the noise on the first pass. |
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 ( |
| // operator who deliberately set `1` apart from the Agent's default `1`, so both are raised to | ||
| // the floor. Follow-up work needs to track each value's source (user-set vs Agent default) so | ||
| // we can honor an explicit low level while still applying our floor to the Agent default. | ||
| self.config.shared.endpoints.compression.zstd_compressor_level = (value as i32).max(ZSTD_MINIMUM); |
| /// Creates a new `DatadogEventsConfiguration` from the shared typed configuration. | ||
| pub fn from_configuration( | ||
| metrics_encoding: &MetricsEncoding, compression: &Compression, | ||
| ) -> Result<Self, GenericError> { | ||
| Ok(Self { |
| Ok(config.as_typed()?) | ||
| /// Creates a new `DatadogEventsConfiguration` from the shared typed configuration. | ||
| pub fn from_configuration( | ||
| metrics_encoding: &MetricsEncoding, compression: &Compression, |
The core Agent streams a fully resolved config, so its schema default for serializer_zstd_compressor_level arrives as a concrete value that would otherwise overwrite ADP's intended level. Add a ZSTD_DEFAULT_OVERRIDE const in the config layer and, when the incoming level equals the Agent's schema default, substitute it, preserving ADP's harder-compression behavior. Any other value is treated as an explicit operator choice and left untouched.
| pub fn from_configuration( | ||
| metrics_encoding: &MetricsEncoding, compression: &Compression, | ||
| ) -> Result<Self, GenericError> { | ||
| Ok(Self { | ||
| max_payload_size: metrics_encoding.max_payload_size, | ||
| max_uncompressed_payload_size: metrics_encoding.max_uncompressed_payload_size, | ||
| compressor_kind: compression.compressor_kind.clone(), | ||
| zstd_compressor_level: compression.zstd_compressor_level, | ||
| log_payloads: metrics_encoding.log_payloads, | ||
| }) | ||
| } | ||
| } |
| fn consume_serializer_zstd_compressor_level(&mut self, value: i64) { | ||
| self.config.shared.endpoints.compression.zstd_compressor_level = value as i32; | ||
| // TODO: The core Agent streams a fully resolved config, so its schema default for | ||
| // `serializer_zstd_compressor_level` arrives here as a concrete value rather than being | ||
| // absent. When the incoming level matches that Agent default we swap in ADP's intended | ||
| // level; without this the Agent default would silently override it. We compare against the | ||
| // schema-generated default, so an operator who deliberately sets exactly the Agent default is | ||
| // indistinguishable from the default itself and also gets overridden. Removing that ambiguity | ||
| // needs per-value source tracking (user-set vs Agent default), which is follow-up work. | ||
| let agent_default = DatadogConfiguration::default().serializer_zstd_compressor_level; | ||
| self.config.shared.endpoints.compression.zstd_compressor_level = if value == agent_default { | ||
| ZSTD_DEFAULT_OVERRIDE | ||
| } else { | ||
| value as i32 | ||
| }; |
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
) ## AI Summary Build `DatadogEventsConfiguration` from the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-fmt check-docs` - `make test` - `make check-deny check-unused-deps check-licenses check-features generate-api-docs` - `cargo check -p saluki-components -p agent-data-plane` ## References No issue.
AI Summary
Build
DatadogEventsConfigurationfrom the typed shared configuration model instead of deserializing the generic configuration map. Remove component-local source defaults and retire the configuration smoke test.Change Type
How did you test this PR?
make build-schema-overlay && make fmtmake check-fmt check-docsmake testmake check-deny check-unused-deps check-licenses check-features generate-api-docscargo check -p saluki-components -p agent-data-planeReferences
No issue.