[ACTP] parse the par-control configuration from Go - #54590
Conversation
Go Package Import DifferencesBaseline: 296059f
|
|
🎯 Code Coverage (details) 🔗 Commit SHA: 6d10cfa | Docs | View more details | Give us feedback! |
Files inventory check summaryFile checks results against ancestor 296059ff: Results for datadog-agent_7.84.0~devel.git.449.6d10cfa.pipeline.132944455-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.449.6d10cfa.pipeline.132944455-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
23 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: c45db9e Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_memory | memory utilization | +0.42 | [+0.20, +0.64] | 1 | Logs |
| ➖ | quality_gate_logs | % cpu utilization | +0.21 | [-0.66, +1.09] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.04 | [+0.01, +0.08] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | -0.04 | [-0.09, +0.01] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.04 | [-0.08, -0.01] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.11 | [-0.15, -0.06] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.16 | [-0.24, -0.08] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.29 | [-0.51, -0.06] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | -0.29 | [-0.41, -0.17] | 1 | Logs bounds checks dashboard |
| ➖ | dsd_uds_10mb_3k_timestamped_contexts_cpu | % cpu utilization | -1.19 | [-1.48, -0.90] | 1 | Logs |
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 | 173.68MiB ≤ 178MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 744.81KiB ≤ 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 | 528.03MiB ≤ 538MiB | 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 | 17 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 210.01MiB ≤ 229MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 263.50MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 398.25 ≤ 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 | 408.92MiB ≤ 453MiB | 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.08MiB ≤ 76MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.78 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 325.24MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 76.59 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 301.71MiB ≤ 314MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 22.81 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 307.99MiB ≤ 343MiB | 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_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 memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, 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 total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_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_security_no_fs_load, bounds check cpu_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_mean_fs_load, 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_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 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 memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
73f61a5 to
12769e4
Compare
1eb7a41 to
0291af5
Compare
12769e4 to
9121269
Compare
0291af5 to
86f3e34
Compare
9121269 to
0f54bd3
Compare
86f3e34 to
e21877b
Compare
6c69553 to
94b705e
Compare
5bdf5d0 to
45ae99c
Compare
94b705e to
2f5ff72
Compare
45ae99c to
17a1a1c
Compare
2f5ff72 to
bb6b622
Compare
89db57e to
9a22acb
Compare
bb6b622 to
2aaf840
Compare
9a22acb to
d642df7
Compare
85d1a07 to
6c22069
Compare
06d94dd to
73a64c3
Compare
66e117b to
e323571
Compare
73a64c3 to
7d7201c
Compare
e323571 to
1a16146
Compare
3bf6f91 to
f83ca17
Compare
1a16146 to
6b010be
Compare
f83ca17 to
8800d1d
Compare
00a3402 to
ea55c73
Compare
8800d1d to
e5cbec0
Compare
ea55c73 to
690bb57
Compare
e5cbec0 to
c3e9fbb
Compare
690bb57 to
c2aaa60
Compare
c3e9fbb to
8e2bbae
Compare
c2aaa60 to
7677da0
Compare
c0227c4 to
d9b966c
Compare
01b23ab to
f821eba
Compare
d9b966c to
2ad5ede
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 314690cdcd
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
I think this should go under cmd/privateactionrunner-control ?
There was a problem hiding this comment.
Is that something new? I don't see that.
There was a problem hiding this comment.
I meant AFAIK all entrypoint are currently under cmd/ so I believe we might need to put ours there ?
There was a problem hiding this comment.
this seems really tedious to maintain. It would be good if agent runtime could provide some support. Maybe we could generate from https://github.com/DataDog/datadog-agent/blob/011be837fa5b36920ef14e713dfe667c02111d5b/pkg/config/schema/yaml/private_action_runner.yaml ?
There was a problem hiding this comment.
I reached out to @jose-manuel-almaza since he's working on the config layer as well. I would love to share some of that work.
| use anyhow::{Context, Result, bail}; | ||
| use std::path::Path; | ||
|
|
||
| pub const DEFAULT_IDENTITY_FILE_NAME: &str = "privateactionrunner_private_identity.json"; |
There was a problem hiding this comment.
AFAIK we want to start the executor on startup to ensure its working properly / do the enrollment if needed. I was thinking the executor could keep owning the full identity lifecycle and the control plane would not need to know how to retrieve it ?
A single "EnsureRunnerIdentity" method which returns the identity and handles all the cases
There was a problem hiding this comment.
par-control needs the identity because it's used by the OPMS client to sign requests.
I think your question is more whether Rust should get identity from the executor instead. A new EnsureRunnerIdentity RPC in the executor would centralize enrollment in Go, but it would require returning the private key over RPC and keeping the executor alive at boot for a non-execution purpose. Which seems less secure and more complex.
Personally I prefer the ensure-enrollment command approach because it keeps actual enrollment in Go and lets us just read a file in Rust to get identity.
There was a problem hiding this comment.
Discussed over slack, no strong opinion we can always revisit later
par-control now receives its configuration from `privateactionrunner bootstrap-par-control` instead of loading Agent configuration itself. It runs the command given by --bootstrap-command, reads the single PAR_CONTROL_CONFIG= prefixed JSON line, validates it at the trust boundary, and exits successfully when split mode is disabled. This deletes the Rust-owned configuration stack: local YAML loading, Fleet policy discovery and merging, environment decoding and aliases, precedence and deep merging, the Windows Fleet registry lookup, persisted identity discovery, inline versus persisted identity selection, the Agent IPC certificate path fallback, and site/dd_url normalization. Go already owns all of it, and the duplicate implementation is what let the two runners disagree. The org ID and runner ID now have one source of truth: Rust no longer re-derives them from the URN. Because par-control no longer reads datadog.yaml, it takes no --config, and the agent version comes from the bootstrap output rather than being injected at build time, so version.bzl is gone. Dropping the config loader also drops saluki-config, figment, windows-registry, and the Windows registry windows-sys features. The logger is initialized at Trace before bootstrap because its level filter is immutable afterward; the configured level is applied with set_max_level once bootstrap reports it. The configuration payload carries the runner private key and may carry proxy credentials, so it is suppressed from forwarded logs and never included in an error, including the malformed-JSON case where a serde error would otherwise quote it. Errors name the bootstrap executable only, never the full argv.
What does this PR do?
Makes
par-controla consumer of the configuration Go resolves, instead of anindependent configuration authority.
On startup it runs
privateactionrunner bootstrap-par-control(#54870), reads thesingle
PAR_CONTROL_CONFIG=prefixed JSON line from its stdout, validates it atthe trust boundary, and exits successfully when split mode is disabled.
This deletes the Rust-owned configuration stack:
aliases, configuration precedence and deep merging;
siteanddd_urlnormalization, and operational defaults already owned by theGo PAR config.
config.rsdrops from ~1,090 lines to a parser plus validation. The org ID andrunner ID now have exactly one source of truth: Rust no longer re-derives them from
the URN.
Because par-control no longer reads
datadog.yaml, it takes no--config, and theAgent version comes from the bootstrap output rather than being injected at build
time, so
version.bzlis gone. Dropping the loader also drops the directsaluki-config,figment, andwindows-registrydependencies —saluki-configand
figmentremain in the lockfile transitively viadatadog-agent-commons,which #54591 needs for executor IPC mTLS.
The logger is initialized at
Tracebefore bootstrap because its level filter isimmutable afterward; the configured level is applied with
log::set_max_leveloncebootstrap reports it.
The configuration payload carries the runner private key and may carry proxy
credentials, so it is suppressed from the forwarded bootstrap logs and never
included in an error — including the malformed-JSON case, where a serde error would
otherwise quote the input. Errors name the bootstrap executable only, never the
full argv.
Behavior changes
Using the canonical Go loader intentionally changes several previous Rust
behaviors:
ENC[...]values are now resolved through the Agent secret backend;environment aliases, precedence, proxy resolution and endpoint normalization match
the Go executor exactly; and executor-socket and IPC-certificate defaults match the
Agent's own path resolution. par-control still implements proxy transport through
reqwest, so exact transport-semantic equivalence with Go'shttpproxyis notimplied.
Validation
dda env dev run -- bazel test //pkg/privateactionrunner/par-control:par-control_testenv -u PKG_CONFIG_LIBDIR RUSTFLAGS='--cfg tokio_unstable' cargo test --manifest-path pkg/privateactionrunner/par-control/Cargo.tomlenv -u PKG_CONFIG_LIBDIR RUSTFLAGS='--cfg tokio_unstable' cargo clippy --manifest-path pkg/privateactionrunner/par-control/Cargo.toml --all-targets -- -D warnings(cd rust && cargo deny --manifest-path ../Cargo.toml check licenses)Stack 4 of 9; based on #54870 and followed by #55121.