chore(config): migrate OttlFilterConfiguration to typed config#1980
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the OTTL filter processor to be constructed from the resolved typed traces-domain configuration (SalukiConfiguration) rather than deserializing component-owned YAML (GenericConfiguration) inside the component, aligning this component with the ongoing typed-config cutover.
Changes:
- Build
OttlFilterConfigurationfromagent_data_plane_config::domains::traces::OttlFilterand reuse the sharedOttlErrorMode. - Remove the component-local serde/YAML config module (
ottl_filter_processor/config.rs) and update tests accordingly. - Wire the baseline traces pipeline to pass
ConfigurationSystemso it can read the typed config model.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| bin/agent-data-plane/src/components/ottl_filter_processor/mod.rs | Switches config input to typed traces-domain settings, updates error mode type, and refactors tests away from ConfigurationLoader. |
| bin/agent-data-plane/src/components/ottl_filter_processor/config.rs | Removes the component-owned serde/YAML configuration types now superseded by the shared typed config model. |
| bin/agent-data-plane/src/cli/run.rs | Loads typed config via ConfigurationSystem::config() and passes domains.traces.ottl_filter into the filter processor configuration. |
Comments suppressed due to low confidence (1)
bin/agent-data-plane/src/components/ottl_filter_processor/mod.rs:123
- The inline comment describes propagating an error and dropping the whole payload, but the current implementation cannot propagate errors from
transform_bufferand instead drops the individual span whenerror_mode=propagate. Adjust the comment to match the actual behavior.
//propagate: The processor returns the error up the pipeline. This will result in the payload
// being dropped from the collector.
// AZH: The current API of SynchronousTransform::transform_buffer does not propagate errors;
// it only logs them.
error!(error = %e, "OTTL filter condition error; dropping span (error_mode=propagate)");
return true;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 610dddafed
ℹ️ 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".
Binary Size Analysis (Agent Data Plane)Baseline: 7b98c6f · Comparison: eed5d65 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Build the OTTL filter processor from the resolved traces configuration and remove source-map parsing from the component.
webern
left a comment
There was a problem hiding this comment.
Claude Code, reviewing on behalf of @webern.
Clean-room review of this OTTL filter cutover, focused on regression risk. No blocking issues found; this looks good to merge.
Input sanitization — preserved (and strengthened). The deleted component config enforced deny_unknown_fields on both the root object and the traces subsection, and validated error_mode against a fixed enum. The typed path now matches that: deny_unknown_fields is applied to both OttlFilterConfig and OttlFilterTraces, and error_mode is the typed OttlErrorMode (rename_all = "lowercase"), so a misspelled mode or an unsupported key (e.g. traces.spanevent) still fails at load rather than being silently ignored. The new config-system test exercises exactly these cases. This resolves the earlier validation-loss concern.
Config reachability — preserved. error_mode and traces.span (now span_conditions) both remain reachable through the model.
Behavioral parity — preserved. The default remains Propagate; the empty/trimmed-condition skip and all three error-mode branches are unchanged.
Test coverage — preserved. Behavioral tests are retained and now construct the typed model directly. The removed deserialization tests are appropriately relocated to the config-system layer with broader coverage.
Minor / optional: a couple of the test doc comments still describe the old ottl_filter_config / traces.span YAML keys rather than the typed model fields — worth a touch-up but not blocking.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
## AI Summary Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing `GenericConfiguration` in the component. Remove the component-owned source serde configuration and use the shared `OttlErrorMode` model type. ## Change Type - [x] Non-functional (chore, refactoring, docs) ## How did you test this PR? - `make build-schema-overlay && make fmt` - `make check-all` - `make test` - `make check-docs` - Targeted OTTL filter processor tests ## References Related to configuration cutover PR #1979.
AI Summary
Build the OTTL filter processor from the resolved traces-domain configuration instead of parsing
GenericConfigurationin the component. Remove the component-owned source serde configuration and use the sharedOttlErrorModemodel type.Change Type
How did you test this PR?
make build-schema-overlay && make fmtmake check-allmake testmake check-docsReferences
Related to configuration cutover PR #1979.