refactor(config): replace bottlecap Config with upstream Config<LambdaConfig> by duncanista · Pull Request #1251 · DataDog/datadog-lambda-extension

duncanista · 2026-06-10T18:06:41Z

Overview

Stacked on top of #1249. Completes the migration to the shared datadog-agent-config crate: bottlecap::config::Config is now a type alias for datadog_agent_config::Config<LambdaConfig>, Lambda-specific fields live under .ext, and the 10 in-tree config submodules that duplicated upstream implementations are removed.

Structural changes

bottlecap/src/config/mod.rs shrinks from 2243 → 602 lines. The legacy Config struct, ConfigBuilder, ConfigSource, ConfigError, the #[macro_export] merge_* macros, all per-field deserializer helpers, and the test module that mirrored upstream's behavior — all gone. What remains: a type Config alias, a get_config(path) wrapper, the LambdaConfig extension itself (from the parent PR), and re-exports of upstream modules under the same crate::config::* paths so existing imports keep working.
New module bottlecap/src/config/propagation_wrapper.rs holds a newtype PropConfig(Arc<Config>) so we can implement the foreign PropagationConfig trait on the foreign Config<E> type without tripping Rust's orphan rule. The wrapper is scoped to the dd-trace-rs propagator boundary — nothing else uses it.
bottlecap/src/traces/propagation/mod.rs wraps the inner propagator in PropConfig instead of passing Config directly. All call sites that previously handed Arc<Config> to the propagator are unchanged — the wrapping happens inside DatadogCompositePropagator::new.
Deleted files (each redundant with upstream):
- additional_endpoints.rs, apm_replace_rule.rs, env.rs, flush_strategy.rs, log_level.rs, logs_additional_endpoints.rs, processing_rule.rs, service_mapping.rs, trace_propagation_style.rs, yaml.rs
~70 field-access sites referencing Lambda-specific fields (config.api_key_secret_arn, config.serverless_logs_enabled, config.enhanced_metrics, etc.) updated to read through config.ext.X. Test struct literals that constructed Config { ... } with Lambda fields now nest them under ext: LambdaConfig { ..., ..Default::default() }.

What stays the same

LambdaConfig itself (and its 33 tests) — already shipped in the parent PR; no behavior change in this commit.
All other tests pass: 501 lib + 5 integration tests green.
The .ext indirection is invisible to callers that hold an Arc<Config> thanks to Rust's field-access auto-deref — they just go from config.foo to config.ext.foo for the 19 Lambda fields.

Testing

$ cargo test --lib
test result: ok. 501 passed; 0 failed; 0 ignored

$ cargo test --tests
test result: ok. (all integration suites green)

Follow-ups

Re-target this PR's base from feat/use-datadog-agent-config-crate to main once feat(config): introduce extension config on top of shared datadog-agent-config #1249 merges.
Optional: contribute custom_metrics_exclude_tags upstream so it becomes part of core Config rather than living in the Lambda extension (discussed in feat(config): introduce extension config on top of shared datadog-agent-config #1249).

Introduces a `LambdaExtension` struct that implements the upstream `ConfigExtension` trait from `datadog-agent-config`, plus a `LambdaSource` deserialization shape used for both env-var and YAML loading via figment's dual-extraction. This is the first step of migrating bottlecap's in-tree config module onto the shared serverless-components `datadog-agent-config` crate. The extension carries the 19 Lambda-specific fields with no upstream equivalent (api_key_secret_arn, kms_api_key, api_key_ssm_arn, serverless_logs_enabled, serverless_flush_strategy, enhanced_metrics, lambda_proc_enhanced_metrics, capture_lambda_payload, capture_lambda_payload_max_depth, compute_trace_stats_on_extension, span_dedup_timeout, api_key_secret_reload_interval, dd_org_uuid, serverless_appsec_enabled, appsec_rules, appsec_waf_timeout, api_security_enabled, api_security_sample_delay, custom_metrics_exclude_tags). Behavior preserved end-to-end with 33 tests covering each field from both DD_* env vars and datadog.yaml, plus: - DD_LOGS_ENABLED <-> DD_SERVERLESS_LOGS_ENABLED OR-merge - FlushStrategy ("end", "periodically,N", invalid -> Default) - Duration parsing (seconds, microseconds, ignore-zero) - org_uuid -> dd_org_uuid and lambda_customer_metrics_exclude_tags -> custom_metrics_exclude_tags source-to-config field mappings - env precedence over YAML - Forgiving fallback when a single field is malformed The dep is pinned to the pre-merge SHA of DataDog/serverless-components#135 (libdatadog rev alignment) for development; will be re-pinned to the merged SHA before opening the migration PR. Follow-ups (in subsequent commits): - Replace bottlecap::config::Config with datadog_agent_config::Config<LambdaExtension> - Delete duplicated env.rs / yaml.rs / deserializer modules

DataDog/serverless-components#135 merged at aaac1a5d63faf664750a3bf51b50b79449a31625. The previous pin was the pre-merge branch SHA used during development.

Datadog-agent-config transitively pulls dogstatsd@aaac1a5d (uses parse_metric_namespace from it); bottlecap's existing dogstatsd pin was still at 5b68f50f. Cargo treated the two as separate packages, which broke the FIPS clippy job — the new transitive copy didn't inherit bottlecap's rustls/fips feature plumbing and ring ended up in the dependency graph. Bumping dogstatsd and datadog-fips to the same SHA collapses the duplicate, restoring a clean FIPS dependency tree.

… mod.rs `LambdaExtension` collides nominally with "the Datadog Lambda Extension" (this entire project). `LambdaConfig` reads more naturally for the extension type that holds Lambda-specific configuration fields. While here, fold the standalone lambda_extension.rs into config/mod.rs so consumers don't need a separate module hop. The inlined code uses fully-qualified `datadog_agent_config::merge_fields!` / `datadog_agent_config::merge_string!` invocations to coexist with the legacy `#[macro_export]` macros still at the top of mod.rs — both go away together once the migration onto the upstream Config lands. Renames carried through: LambdaExtension -> LambdaConfig LambdaSource -> LambdaConfigSource mod lambda_extension -> (gone; inlined) All 33 LambdaConfig tests still pass.

Bumps datadog-agent-config to the upstream PR branch SHA carrying DataDog/serverless-components#136 (which switches the libdd deps to default-features = false and exposes new https/fips features), and plumbs them through bottlecap's default/fips feature spec so the right TLS provider is selected per build. Without this, datadog-agent-config's libdd transitive dependencies implicitly enabled `https` (ring-backed hyper-rustls) on top of the fips path that bottlecap activates, leaving both providers in the dependency graph. The datadog-fips build script then rejected the build because ring v0.17 was reachable via: ring v0.17 -> libdd-common feature "https" -> hyper-rustls feature "ring" -> rustls-webpki feature "ring" With agent-config now opting in via consumer features instead of defaults, the FIPS dep tree is clean again. Also regenerated LICENSE-3rdparty.csv to include the new datadog-agent-config package, per the dd-rust-license-tool check. TODO: re-pin to the merge SHA once DataDog/serverless-components#136 lands.

Picks up the doc-comment clarification from the copilot review on DataDog/serverless-components#136 (https/fips features aren't Cargo-enforced).

DataDog/serverless-components#136 merged at bb4dedeee20b949db3143c05e5a779b843a8a484. The previous pin was the pre-merge branch SHA used during development.

Three real fixes from independent review: 1. **Cargo.lock dedup** — bump dogstatsd and datadog-fips from `aaac1a5d` to `bb4dedee` so they share the same serverless-components SHA as datadog-agent-config. Otherwise cargo resolves two distinct `dogstatsd` source entries (the dep transitively from agent-config pulled in a second copy at the newer rev). Source trees for both crates are byte-identical between the two SHAs; only #136 changed between them. PR description matches reality now. 2. **`capture_lambda_payload_max_depth` graceful deserializer** — the field was missing `#[serde(deserialize_with = ...)]`, violating the upstream `ConfigExtension::Source` contract. Without it, a malformed env value would silently reset *all* extension fields to their defaults (api_key_secret_arn, kms_api_key, …) and emit only a `tracing::warn!`. Adds `deser_opt_lossless` which returns `None` on bad input so the rest of the extension keeps its values. 3. **Test coverage gaps**: - YAML round-trip for the two source-to-config renames (`org_uuid → dd_org_uuid` and `lambda_customer_metrics_exclude_tags → custom_metrics_exclude_tags`). These were the exact cases where YAML behavior matters most and they only had env tests. - `FlushStrategy::EndPeriodically` (`"end,N"`) and `FlushStrategy::Continuously` (`"continuously,N"`) — the upstream enum has all four variants and bottlecap will rely on them behaviorally; previously only `End` and `Periodically` had coverage. Test count: 33 → 37.

duncanista

Reviewed the migration diff (feat/use-datadog-agent-config-crate...feat/migrate-bottlecap-to-upstream-config). This is a mechanical refactor — Config → Config<LambdaConfig> with .ext. indirection on 19 Lambda fields, plus a PropConfig newtype to ferry the foreign Config<E> past the orphan rule. No correctness bugs found. The structural pieces (PropConfig impl, mod.rs rewrite, ~70 read-site rewrites, ~14 test struct-literal rewrites) all check out.

What I verified

Field migration completeness. Grepped every one of the 19 Lambda field names across bottlecap/src/ and bottlecap/tests/. The only remaining bare config.<lambda_field> (no .ext.) accesses are inside LambdaConfig/LambdaConfigSource struct literals or comments. Nothing missed.
Test struct-literal rewrites. Every Config { lambda_field: …, ..Default::default() } is correctly rewritten to nest the Lambda fields under ext: LambdaConfig { …, ..Default::default() } with the outer ..Config::default() (or ..no_config.as_ref().clone()) preserved. Test intent is preserved in the two cases where a Lambda field was bundled with core fields (e.g. lifecycle/invocation/processor.rs test at the new line 2407 has service: Some("test-service".to_string()) at the top level AND serverless_appsec_enabled: true correctly placed under ext:).
metrics/enhanced/lambda.rs test_disabled / test_snapstart_restore_duration_metric_disabled. These override enhanced_metrics over a non-default no_config base. The rewrite correctly uses ..no_config.ext.clone() for the extension and ..no_config.as_ref().clone() for the rest — both layers of "keep everything else from no_config" are preserved.
mod.rs rewrite (2243 → 602 lines). Verified that nothing outside the deleted files references the dropped ConfigBuilder / ConfigSource / ConfigError / merge_* macros. The re-exports cover every existing crate::config::{env, flush_strategy, processing_rule, log_level, …}::X import I could find (lifecycle/invocation_times.rs, lifecycle/flush_control.rs, logs/processor.rs, etc.).
PropConfig impl parity. The new PropagationConfig for PropConfig impl is a verbatim forward of the old impl PropagationConfig for Config (same emptiness checks, same None for inject, same hard-coded 512 for datadog_tags_max_length). The self.0.foo derefs through Arc<Config> to the same fields as the prior self.foo access.
Stacked-PR base. Re-targeting onto main after #1249 lands should be clean — this PR's diff touches only files modified in this branch's HEAD commit (e5cebae0), and #1249 doesn't touch the same lines in those files.

Findings are non-blocking nits — see inline comments.

Three non-blocking nits from the independent review: 1. PropConfig's inner Arc<Config> is no longer pub — keeps the wrapper boundary tight; the new() constructor is sufficient and no callers outside the module reach for the inner field directly. 2. PropConfig::new returns Arc<Self> instead of Self. Drops the redundant `Arc::new(...)` wrap at the single call site in traces/propagation/mod.rs. 3. Documents the hard-coded 512 in datadog_tags_max_length — it matches dd-trace-rs's default; bottlecap intentionally doesn't expose DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH. Saves the next reader a round-trip through upstream to confirm parity. No behavior change. All 505 lib tests still pass.

Per Copilot review on #1249. The existing `tests` module above already has `#[cfg_attr(coverage_nightly, coverage(off))]` to keep test code out of coverage metrics on nightly coverage builds; the new `lambda_config_tests` module should match.

…aConfig> Completes the migration started in the previous PR onto the shared datadog-agent-config crate. bottlecap::config::Config is now a type alias for datadog_agent_config::Config<LambdaConfig>; Lambda-specific fields live under .ext (per the upstream ConfigExtension trait), and the 10 in-tree config submodules that duplicated upstream implementations are removed entirely. What changes structurally: - bottlecap/src/config/mod.rs shrinks from 2243 lines to ~600. The legacy Config struct, ConfigBuilder, ConfigSource, ConfigError, the #[macro_export] merge_* macros, all the per-field deserializer helpers, and the entire test module that mirrored upstream's behavior are gone. What remains: a `type Config` alias, a `get_config(path)` wrapper, the LambdaConfig extension itself, and re-exports of upstream modules under the same `crate::config::*` paths so existing imports keep working. - New module bottlecap/src/config/propagation_wrapper.rs holds a newtype `PropConfig(Arc<Config>)` so we can implement the foreign PropagationConfig trait on the foreign Config<E> type without running afoul of Rust's orphan rule. The wrapper is scoped to the dd-trace-rs propagator boundary; nothing else uses it. - bottlecap/src/traces/propagation/mod.rs wraps the inner propagator in PropConfig instead of passing Config directly. All call sites that previously handed `Arc<Config>` to the propagator are unchanged - the wrapping happens inside DatadogCompositePropagator::new. - Deleted files (each redundant with upstream): additional_endpoints.rs, apm_replace_rule.rs, env.rs, flush_strategy.rs, log_level.rs, logs_additional_endpoints.rs, processing_rule.rs, service_mapping.rs, trace_propagation_style.rs, yaml.rs - All ~70 field-access sites referencing Lambda-specific fields (config.api_key_secret_arn, config.serverless_logs_enabled, config.enhanced_metrics, etc.) updated to read through config.ext.X. Test struct literals that construct Config { ... } with Lambda fields now nest them under `ext: LambdaConfig { ..., ..Default::default() }`. What stays the same: - LambdaConfig itself (and its 33 tests) - already shipped in the parent PR; no behavior change in this commit. - All other tests pass: 501 lib + 5 integration tests green. - The .ext indirection is invisible to callers that hold an `Arc<Config>` thanks to Rust's field-access auto-deref - they just go from `config.foo` to `config.ext.foo` for the 19 Lambda fields. Stacked on top of the LambdaConfig foundation PR (#1249).

Three non-blocking nits from the independent review: 1. PropConfig's inner Arc<Config> is no longer pub — keeps the wrapper boundary tight; the new() constructor is sufficient and no callers outside the module reach for the inner field directly. 2. PropConfig::new returns Arc<Self> instead of Self. Drops the redundant `Arc::new(...)` wrap at the single call site in traces/propagation/mod.rs. 3. Documents the hard-coded 512 in datadog_tags_max_length — it matches dd-trace-rs's default; bottlecap intentionally doesn't expose DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH. Saves the next reader a round-trip through upstream to confirm parity. No behavior change. All 505 lib tests still pass.

datadog-prod-us1-4 · 2026-06-10T18:56:21Z

✨ Fix all issues with BitsAI

⚠️ Warnings

🚦 4 Pipeline jobs failed

DataDog/datadog-lambda-extension | e2e-test-status (amd64)

DataDog/datadog-lambda-extension | e2e-test-status (amd64, fips)

DataDog/datadog-lambda-extension | integration-suite: [auth]

View all 4 failed jobs.

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: a223aee | Docs | Datadog PR Page | Give us feedback!}

litianningdatadog · 2026-06-17T21:05:38Z

+    #[must_use]
+    pub fn new(config: Arc<Config>) -> Arc<Self> {
+        Arc::new(Self(config))
+    }


nit: new() usually returns Self, so returning Arc::new() feels a bit counterintuitive or I missed something?

litianningdatadog

Approved with nit comment

jchrostek-dd · 2026-06-18T13:44:11Z

Can you add some notes in the description about why we want to do this.

duncanista added 7 commits June 10, 2026 11:42

chore(deps): re-pin datadog-agent-config to merged SHA

ade85e7

DataDog/serverless-components#135 merged at aaac1a5d63faf664750a3bf51b50b79449a31625. The previous pin was the pre-merge branch SHA used during development.

chore(deps): re-pin datadog-agent-config to address PR #136 review

ec0d1e3

Picks up the doc-comment clarification from the copilot review on DataDog/serverless-components#136 (https/fips features aren't Cargo-enforced).

chore(deps): re-pin datadog-agent-config to #136 merge SHA

f6abcb4

DataDog/serverless-components#136 merged at bb4dedeee20b949db3143c05e5a779b843a8a484. The previous pin was the pre-merge branch SHA used during development.

duncanista mentioned this pull request Jun 10, 2026

feat(config): introduce extension config on top of shared datadog-agent-config #1249

