Skip to content

refactor(rc): wire libdatadog Remote Config client#222

Closed
iunanua wants to merge 3 commits into
mainfrom
igor/rc/libdatadog-rc-client-claude
Closed

refactor(rc): wire libdatadog Remote Config client#222
iunanua wants to merge 3 commits into
mainfrom
igor/rc/libdatadog-rc-client-claude

Conversation

@iunanua

@iunanua iunanua commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Bring up an end-to-end Remote Config client driven by datadog-remote-config from libdatadog, replacing the prior placeholder.
  • Adapt to the updated upstream API: typed ParserRegistry, RemoteConfigContent trait (with const PRODUCT and fn parse)

Currently tracks libdatadog branch igor/rc/di-refactor-claude. The [patch.crates-io] block is marked TODO and should be dropped once the matching libdatadog versions are published to crates.io.

Notes

It is registering a custom parser for ApmTracingConfig but it could use the builtin RC parser for DynamicConfigFile. In that case we'll need to update config and sampler methods and types related with trace_sampling_rules. I have decided not to change it after the sampler migration is in place.

🤖 Generated with Claude Code

iunanua and others added 3 commits May 8, 2026 16:22
…egistry

Adapt to the updated `datadog-remote-config` public API:

- Drop `ProductParser` and `std::any::Any` imports; add `RemoteConfigContent`.
- Replace the manual `RemoteConfigParsedData` impl on `ApmTracingConfig`
  (with `as_any` + `product`) with a single `RemoteConfigContent` impl
  exposing `const PRODUCT` and `fn parse(&[u8]) -> anyhow::Result<Self>`.
  The `RemoteConfigParsedData` marker is now provided by a blanket impl
  upstream.
- Remove `apm_tracing_parser()`; `build_fetcher` uses the typed builder
  `ParserRegistry::new().with::<ApmTracingConfig>()`.
- Update `Updated` alias to `anyhow::Result<Option<Box<dyn RemoteConfigParsedData>>>`
  to match `RegistryParser::Parsed`. `None` is the new "no parser registered"
  signal that replaces the old `IgnoredProduct` sentinel; `apply_file` and
  `apply_remove` early-return on `None` with the same no-op semantics.
- Update `test_apm_tracing_parser_round_trip` to call `ApmTracingConfig::parse`
  directly and assert on `ApmTracingConfig::PRODUCT`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Source every libdd-* crate (workspace-direct and transitive through
datadog-remote-config) from a single libdatadog git ref, removing the
libdd-common-rc renamed alias that previously bridged a v3/v4 split.

Tracks libdatadog branch igor/rc/di-refactor-claude. Adds direct
libdd-capabilities-impl dependency for NativeCapabilities and updates
call sites for the new TraceExporter<H> API and async
wait_agent_info_ready.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@iunanua iunanua changed the title feat(rc): wire libdatadog Remote Config client refactor(rc): wire libdatadog Remote Config client May 8, 2026
gh-worker-dd-mergequeue-cf854d Bot pushed a commit that referenced this pull request Jun 1, 2026
)

# What does this PR do?

Fixes adaptive-sampling Remote Config in `datadog-opentelemetry`, rebuilt on top of PR #154. Also adds `DD_TRACE_SAMPLE_RATE` support.

Before this PR, four things were broken:

1. **`tracing_sampling_rate` from RC was ignored.** The handler only acted on `tracing_sampling_rules`; a rate-only payload installed nothing.
2. **RC list-shape `tags` were rejected.** RC encodes `tags` as `[{"key": "env", "value_glob": "prod"}]`, but `libdd_sampling::SamplingRuleConfig::tags` only accepted the map shape, so the parse errored and the whole update was dropped.
3. **Env `DD_TRACE_SAMPLING_RULES` were wiped on every RC update.** `update_sampling_rules_from_remote` does a full override, so any RC change replaced env rules, even when RC only sent a global rate.
4. **`DD_TRACE_SAMPLE_RATE` had no effect.** No binding existed.

# Motivation

End-to-end adaptive sampling didn't work.

# What changed

**Composition.** `ApmTracingHandler::process_config` now follows the multi-source precedence model:

| env rules | env `DD_TRACE_SAMPLE_RATE` | RC `tracing_sampling_rules` | RC `tracing_sampling_rate` | Effective rule chain |
|---|---|---|---|---|
| present | any | absent / null | absent / null | `env_rules` |
| present | unset | absent / null | present | `env_rules + catch_all(rc_rate)` |
| present | set | absent / null | absent / null | `env_rules + catch_all(env_rate)` |
| present | set | absent / null | present | `env_rules + catch_all(rc_rate)` |
| any | any | non-empty array | absent / null | `rc_rules + catch_all(env_rate)` if env_rate set |
| any | any | non-empty array | present | `rc_rules + catch_all(rc_rate)` |

The synthetic catch-all uses libdatadog's default provenance, mapping to DM `-3` (LOCAL_USER). See the "legacy behavior" comment in [test_trace_sampling_rules_override_rate](https://github.com/DataDog/system-tests/blob/main/tests/parametric/test_dynamic_configuration.py#L872).

**`DD_TRACE_SAMPLE_RATE`.** New `Config::trace_sample_rate(): Option<f64>`. When set, the sampler installs an implicit catch-all so `DD_TRACE_RATE_LIMIT` applies. Unset means no catch-all (libdatadog's no-rule path samples at 100%).

**Tag normalization.** RC encodes `tags` as the list shape `[{"key", "value_glob"}]`. This is parsed natively by `libdd-sampling` ≥ 2.1.0 (DataDog/libdatadog#2033), so this PR bumps `libdd-sampling` 1.0.0 → 2.1.0 (pulling `libdd-common` → 4.2.0) and no in-tracer normalization is needed. An earlier revision carried a `normalize_rc_tags` shim for this; it has been removed now that the upstream release is available. Regression coverage: `test_handler_rc_rules_with_list_tags_applied` (list-shape tags apply, tags preserved as a map) and `test_handler_malformed_tags_rejects_update` (malformed list entries still rejected wholesale, not silently broadened).

**Fail-closed behavior.** When libdatadog rejects an update (malformed tags, out-of-range rate), `process_config` returns `Err` so the RC dispatcher reports `apply_state=3` and the prior policy survives. Out-of-range RC `tracing_sampling_rate` (outside `[0.0, 1.0]`) and non-numeric values are rejected up-front.

**Env/code rate validation.** `DD_TRACE_SAMPLE_RATE` (and the programmatic `set_trace_sample_rate`) get the same range check: only finite values in `[0.0, 1.0]` are honored; out-of-range values are logged and treated as unset rather than installed as a catch-all rule that libdatadog would clamp (negative ⇒ drop all, > 1.0 ⇒ keep all).

**Target check.** A config's `service_target` is honored before applying: a payload whose specific (non-`*`) `service`/`env` doesn't match this tracer — primary service or an advertised extra service, compared case-insensitively — is ignored, so a mistargeted RC delivery can never change this service's sampling. Mirrors dd-trace-py/go.

# Additional Notes

- Four parametric tests unblocked by this PR. Companion PR DataDog/system-tests#7007.
- Coordinated with @iunanua's PR #222 (libdatadog RC client wiring). #227 lands first; #222 rebases on top.



Co-authored-by: brian.marks <brian.marks@datadoghq.com>
@iunanua iunanua closed this Jun 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant