chore(config): migrate OTLP components to typed config#1989
Conversation
Cut OtlpConfig, the OTLP source, decoder, forwarder, and relay over from the raw GenericConfiguration map to the typed SalukiConfiguration model. - Source/relay/decoder/forwarder build from `domains.otlp` and `domains.traces.otlp` slices; smoke tests retired in favor of construction and translator tests. - Convert the seeded OTLP knobs (context sizing, HTTP transport, trace interner size, top-level-by-span-kind) to the Rule 2 doctrine: non-optional in SalukiOnly with shared defaults defined once in agent-data-plane-config. - OtlpTracesTranslator now takes the two native bools it reads instead of the raw TracesConfig; TracesConfig is kept only for the not-yet-migrated Datadog trace encoder. - gRPC max_recv_msg_size_mib of 0 now maps to grpc-go's 4 MiB default, matching the Datadog Agent and the schema documentation.
dd-octo-sts
Bot
added
area/components
There was a problem hiding this comment.
Pull request overview
Migrates OTLP pipeline components (source/relay/decoder/forwarder and trace translator inputs) from deserializing the raw GenericConfiguration map to consuming the resolved typed SalukiConfiguration domain models, aligning OTLP with the typed-config construction pattern introduced in prior config-migration work.
Changes:
- Refactors OTLP component configuration constructors to accept typed model slices (
domains.otlp/domains.traces.otlp) and removes component-local serde/defaults and config smoke tests. - Centralizes/normalizes OTLP sizing/transport defaults in the typed config domains and seeding layer (including ByteSize parsing for “bare int” vs “suffixed string” inputs).
- Adds a shared helper for translating
max_recv_msg_size_mibinto bytes (with “0 means grpc-go default” semantics) and wires it into OTLP source/relay.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/saluki-components/src/sources/otlp/resolver.rs | Switches context resolver sizing to use typed OTLP config fields. |
| lib/saluki-components/src/sources/otlp/mod.rs | Rebuilds OTLP source config from typed domains; updates translator/server wiring; replaces smoke tests with construction/mapping tests. |
| lib/saluki-components/src/relays/otlp/mod.rs | Reworks OTLP relay config to parse endpoints/max message size from typed OTLP domain. |
| lib/saluki-components/src/forwarders/otlp/mod.rs | Updates forwarder config to read trace OTLP settings from typed traces domain. |
| lib/saluki-components/src/decoders/otlp/mod.rs | Updates decoder config to read trace OTLP settings from typed traces domain; removes smoke tests. |
| lib/saluki-components/src/common/otlp/traces/translator.rs | Adjusts trace translator constructor to take the two relevant booleans directly instead of a raw config struct. |
| lib/saluki-components/src/common/otlp/mod.rs | Adds shared grpc_max_recv_msg_size_bytes helper implementing the “0 => grpc default” rule. |
| lib/saluki-components/src/common/otlp/config.rs | Removes shared raw-map OTLP receiver config structs, keeping only the legacy traces config needed by the Datadog trace encoder for now. |
| lib/agent-data-plane-config/src/domains/traces.rs | Defines typed defaults and a concrete Default for OtlpTraces aligned with seeded vs schema-owned keys. |
| lib/agent-data-plane-config/src/domains/otlp.rs | Adds typed defaults and concrete Default impls for OTLP contexts/HTTP transport consistent with seeded keys. |
| lib/agent-data-plane-config-system/src/saluki_only.rs | Makes seeded OTLP knobs non-optional with serde defaults; supports ByteSize for interner-size keys; updates seeding logic and tests. |
| bin/agent-data-plane/src/cli/run.rs | Updates topology assembly to pull OTLP component configs from the resolved typed config model via ConfigurationSystem. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let context_string_interner_size = NonZeroUsize::new(config.context_string_interner_bytes as usize) | ||
| .ok_or_else(|| generic_error!("context_string_interner_size must be greater than 0"))?; |
| let traces_interner_size = std::num::NonZeroUsize::new(self.traces_string_interner_bytes as usize) | ||
| .ok_or_else(|| generic_error!("otlp_config.traces.string_interner_size must be greater than 0"))?; |
| let traces_interner_size = std::num::NonZeroUsize::new(self.traces_string_interner_bytes as usize) | ||
| .ok_or_else(|| generic_error!("otlp_config.traces.string_interner_size must be greater than 0"))?; |
| pub fn grpc_max_recv_msg_size_bytes(max_recv_msg_size_mib: u64) -> usize { | ||
| let mib = if max_recv_msg_size_mib == 0 { | ||
| OTLP_GRPC_DEFAULT_MAX_RECV_MSG_SIZE_MIB | ||
| } else { | ||
| max_recv_msg_size_mib | ||
| }; | ||
| (mib * 1024 * 1024) as usize | ||
| } |
Binary Size Analysis (Agent Data Plane)Baseline: 7b98c6f · Comparison: 80675f2 · 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
webern
left a comment
There was a problem hiding this comment.
Claude Code operating on behalf of webern. I found two default-value regressions in the typed cutover that should be fixed before merging. Both are reachable with an otherwise empty OTLP configuration and change the externally observable native OTLP pipeline behavior.
| pub fn from_configuration(otlp: &OtlpDomain, traces_otlp: &OtlpTraces) -> Result<Self, GenericError> { | ||
| Ok(Self { | ||
| metrics_enabled: otlp.receiver.metrics_enabled, | ||
| logs_enabled: otlp.receiver.logs_enabled, |
There was a problem hiding this comment.
P1: This changes the default for OTLP logs from enabled to disabled. Before this cutover, common::otlp::config::LogsConfig used default_logs_enabled() -> true; the resolved Datadog model's OtlpConfigLogs.enabled defaults to false, and this constructor now consumes that value. With no explicit otlp_config.logs.enabled, the native source will therefore stop accepting/dispatching OTLP logs. Please preserve the old effective default in the typed configuration/translation and add an empty-config regression assertion.
| metrics_enabled: otlp.receiver.metrics_enabled, | ||
| logs_enabled: otlp.receiver.logs_enabled, | ||
| traces_enabled: traces_otlp.enabled, | ||
| grpc_endpoint: otlp.receiver.grpc.endpoint.clone(), |
There was a problem hiding this comment.
P1: The default listener addresses regress from 0.0.0.0:4317 (gRPC) and 0.0.0.0:4318 (HTTP) in the old common::otlp::config structs to the generated Datadog schema defaults of localhost:4317 and localhost:4318. Since this constructor now consumes the resolved model, an otherwise empty config binds only loopback and remote OTLP clients can no longer connect. Please preserve the previous effective defaults in the typed config/translation (for both endpoints) and add an empty-config regression assertion.
webern
left a comment
webern
left a comment
There was a problem hiding this comment.
Claude Code operating on behalf of webern. I found two default-value regressions in the typed cutover that should be fixed before merging. Both are reachable with an otherwise empty OTLP configuration and change the externally observable native OTLP pipeline behavior.
| pub fn from_configuration(otlp: &OtlpDomain, traces_otlp: &OtlpTraces) -> Result<Self, GenericError> { | ||
| Ok(Self { | ||
| metrics_enabled: otlp.receiver.metrics_enabled, | ||
| logs_enabled: otlp.receiver.logs_enabled, |
There was a problem hiding this comment.
P1: This changes the default for OTLP logs from enabled to disabled. Before this cutover, common::otlp::config::LogsConfig used default_logs_enabled() -> true; the resolved Datadog model's OtlpConfigLogs.enabled defaults to false, and this constructor now consumes that value. With no explicit otlp_config.logs.enabled, the native source will therefore stop accepting/dispatching OTLP logs. Please preserve the old effective default in the typed configuration/translation and add an empty-config regression assertion.
| metrics_enabled: otlp.receiver.metrics_enabled, | ||
| logs_enabled: otlp.receiver.logs_enabled, | ||
| traces_enabled: traces_otlp.enabled, | ||
| grpc_endpoint: otlp.receiver.grpc.endpoint.clone(), |
There was a problem hiding this comment.
P1: The default listener addresses regress from 0.0.0.0:4317 (gRPC) and 0.0.0.0:4318 (HTTP) in the old common::otlp::config structs to the generated Datadog schema defaults of localhost:4317 and localhost:4318. Since this constructor now consumes the resolved model, an otherwise empty config binds only loopback and remote OTLP clients can no longer connect. Please preserve the previous effective defaults in the typed config/translation (for both endpoints) and add an empty-config regression assertion.
…over The OTLP receiver keys are witnessed against the vendored Datadog schema, whose defaults (localhost:4317/4318, logs disabled) diverge from ADP's historical effective defaults (0.0.0.0:4317/4318, logs enabled). After the typed cutover the components consumed the resolved model directly, so an empty config regressed to the Agent defaults: the native OTLP source stopped accepting logs and both native and proxy modes bound only loopback. Represent ADP's divergent default at the config boundary instead of as a component-local fallback: - Add `saluki_overrides_default` to the overlay model. When set, the config generator omits the schema `default` from the pruned leaf so typify emits an absence-aware `Option<T>` (the classifier still reads the real schema default from the schema map, so it is unaffected). - Flag `otlp_config.logs.enabled`, `otlp_config.receiver.protocols.grpc.endpoint`, and `...http.endpoint` in the overlay and regenerate. - The witness now hands these keys as `Option`; the translator applies the value only when present, leaving the typed model's `Default` (ADP's effective default) in place when the key is absent. An explicit value — including `logs.enabled: false` or an endpoint equal to the Agent default — still wins, and the env-var forms keep working. - Set the OTLP receiver model defaults to ADP's values via shared const/fn. Adds regression tests that translate an empty/raw config into the resolved typed model (empty -> logs enabled, 0.0.0.0:4317, 0.0.0.0:4318; explicit false/custom endpoints preserved; env var honored). The 0 => 4 MiB gRPC receive-size behavior is unchanged.
There's some weird stuff going on here with preserving "absence" in an attempt to preserve exact behavioral parity, but I think it's OK. Cuts the OTLP components over from the raw `GenericConfiguration` map to the typed `SalukiConfiguration` model, following the PR #1979 pattern: `OtlpConfig` (shared receiver config), the OTLP source, decoder, forwarder, and relay. Each component now builds from typed model slices (`domains.otlp`, `domains.traces.otlp`) instead of deserializing the raw config: - **Source / relay / decoder / forwarder**: `from_configuration` takes borrowed model slices; the call site in `run.rs` reads them from the config system. Component structs carry no serde and no defaults. - **Seeded OTLP knobs** (context sizing, HTTP receiver transport, trace interner size, top-level-by-span-kind) are converted to the required-with-default doctrine: non-optional in `SalukiOnly` with the default defined once as a shared `const`/`fn` in `agent-data-plane-config` and referenced by both the model `Default` and the `SalukiOnly` serde default. The two byte-size interner keys are typed `ByteSize` so they accept both a bare integer and a suffixed string. - **`OtlpTracesTranslator`** now takes the two native booleans it actually reads instead of the whole raw `TracesConfig`. `TracesConfig` is kept only for the not-yet-migrated Datadog trace encoder. - The per-component config smoke tests are retired (coverage now lives in the translator and construction tests). Because the OTLP receiver keys are witnessed against the vendored Datadog schema, their defaults now come from that schema rather than Saluki's former hardcoded values. When a key is left unset: - gRPC/HTTP receiver endpoints default to `localhost:4317` / `localhost:4318` (previously `0.0.0.0:...`), matching the Datadog Agent. - `otlp_config.logs.enabled` defaults to `false` (previously `true`), matching the Datadog Agent. - `otlp_config.receiver.protocols.grpc.max_recv_msg_size_mib` of `0` now maps to grpc-go's built-in 4 MiB limit, matching the Agent and the schema documentation (previously `0` produced a 0-byte limit). Deployments that set these keys explicitly (including the OTLP correctness cases, which pin the gRPC endpoint to `0.0.0.0:4317`) are unaffected. - [x] Non-functional (chore, refactoring, docs) - `make build-schema-overlay && make fmt` (no generated drift), `make check-all`, `make check-docs`. - Unit tests for `agent-data-plane-config`, `agent-data-plane-config-system`, `saluki-components`, and `agent-data-plane`, including new `SalukiOnly` transport/default tests (both byte-size input forms) and per-component construction tests. - Statically reviewed the OTLP correctness cases: all send over gRPC on the explicitly configured `0.0.0.0:4317`, so the receiver default changes above do not affect them. - Progresses #1788 - Merges into `m/pr5-cutover`
Human Summary
There's some weird stuff going on here with preserving "absence" in an attempt to preserve exact behavioral parity, but I think it's OK.
AI Summary
Cuts the OTLP components over from the raw
GenericConfigurationmap to the typedSalukiConfigurationmodel, following the PR #1979 pattern:OtlpConfig(shared receiverconfig), the OTLP source, decoder, forwarder, and relay.
Each component now builds from typed model slices (
domains.otlp,domains.traces.otlp)instead of deserializing the raw config:
from_configurationtakes borrowed model slices;the call site in
run.rsreads them from the config system. Component structs carry no serdeand no defaults.
top-level-by-span-kind) are converted to the required-with-default doctrine: non-optional in
SalukiOnlywith the default defined once as a sharedconst/fninagent-data-plane-configand referenced by both the model
Defaultand theSalukiOnlyserde default. The two byte-sizeinterner keys are typed
ByteSizeso they accept both a bare integer and a suffixed string.OtlpTracesTranslatornow takes the two native booleans it actually reads instead of thewhole raw
TracesConfig.TracesConfigis kept only for the not-yet-migrated Datadog traceencoder.
construction tests).
Behavior notes
Because the OTLP receiver keys are witnessed against the vendored Datadog schema, their defaults
now come from that schema rather than Saluki's former hardcoded values. When a key is left unset:
localhost:4317/localhost:4318(previously0.0.0.0:...), matching the Datadog Agent.otlp_config.logs.enableddefaults tofalse(previouslytrue), matching the Datadog Agent.otlp_config.receiver.protocols.grpc.max_recv_msg_size_mibof0now maps to grpc-go's built-in4 MiB limit, matching the Agent and the schema documentation (previously
0produced a 0-bytelimit).
Deployments that set these keys explicitly (including the OTLP correctness cases, which pin the
gRPC endpoint to
0.0.0.0:4317) are unaffected.Change Type
How did you test this PR?
make build-schema-overlay && make fmt(no generated drift),make check-all,make check-docs.agent-data-plane-config,agent-data-plane-config-system,saluki-components,and
agent-data-plane, including newSalukiOnlytransport/default tests (both byte-size inputforms) and per-component construction tests.
0.0.0.0:4317, so the receiver default changes above do not affect them.References
m/pr5-cutover