Support additional Cluster Agent Remote Configuration clients - #55618
Support additional Cluster Agent Remote Configuration clients#55618clamoriniere wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
AI review by Codex (OpenAI) - workflow run
Patch is incorrect: additional Remote Config instances are collected but omitted from human-readable status output.
There was a problem hiding this comment.
AI review by Codex (OpenAI) - workflow run
Patch is incorrect: the new status endpoint sanitization can still expose credentials embedded in URL query parameters or fragments.
There was a problem hiding this comment.
An additional client starts before its subsystem adds the subscription. Cached configuration can arrive during this gap, and later unchanged polls do not send it again.
🤖 Datadog Autotest · Commit 1b6506f · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
🎯 Code Coverage (details) 🔗 Commit SHA: 9322eb4 | Docs | View more details | Give us feedback! |
Files inventory check summaryFile checks results against ancestor eb7754dc: Results for datadog-agent_7.84.0~devel.git.591.9322eb4.pipeline.134080467-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.591.9322eb4.pipeline.134080467-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
|
The service kept its status in package-level expvars, so every instance in a process wrote to the same three fields and the last one to poll won. It also always followed runtime updates to "api_key", which would replace the key of an instance given its own. Give each instance its own status entry, keyed by a name the caller chooses, and record the endpoint it polls so instances can be told apart. The default client keeps reporting under the same top-level fields, so nothing changes for a process with a single service. Add WithoutAPIKeyUpdates for instances whose key is static. Export DefaultStatusInstance and DefaultDatabaseFileName so callers that need to keep instances distinct compare against these rather than repeating the literals. Assisted-by: Claude:claude-opus-5
The Cluster Agent assumed a single Remote Configuration client per process, so endpoint and API key ownership was process wide and every subsystem polled the same backend. Add cluster_agent.remote_configuration.additional_clients: a client is named after the subsystem it serves, which selects the Remote Configuration products it owns, and carries its own endpoint, API key, TUF roots and cache database. Products no client claims stay on the default client and the default cache, so existing deployments are unaffected. Clients are built lazily, only once a product they own is requested, so configuring one costs nothing until the subsystem is enabled. Configuration that cannot work is rejected at startup rather than degrading quietly: an unknown client name, a cache filename that is not a basename or that collides with another client's, and products from one subsystem split across two clients. The setting is not published in the generated documentation. It exists for testing the Cluster Agent against a second backend, not for customers. Assisted-by: Claude:claude-opus-5
An additional client was started as soon as it was built, before the caller had a chance to subscribe to its products. The client notifies only the listeners registered when an update arrives and never replays: it returns early when no product changed, and calls listeners only for products that did. A client that polls before its subsystem subscribes therefore consumes the current configuration into its state and never delivers it, leaving the subsystem without configuration until the backend happens to change something. This is reachable on any restart, where the cache already holds configuration. Build the client stopped and start it from StartClients once every subsystem has resolved its client and subscribed, which is how the default client is already handled: subscribeAgentConfig and subscribeAgentTask run before defaultRCClient.Start(). Also keep only scheme, host and path of the endpoint published to the status. Clearing userinfo alone left credentials passed in a query string or fragment, and that value reaches flares and support tickets. Assisted-by: Claude:claude-opus-5
processLevelRemoteConfigProducts had no production reader: additional clients take their products from curated presets, so the rule is asserted against those presets in a test rather than enforced at runtime. Production files in this package build on darwin but the tests do not, so a var defined in production and read only from a test reads as unused there. That is why only the macOS lint job failed. Move it next to the assertion that uses it. Assisted-by: Claude:claude-opus-5
ff7fd58 to
9322eb4
Compare
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: eb7754d Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_logs | % cpu utilization | +0.95 | [+0.07, +1.84] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +0.55 | [+0.51, +0.60] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | +0.37 | [+0.25, +0.49] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | +0.33 | [+0.25, +0.40] | 1 | Logs bounds checks dashboard |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_cpu | % cpu utilization | +0.18 | [-0.06, +0.42] | 1 | Logs |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_memory | memory utilization | +0.17 | [-0.05, +0.38] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +0.10 | [+0.06, +0.14] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.19 | [-0.23, -0.15] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.24 | [-0.28, -0.21] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.93 | [-1.16, -0.70] | 1 | Logs bounds checks dashboard |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 172.80MiB ≤ 179MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 746.69KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 515.71MiB ≤ 537MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.14MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 210.56MiB ≤ 220MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.42MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 365.68 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 19 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 433.92MiB ≤ 455MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.94GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 72.23MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.32 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 325.19MiB ≤ 355MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 60.48 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 306.06MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.43 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 313.17MiB ≤ 345MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 10/10 replicas passed. Gate passed.
What does this PR do?
Allows the Cluster Agent to run several Remote Configuration clients in one process, each with its own endpoint, API key, TUF roots and cache database.
An additional client is named after the subsystem it serves, which selects the RC products it owns —
autoscaling,kubeactions,private_action_runnerorapm_instrumentation:Products no client claims stay on the default client and the default cache, so existing deployments are unaffected. Clients are built lazily, only once a product they own is requested.
The setting is not published in the generated
datadog.yamldocumentation: it exists for testing the Cluster Agent against a second backend, not for customer configuration.Motivation
The Cluster Agent assumed a single Remote Configuration client per process, which made endpoint and API key ownership process wide: every service watched the top-level
api_key, status expvars were shared, and instances would have reused the same cache database.Running autoscaling against a different Remote Configuration backend than the rest of the Cluster Agent requires those resources to be per-instance.
Describe how you validated your changes
Unit tests covering spec parsing and validation, product routing, lazy client creation, per-instance status and API key isolation.
Also validated end to end on a local kind cluster, with the autoscaling client pointed at a second Remote Configuration backend while the default client stayed on production: both clients authenticated independently, kept separate cache databases, and the autoscaling client received live configuration.
Additional Notes
Configuration that cannot work is rejected at startup rather than degrading quietly: an unknown client name, a cache filename that is not a basename or that collides with another client's, and products used by one subsystem split across two clients.
Stacked below #55619, which reports these clients in
datadog-cluster-agent status.🤖 PR description and code assisted by Claude:claude-opus-5