feat(ndm): add Agent workload balancing - #54652
Conversation
Adds the comp/workloadbalancing skeleton that holds per-group active/standby state, so a network device can be handed from one Agent to another without restarting either one. The component tracks state per group rather than per Agent, which is the main difference from haagent. It also fails open: a group we hold no assignment for is Unmanaged and runs. Only an explicit standby suppresses a group, because duplicate monitoring during a handoff is recoverable and a silent device is not. Nothing consumes the component yet. The Remote Config listener, the check gating, and the metric follow in later PRs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0266106bcc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Set to true to enable Agent workload balancing. When enabled, the Agent only runs checks | ||
| for the workload balancing groups it has been assigned, allowing a device to be moved | ||
| between Agents without restarting either one. |
There was a problem hiding this comment.
Document unmanaged groups as still running
For users reading the generated public datadog.yaml, this says enabling workload balancing makes the Agent only run assigned groups. The component added in this commit deliberately treats a missing assignment as Unmanaged, and IsGroupActive returns true unless the state is Standby, so a group that is never assigned or later removed will still run. Please update this description, or change the state semantics, so the public config does not promise suppression of all unassigned groups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed in 2b694d7.
The description now says the Agent skips the groups assigned to another Agent, and spells out that a group this Agent holds no assignment for keeps running. That is deliberate: IsGroupActive only suppresses on an explicit standby, because a device nobody polls is a worse failure than a device two Agents poll during a handoff.
Note: This reply was written by Claude.
Adds the NDM_AGENT_WORKLOAD_BALANCING product and the update callback that applies its assignments. Each config names a group and the Agent that should actively poll it. Remote Config resends every config targeting this Agent on each update, so the set is authoritative: a group missing from it has lost its assignment and is dropped rather than left on standby. An empty update clears every group, which is the opposite of what the HA Agent does with one. HA stops running its checks when it loses its assignment; we keep running ours, because the failure that matters here is a device nobody is polling. The listener is only registered when agent_workload_balancing.enabled is true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Provides the component everywhere haagent is already provided, so anything that constructs a collector can depend on it. Twelve sites, matching haagent exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎯 Code Coverage (details) 🔗 Commit SHA: 507c1f7 | Docs | Datadog PR Page | Give us feedback! |
Files inventory check summaryFile checks results against ancestor 1d86ce75: Results for datadog-agent_7.84.0~devel.git.56.507c1f7.pipeline.130068453-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.56.507c1f7.pipeline.130068453-1_amd64.deb:No change detected |
Adds the group a check belongs to and the runner-side gate that acts on it. The SNMP instance setting agent_workload_balancing_group flows through InstanceConfig into CheckConfig and out via Check#WorkloadBalancingGroupID, a new method on the Check interface that defaults to the empty string for everything else. It is deliberately left out of DeviceDigest so moving a device between groups does not change its identity. The worker skips a check only when workload balancing is enabled, the check names a group, and that group is not active on this Agent. A check with no group, or a group we have never been assigned, always runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
10 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 1d86ce7 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_private_action_runner | memory utilization | +0.08 | [-0.04, +0.20] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.12 | [-0.15, -0.08] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.20 | [-0.24, -0.17] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | -0.40 | [-0.45, -0.34] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.68 | [-0.73, -0.64] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.87 | [-0.96, -0.77] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -1.30 | [-1.54, -1.06] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -3.44 | [-4.27, -2.62] | 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 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 148.25MiB ≤ 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 734.22KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 492.87MiB ≤ 512MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.13MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 17 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 184.02MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.16MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 384.39 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 375.93MiB ≤ 430MiB | 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 | 71.79MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 27.45 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 300.73MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 60.79 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 280.64MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.06 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 287.79MiB ≤ 320MiB | 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".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_idle | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_no_fs_load | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_no_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
CI Pass/Fail Decision
✅ Passed. All Quality Gates passed.
- quality_gate_metrics_logs, bounds check intake_connections: 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 missed_bytes: 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 intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 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 cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_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 memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 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 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 intake_connections: 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_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.
…onent @DataDog/ndm-core has no entry in the slack and jira notification channel maps, which fails slack_teams_channels_check. Use the team name the rest of NDM uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Go Package Import DifferencesBaseline: 1d86ce7
|
Three fixes from the automated review: - PythonCheck implements check.Check but lives behind the python build tag, so widening the interface broke every python-enabled build. It now returns an empty group. - An RC update we cannot parse used to drop its group back to unmanaged, which starts this Agent polling a device another Agent already holds. Keep the last assignment applied for that config path instead. A config that stops being sent is still dropped. - The agent_workload_balancing.enabled description in the public config promised the Agent only runs assigned groups. It skips groups assigned to another Agent; an unassigned group keeps running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…llector is built The first pass mirrored haagent's twelve fx sites, but haagent is provided in several of them for reasons unrelated to the collector, such as the haagent inventory metadata component. Only the collector component depends on workloadbalancing, so the module belongs in the three graphs that build it: the agent run subcommand, the cluster agent, and the cloudfoundry cluster agent. The other nine either use the noop collector or never construct one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
36b7545 to
2c455d6
Compare
…hes over The component and its Remote Config wiring add about 37 KiB on disk, and five gates had less buffer than that: agent_rpm_arm64 and agent_suse_arm64 were 9 KiB under their limit, and the three iot_agent amd64 gates 24 KiB under. Raise those five by enough to cover the whole stack, which lands at 735.55 MiB and 46.47 MiB with the metric and metadata PRs applied on top. On-wire sizes all still pass and are untouched.
|
Heads up for @DataDog/agent-build: The component and its Remote Config wiring add about 37 KiB, and these five had less buffer than that already: Happy to go through the exception process instead if you would rather the limits stayed put.
|
A group whose assignment named no Agent went to standby everywhere, because the leader comparison is exact and no hostname matches the empty string. Nothing validated active_agent, so a group the backend has created but not yet assigned would leave its device unpolled, which is the one outcome this component exists to avoid. An unnamed leader is now unmanaged, which runs the checks and still reports the group so the gap is visible rather than silent. The fallback for a config we cannot parse wrote its group unconditionally, in the same loop that applied the configs we can. Map iteration is unordered, so when one update carried both a fresh assignment for a group and a stale unparseable config for the same group, the stale one won about one time in ten. Apply the parseable configs first and let the fallback fill only the groups that pass left untouched. Alongside those: guard appliedConfigs with the mutex that already guards the group states rather than relying on the RC client staying serial, bound the group ID that reaches a metric tag, say so once when the group count is higher than expected, and name both hostnames when a group goes to standby so an assignment that means this host but spells it differently is diagnosable. SetGroupLeader and RemoveGroup had no callers outside the component's own tests now that Remote Config drives everything through setGroupLeaders, and a partial single-group write sits badly next to replace-the-whole-set semantics. Drop them from the interface. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closing to restart from scratch: the design RFC now has Workload Balancing reuse HA Agent's
|
What does this PR do?
Adds NDM Agent workload balancing: a device can be handed from one Agent to another without restarting either one.
The component (
comp/workloadbalancing)def/— theComponentinterface and theActive/Standby/Unmanagedstatesimpl/— anRWMutex-guarded map of group ID to state, theagent_workload_balancing.enabledconfig read, and the Remote Config update callbackfx/,mock/,helpers/— the usual component scaffoldingpkg/config/schema/yaml/core_schema.yaml— the newagent_workload_balancing.enabledsetting, defaultfalseRemote Config
New
NDM_AGENT_WORKLOAD_BALANCINGproduct inpkg/remoteconfig/state/products.go. Each config names a group and the Agent that should actively poll it. The listener is registered only when the setting is enabled.Where the group comes from
The SNMP instance setting
agent_workload_balancing_groupflows throughInstanceConfigintoCheckConfigand out viaCheck#WorkloadBalancingGroupID(), a new method on theCheckinterface that returns the empty string for everything else. It is deliberately left out ofDeviceDigest, so moving a device between groups does not change its identity.The gate
pkg/collector/workerskips a check only when workload balancing is enabled, the check names a group, and that group is not active on this Agent.Wiring
workloadbalancingfx.Module()at 12 binary sites; the component is threaded into the collector and the check runner.Motivation
First slice of the NDM Agent load balancing RFC. Today moving a device between Agents means editing config on both and restarting them, which leaves a monitoring gap.
The design follows
comp/haagentclosely, with two deliberate differences:Unmanagedand runs, and only an explicit standby suppresses. Duplicate monitoring during a handoff is recoverable; a silent device is not. For the same reason an empty Remote Config update clears every group rather than suppressing everything, and a group that drops out of the set returns to unmanaged.Describe how you validated your changes
go test -tags test ./comp/workloadbalancing/... ./pkg/collector/... ./comp/collector/...— passgo build ./...— cleandda inv linter.goover the touched packages — clean apart from a pre-existing cgo typecheck failure inpkg/collector/python, which this PR does not touchbazel run //:gazellefor the BUILD.bazel filesgo test -tags test ./cmd/agent/subcommands/... ./cmd/dogstatsd/subcommands/... ./pkg/cli/subcommands/...Unit tests cover: enabled/disabled config; the unmanaged-runs default; standby suppression;
RemoveGroup; group independence;GetGroupStatesreturning a copy; Remote Config apply/error states for valid, malformed, and partially valid update sets; and the worker gate across active, standby, unmanaged, and disabled.No manual validation yet — the Remote Config product still needs backend registration before an end-to-end handoff can be exercised.
Additional Notes
Still draft. Remaining work, in its own PRs: the
datadog.agent.workload_balancing.runningmetric, inventory metadata, and an e2e test (blocked on #53246 for Remote Config fakeintake support).