Open

duncanista commented Jun 10, 2026

View reviewed changes

Comment thread bottlecap/src/config/propagation_wrapper.rs Outdated

Comment thread bottlecap/src/config/propagation_wrapper.rs

Comment thread bottlecap/src/traces/propagation/mod.rs Outdated

duncanista force-pushed the feat/migrate-bottlecap-to-upstream-config branch from e5cebae to 561096a Compare June 10, 2026 18:27

duncanista marked this pull request as ready for review June 10, 2026 18:28

duncanista requested review from a team as code owners June 10, 2026 18:28

duncanista requested a review from jchrostek-dd June 10, 2026 18:28

duncanista added 3 commits June 10, 2026 14:34

duncanista force-pushed the feat/migrate-bottlecap-to-upstream-config branch from 561096a to a223aee Compare June 10, 2026 18:35

litianningdatadog reviewed Jun 17, 2026

View reviewed changes

litianningdatadog approved these changes Jun 17, 2026

View reviewed changes

duncanista force-pushed the feat/use-datadog-agent-config-crate branch from f636493 to 2686077 Compare June 18, 2026 17:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(config): replace bottlecap Config with upstream Config<LambdaConfig>#1251

refactor(config): replace bottlecap Config with upstream Config<LambdaConfig>#1251
duncanista wants to merge 11 commits into
feat/use-datadog-agent-config-cratefrom
feat/migrate-bottlecap-to-upstream-config

duncanista commented Jun 10, 2026

Uh oh!

duncanista left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datadog-prod-us1-4 Bot commented Jun 10, 2026 •

edited

Loading

Uh oh!

litianningdatadog Jun 17, 2026

Uh oh!

litianningdatadog left a comment

Uh oh!

jchrostek-dd commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

duncanista commented Jun 10, 2026

Overview

Structural changes

What stays the same

Testing

Follow-ups

Uh oh!

duncanista left a comment

Choose a reason for hiding this comment

What I verified

Uh oh!

Uh oh!

Uh oh!

Uh oh!

datadog-prod-us1-4 Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

Uh oh!

litianningdatadog Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

litianningdatadog left a comment

Choose a reason for hiding this comment

Uh oh!

jchrostek-dd commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

datadog-prod-us1-4 Bot commented Jun 10, 2026 •

edited

Loading