[Backport 7.79.x] macos: fix system-probe.yaml being overwritten on upgrade#51191
Conversation
…# What does this PR do? Saves and restores `system-probe.yaml` across DMG upgrades, mirroring the existing `datadog.yaml` save/restore in the pre/postinst scripts. ### Motivation The Bazel `//cmd/agent/dist:config_files` rule ships a bare `system-probe.yaml` (generated from `system-probe_template.yaml`) in the pkg payload — verified by running `bazel run //packages/agent/product:post_build_install`, which writes both `etc/system-probe.yaml` and `etc/datadog.yaml` to the destdir. The pkg installer overwrites those exact paths during extraction. `datadog.yaml` survives upgrade because preinst saves it and postinst restores it after extraction; `system-probe.yaml` had no such save/restore, so any user customization (NPM, USM, CWS settings, etc.) was lost on every agent upgrade. ### Describe how you validated your changes Ran `bazel run //packages/agent/product:post_build_install -- --destdir=/tmp/check` and confirmed `etc/system-probe.yaml` is present as a bare file in the payload. Verified the new `cp` lines follow the existing pattern (`cp -vf … 2>/dev/null || true`), so a missing source file (the common case where the user never touched `system-probe.yaml`) doesn't fail the install. Full macOS install/upgrade E2E coverage runs on merge to `main`, not on PR branches. ### Additional Notes `security-agent.yaml` is shipped only as `*.yaml.example` (via the omnibus `move` from `pkg/config/example/security-agent.yaml.example`) and therefore survives upgrade without explicit save/restore — out of scope here. Same for `ai_usage_native_host.yaml`, which postinst seeds from `.example` when missing. Co-authored-by: brian.tu <brian.tu@datadoghq.com> (cherry picked from commit 2c81359) ___ Co-authored-by: Brian Tu <brian.tu@datadoghq.com>
|
Files inventory check summaryFile checks results against ancestor 59a166c5: Results for datadog-agent_7.79.0.git.5.8b860d3.pipeline.114631202-1_amd64.deb:No change detected |
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 59a166c Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | +3.65 | [+0.61, +6.68] | 1 | Logs |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | +3.65 | [+0.61, +6.68] | 1 | Logs |
| ➖ | uds_dogstatsd_20mb_12k_contexts_20_senders | memory utilization | +0.59 | [+0.52, +0.65] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulativetodelta_exporter | memory utilization | +0.36 | [+0.14, +0.59] | 1 | Logs |
| ➖ | ddot_metrics_sum_delta | memory utilization | +0.26 | [+0.09, +0.43] | 1 | Logs |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.17 | [+0.13, +0.21] | 1 | Logs bounds checks dashboard |
| ➖ | ddot_metrics | memory utilization | +0.16 | [-0.02, +0.33] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +0.04 | [-0.01, +0.09] | 1 | Logs bounds checks dashboard |
| ➖ | file_to_blackhole_500ms_latency | egress throughput | +0.03 | [-0.37, +0.42] | 1 | Logs |
| ➖ | tcp_dd_logs_filter_exclude | ingress throughput | +0.02 | [-0.09, +0.14] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api_v3 | ingress throughput | +0.02 | [-0.20, +0.24] | 1 | Logs |
| ➖ | ddot_logs | memory utilization | +0.01 | [-0.05, +0.08] | 1 | Logs |
| ➖ | file_to_blackhole_0ms_latency | egress throughput | +0.01 | [-0.52, +0.54] | 1 | Logs |
| ➖ | otlp_ingest_metrics | memory utilization | +0.01 | [-0.14, +0.17] | 1 | Logs |
| ➖ | file_to_blackhole_100ms_latency | egress throughput | +0.01 | [-0.11, +0.13] | 1 | Logs |
| ➖ | docker_containers_memory | memory utilization | -0.00 | [-0.08, +0.08] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api | ingress throughput | -0.00 | [-0.20, +0.19] | 1 | Logs |
| ➖ | file_to_blackhole_1000ms_latency | egress throughput | -0.01 | [-0.46, +0.43] | 1 | Logs |
| ➖ | file_tree | memory utilization | -0.09 | [-0.15, -0.03] | 1 | Logs |
| ➖ | otlp_ingest_logs | memory utilization | -0.13 | [-0.22, -0.03] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulative | memory utilization | -0.37 | [-0.52, -0.21] | 1 | Logs |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.40 | [-0.63, -0.17] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -1.07 | [-2.63, +0.49] | 1 | Logs bounds checks dashboard |
| ➖ | tcp_syslog_to_blackhole | ingress throughput | -1.27 | [-1.42, -1.12] | 1 | Logs |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | docker_containers_cpu | simple_check_run | 10/10 | 653 ≥ 26 | |
| ✅ | docker_containers_memory | memory_usage | 10/10 | 278.25MiB ≤ 370MiB | |
| ✅ | docker_containers_memory | simple_check_run | 10/10 | 717 ≥ 26 | |
| ✅ | file_to_blackhole_0ms_latency | memory_usage | 10/10 | 0.19GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_0ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_1000ms_latency | memory_usage | 10/10 | 0.24GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_1000ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_100ms_latency | memory_usage | 10/10 | 0.20GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_100ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_500ms_latency | memory_usage | 10/10 | 0.22GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_500ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 175.96MiB ≤ 181MiB | 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 | 496.38MiB ≤ 550MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 212.52MiB ≤ 220MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 352.83 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 424.49MiB ≤ 475MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | 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_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_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 cpu_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 memory_usage: 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 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 intake_connections: 10/10 replicas passed. Gate passed.
Backport 2c81359 from #51176.
What does this PR do?
Saves and restores
system-probe.yamlacross DMG upgrades, mirroring the existingdatadog.yamlsave/restore in the pre/postinst scripts.Motivation
The Bazel
//cmd/agent/dist:config_filesrule ships a baresystem-probe.yaml(generated fromsystem-probe_template.yaml) in the pkg payload — verified by runningbazel run //packages/agent/product:post_build_install, which writes bothetc/system-probe.yamlandetc/datadog.yamlto the destdir. The pkg installer overwrites those exact paths during extraction.datadog.yamlsurvives upgrade because preinst saves it and postinst restores it after extraction;system-probe.yamlhad no such save/restore, so any user customization (NPM, USM, CWS settings, etc.) was lost on every agent upgrade.Describe how you validated your changes
Ran
bazel run //packages/agent/product:post_build_install -- --destdir=/tmp/checkand confirmedetc/system-probe.yamlis present as a bare file in the payload. Verified the newcplines follow the existing pattern (cp -vf … 2>/dev/null || true), so a missing source file (the common case where the user never touchedsystem-probe.yaml) doesn't fail the install. Full macOS install/upgrade E2E coverage runs on merge tomain, not on PR branches.Additional Notes
security-agent.yamlis shipped only as*.yaml.example(via the omnibusmovefrompkg/config/example/security-agent.yaml.example) and therefore survives upgrade without explicit save/restore — out of scope here. Same forai_usage_native_host.yaml, which postinst seeds from.examplewhen missing.