[procmgr] Windows spawn profiles foundation - #54731
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b27edf4195
ℹ️ 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".
|
🎯 Code Coverage (details) 🔗 Commit SHA: 5de957c | Docs | Datadog PR Page | Give us feedback! |
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 375b62e Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_logs | % cpu utilization | +2.15 | [+1.28, +3.03] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_private_action_runner | memory utilization | +0.43 | [+0.30, +0.55] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | +0.03 | [-0.04, +0.10] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | -0.02 | [-0.13, +0.10] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.12 | [-0.16, -0.08] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.13 | [-0.27, +0.00] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | -0.21 | [-0.33, -0.10] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -1.25 | [-1.50, -1.00] | 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 | 171.48MiB ≤ 178MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 742.51KiB ≤ 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 | 510.70MiB ≤ 538MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.12MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 202.55MiB ≤ 229MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.26MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 358.93 ≤ 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 | 389.77MiB ≤ 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 | 72.96MiB ≤ 76MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 28.71 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 330.27MiB ≤ 335MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 60.47 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 300.83MiB ≤ 314MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 21.07 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 312.06MiB ≤ 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_idle, bounds check total_bytes_received: 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_metrics_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 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 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_logs, bounds check missed_bytes: 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_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_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_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 memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bef4144e47
ℹ️ 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".
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
18 successful checks with minimal change (< 2 KiB)
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dcf3ac4fb
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e2715b142
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcc2e35663
ℹ️ 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".
Deduplicate spawn and exit-watcher wiring across manual start, boot, restart, and reload paths without changing when config gates are recorded.
Terminate the suspended child and bail instead of resuming without a supervision job, so force-stop and profile cleanup stay correct.
Move had_successful_run onto ManagedProcess so set_config can reset RestartTracker without dropping post-success retry classification, and add tests for reload plus failed respawn retry chains.
Make the side-effecting restart commit explicit and avoid colliding with ProcessConfig::restart_delay(), which returns the configured delay.
Return the deferred PID from mark_stopped on Windows so callers start background job drain even when wait_for_stop falls back without a status.
Share name-based index lookup between handle_create duplicate checks and resolve_index without changing lookup semantics.
Drop module and API docs that restate obvious behavior; keep short notes where Win32 spawn/auth semantics are non-obvious.
When reload queues a replacement retry at a new config generation, drop stale auto-start timers so parallel respawns do not exhaust start_limit_burst.
Treat config-changed reloads like a fresh PM start: reset counters on set_config, start running/failed auto-start processes under boot gates, and discard all pending crash timers from before the reload.
On job-assignment or ResumeThread failure, terminate via the retained process handle, block until exit is observed, then clear spawn resources so UnloadUserProfile does not run while the child still exists.
Align restart-policy log assertion with the current message format and resolve rustfmt/clippy failures on the spawn-profiles branch.
Replace request_stop/wait_for_stop with a single stop() that sends graceful shutdown, force-kills on timeout, and synchronously drains Windows job members before unloading profiles. Use ProcessState::Stopping instead of stop_requested and remove deferred job-drain machinery from the manager.
Restructure stop() with an inline state match, extract wait_for_stop_exit and shared force-kill helpers, and add debug timing logs for stop duration.
Use proc_handle and watcher_handle so the ProcessHandle and JoinHandle are not both named handle.
Skip exit wait when TerminateProcess or job terminate fails, and cap the wait at FORCE_KILL_TIMEOUT so spawn abort cannot block startup or reload under the manager write lock.
These helpers are only referenced by agent-profile tests landing in the stacked list/describe PR; keeping them here fails golangci-lint unused.
Extract remove, apply, and reconcile reload phases into ProcessManager methods so handle_reload_config reads as a short orchestration step. Also drop an unused hyper_util import from grpc integration tests.
Clone the process name and capture pid before abort_before_supervision consumes SuspendedChild, so clear_windows_spawn_resources can run on failure paths. Remove unused named_pipe imports that fail Clippy on Windows CI.
Drop the reload-time clear_secret_caches hook and config_gate stub from spawn-profiles; secret-backend-gates owns cache invalidation.
Drop the config_gate stub module and condition_config_any wiring from spawn-profiles; config-gates owns gate parsing and evaluation.
Log unmet path conditions from condition_path_exists_met and have may_auto_start delegate to start_conditions_met after the auto_start check.
PartialEq already compares &ProcessConfig with ProcessConfig directly.
Separate process state from the daemon event loop so handlers take a single RuntimeHandles bundle and reload logic lives in its own module.
Keeps the manager module split intuitive: process_manager.rs owns the struct and handlers, while mod.rs retains shared helpers and tests.
Move recompute_startup_order into process_manager.rs since only ProcessManager uses it; mod.rs keeps cross-module spawn/reload helpers.
Apply rustfmt to the manager split and remove Windows-only dead code that failed clippy with -D warnings.
Agent-profile spawn needs dd-procmgr-service as LocalSystem to read the agent password from LSA. Re-add the test in #54735 with the Wix change.
Re-add the test dropped from #54731. Agent-profile spawn for PAR requires dd-procmgr-service as LocalSystem (Wix change in this PR) to read the agent password from LSA.
750b479 to
a5ef62f
Compare
…les PR Restore procmgr_win_test.go to main and drop the extracted helpers file. The only E2E delta on this PR is removing the PAR test for #54735.
What does this PR do?
Introduces Windows spawn profiles in dd-procmgr so managed children can run under different security contexts:
ddagentuser)Adds the Windows spawn stack (token logon, user profile load, supervision job, suspended
CreateProcessAsUserW), COAT catalog enforcement for allowed profiles, and gRPC pipe caller authentication.Stack context: PR 1/4 split from #53249. Config gates, secret backend resolution, and process-agent dual-mode integration land in follow-up PRs (
jose/procmgr-config-gates,jose/procmgr-secret-backend-gates,jose/procmgr-windows-process-agent). This PR includes a stubconfig_gatemodule (gates always open) so the crate compiles until PR 2.Motivation
Process-agent on Windows must run as LocalSystem and other agent children should stay on the agent user. Spawn profiles make that explicit and enforceable at spawn time, and are a prerequisite for moving process-agent supervision off legacy SCM in later PRs.
Describe how you validated your changes
pkg/procmgr/coat/...)test/new-e2e/tests/agent-runtimes/procmgr/...)Additional Notes