[ACTP] add par-control process lifecycle - #54589
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 4fdaf12 | Docs | Datadog PR Page | Give us feedback! |
embeaken
changed the title
Files inventory check summaryFile checks results against ancestor 933d9f15: Results for datadog-agent_7.84.0~devel.git.248.4fdaf12.pipeline.131425310-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.248.4fdaf12.pipeline.131425310-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates 33 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 6e12aad Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_security_idle | memory utilization | +0.30 | [+0.19, +0.41] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.26 | [+0.22, +0.30] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | +0.20 | [+0.07, +0.33] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | +0.12 | [+0.05, +0.19] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.17 | [-0.29, -0.05] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.21 | [-0.35, -0.06] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -1.82 | [-2.68, -0.95] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -2.39 | [-2.64, -2.15] | 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.30MiB ≤ 178MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 739.09KiB ≤ 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 | 520.08MiB ≤ 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 | 203.87MiB ≤ 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.82MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 407.17 ≤ 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 | 394.75MiB ≤ 439MiB | 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.54MiB ≤ 76MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.79 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 327.18MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 60.78 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 302.89MiB ≤ 314MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.58 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 313.43MiB ≤ 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_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_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, 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_metrics_logs, bounds check cpu_usage: 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 missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check total_bytes_received: 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_idle, bounds check total_bytes_received: 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_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_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 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_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
73f61a5 to
12769e4
Compare
12769e4 to
9121269
Compare
dd-octo-sts
Bot
added
internal
9121269 to
0f54bd3
Compare
6c69553 to
94b705e
Compare
50a6a7c to
5588f09
Compare
94b705e to
2f5ff72
Compare
5588f09 to
2731b4e
Compare
2aaf840 to
441cbb0
Compare
067d69a to
0eb011f
Compare
441cbb0 to
c8f01e1
Compare
85d1a07 to
6c22069
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
datadog-agent/pkg/privateactionrunner/par-control/BUILD.bazel
Lines 9 to 10 in 6c22069
Run buildifier on the modified BUILD file
This commit modifies BUILD.bazel, but its validation records Bazel build/test and Cargo clippy only, with no evidence that the repository-mandated bazel run //bazel/buildifier step was run. Run buildifier and commit any resulting formatting before merging.
AGENTS.md reference: AGENTS.md:L169-L169
ℹ️ 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".
6c22069 to
cd87d21
Compare
2d86206 to
ab23c0a
Compare
cd87d21 to
6ba350e
Compare
ab23c0a to
2a98a22
Compare
481cfaf to
9960c56
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 787ec32721
ℹ️ 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".
| fn read_fleet_policy(dir: &str) -> Result<Option<RawConfig>> { | ||
| if dir.is_empty() { | ||
| return Ok(None); | ||
| } | ||
| let path = Path::new(dir).join("datadog.yaml"); | ||
| let contents = match std::fs::read_to_string(&path) { | ||
| Ok(contents) => contents, | ||
| Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), | ||
| Err(error) => { | ||
| return Err(error) | ||
| .with_context(|| format!("failed to read fleet policy: {}", path.display())); | ||
| } | ||
| }; | ||
| serde_yaml::from_str(&contents) | ||
| .with_context(|| format!("failed to parse fleet policy: {}", path.display())) | ||
| .map(Some) | ||
| } |
There was a problem hiding this comment.
I'm confused as to calling it "fleet policies" ? AFAIK it as always been "agent configuration" ?
There was a problem hiding this comment.
Looked into it - "fleet policies" are a subset of "agent configuration".
Specifically, "fleet policies" refer to just config overrides from the Fleet Automation system, and "agent configuration" is the full resolved config after everything including fleet policies is resolved. This PR uses "fleet policy" correctly IMO because it treats it as its own layer of config resolution.
| let value = match raw.trim() { | ||
| "1" | "t" | "T" | "TRUE" | "true" | "True" => true, | ||
| "0" | "f" | "F" | "FALSE" | "false" | "False" => false, | ||
| _ => bail!("invalid boolean value for {name}: {raw:?}"), | ||
| }; |
There was a problem hiding this comment.
that seems really lenient ? Is this the same in the go codebase ?
There was a problem hiding this comment.
Looks like the type casting uses a fork of cast library here -
datadog-agent/pkg/config/basic/convert.go
Line 21 in 85b0ed7
The code for it handles a lot (GH downtime not letting me link to it...). Codex review actually complained about the behavior not matching Go when it was simpler.
Adds the initial `par-control` process lifecycle layer: - gates startup on PAR and split mode configuration, resolved in a single pass over `datadog.yaml` and the fleet policy overlay; - starts the on-demand executor through the shared `dd-procmgrd` client, relying on dd-procmgrd to serialize starts; - bounds every process-manager RPC through the shared client's channel instead of a hand-rolled timeout wrapper; - leaves stdout/stderr redirection to the dd-procmgrd process definition.
Treat an empty PAR boolean environment variable as unset so it falls back to YAML or the default instead of aborting startup, matching how the fleet dir and log level are already resolved. Correct the idle_timeout_seconds description: the executor now self-terminates after exactly this many seconds, not three times as many.
5 participants
What does this PR do?
Adds the initial
par-controlprocess lifecycle layer:dd-procmgrdclientidle_timeout_secondsauthoritative instead of scaling it behind a control-plane stop.Validation
dda env dev run -- bazel test //pkg/privateactionrunner/par-control:par-control_test //pkg/privateactionrunner/executor:executor_test //comp/privateactionrunner/impl:impl_test //pkg/procmgr/rust:dd-procmgrd_testdda env dev run -- env -u PKG_CONFIG_LIBDIR cargo test -p par-controldda env dev run -- env -u PKG_CONFIG_LIBDIR cargo clippy -p par-control -p dd-procmgr-client --all-targets -- -D warnings