chore(config): cutover adp bin components to typed config system#1995
chore(config): cutover adp bin components to typed config system#1995webern wants to merge 3 commits into
Conversation
Binary Size Analysis (Agent Data Plane)Baseline: 33d6ac6 · Comparison: 719e11d · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
💡 Codex Reviewsaluki/bin/agent-data-plane/src/cli/run.rs Line 180 in 831aeca When the new config stream is not in use, This deserialize still rejects wider input forms before the translator can parse them; for example, ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
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 ( |
|
47a5a4f to
f638633
Compare
f638633 to
c892b06
Compare
) Cut the OTTL transform processor over from the raw `GenericConfiguration` map to the typed `SalukiConfiguration` model, following the pattern already applied to the sibling OTTL filter processor. - `OttlTransformConfiguration::from_configuration` now takes the resolved `domains::traces::OttlTransform` settings and copies them directly, instead of deserializing an `OttlTransformConfig` out of `GenericConfiguration`. The component-local config module (its `ErrorMode` enum and `OttlTransformConfig` struct) is removed; the component reuses the model's shared `OttlErrorMode`. - The call site in `run.rs` reads `saluki.domains.traces.ottl_transform`. Since this was the last consumer of the raw config in `add_baseline_traces_pipeline_to_blueprint`, the now-unused `GenericConfiguration` parameter is dropped from that function. - The seed source `OttlTransformConfig` in `saluki_only.rs` is aligned with the filter's: its `error_mode` deserializes directly into `OttlErrorMode` (rather than a loose `Option<String>` fed through a hand-rolled parse helper) and it now rejects unknown fields, restoring the strict deserialization the original component had. `ottl_transform_config` is absent from the vendored Datadog schema, so it is a Saluki-only key on the seed track; there is no witnessed alias for it. - [x] Non-functional (chore, refactoring, docs) - Rewrote the component tests to build from typed traces settings; all OTTL transform tests pass. - Added an `ottl_transform_config` round-trip rejection test mirroring the filter's. - `make build-schema-overlay` leaves the tree unchanged; `make fmt`, `make check-all`, `make test`, and `make check-docs` all pass. - Progresses #1788 - Merges into #1987
It thrashed a bit on preserving the exact behavior that `DurationString` was providing and on viper `time.Duration` parsing correctness. Overall the changes it made seem fine to me. Migrates `HostTagsConfiguration` from the raw `GenericConfiguration` map to the typed `SalukiConfiguration` model, continuing the ADP config cutover. `expected_tags_duration` is witnessed and already modeled at `shared.tags.expected_tags_duration`, so the component now reads it directly from the typed slice instead of parsing a `DurationString` off `GenericConfiguration`. The component's hand-written zero default is dropped; the generated schema default (`0s`) is authoritative. `RemoteAgentClientConfiguration` is a shared struct still consumed by several not-yet-migrated remote-agent components, so it stays on `GenericConfiguration` for now. It is built at the call site and passed into `HostTagsConfiguration`, which removes `GenericConfiguration` from the component entirely. - [x] Non-functional (chore, refactoring, docs) `make check-all`, `make test`, and `make check-docs` all pass. Translation of `expected_tags_duration` into the typed model is already covered by existing translator and config-system tests. - Progresses #1788 - Merges into #1987
|
Closing this branch. Like its parent, it builds on the typed-configuration system that changed substantially underneath it (redesigned environment-variable handling and validate-then-commit runtime update semantics), which invalidated this ADP-binary cutover approach. The work is preserved as a frozen snapshot on my fork at archive/pr1995-adp-bin-cutover for future reference. |
Human Summary
Cut over some of the
agent-data-planebinary components to the new typed config system.As it turns out, many of the
GenericConfiguration-reading components and sites inagent-data-planeare difficult to cut over without further work. Many are involved in bootstrapping and remote agent set-up. Some are pretty complicated (#1998). But for those that seemed pretty reasonable to cut over to typed config, this PR does so.AI Summary
Continues the
agent-data-planecutover from the rawGenericConfigurationmap to the typedSalukiConfigurationmodel. Two components move over, plus a supporting consolidation of durationparsing.
OTTL transform processor.
OttlTransformConfiguration::from_configurationnow takes theresolved
domains::traces::OttlTransformsettings and copies them, instead of deserializing anOttlTransformConfigout ofGenericConfiguration. The component-local config module (itsErrorModeenum andOttlTransformConfigstruct) is removed in favor of the model's sharedOttlErrorMode. This was the last raw-config consumer inadd_baseline_traces_pipeline_to_blueprint, so its now-unusedGenericConfigurationparameter isdropped. The
ottl_transform_configseed source insaluki_only.rsis aligned with the OTTLfilter's:
error_modedeserializes directly intoOttlErrorMode(replacing a looseOption<String>and a hand-rolled parse helper) and it rejects unknown fields.Host tags.
expected_tags_durationis witnessed and already modeled atshared.tags.expected_tags_duration, soHostTagsConfigurationreads it directly from the typedslice rather than parsing a
DurationStringoffGenericConfiguration; the hand-written zerodefault is dropped in favor of the generated schema default.
RemoteAgentClientConfigurationisstill shared by not-yet-migrated remote-agent components, so it stays on
GenericConfiguration,built at the call site and passed in. This removes
GenericConfigurationfrom the componententirely.
Duration parsing consolidation. The viper/cast coercion that
saluki_config::DurationStringperformed (trim whitespace, parse the Go duration grammar, else treat a bare integer as
nanoseconds; reject negatives and values past the
time.Durationi64::MAX-nanosecond bound) nowlives once in the
go-durationcrate asparse_viper_durationplus checked numeric conversions.DurationStringand the typedDatadogConfigurationduration deserializer both build on these, sothe typed path accepts and rejects exactly what
DurationStringdid before the migration. Notablythe typed path no longer silently clamps negative numeric durations to zero, which would have
disabled duration-gated features like host-tag enrichment.
Docs. A note in
AGENTS.mdpoints at the config ingest crates(
lib/agent-data-plane-config,lib/agent-data-plane-config-system).ottl_transform_configis absent from the vendored Datadog schema, so it remains a Saluki-only keywith no witnessed alias.
Change Type
How did you test this PR?
ottl_transform_configround-trip rejection test mirroring the filter's.trimming, negative and overflow rejection) on both
go-durationand the typed deserializer.expected_tags_durationis covered by existing translator andconfig-system tests.
make build-schema-overlayleaves the tree unchanged;make fmt,make check-all,make test,and
make check-docsall pass.References