bazel: migrate curl & openssl to dd_cc_packaged & dd_collect_dependencies#49403
Conversation
adc2142 to
a43e57d
Compare
509ee09 to
13e245b
Compare
a43e57d to
aeb3d26
Compare
aeb3d26 to
c546639
Compare
532c884 to
cb43834
Compare
c546639 to
8f3c11a
Compare
cb43834 to
a27f904
Compare
Files inventory check summaryFile checks results against ancestor 0f36ad4c: Results for datadog-agent_7.80.0~devel.git.532.6f21cc3.pipeline.111668039-1_amd64.deb:Detected file changes:
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 2733cb8 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | +0.61 | [-2.26, +3.48] | 1 | Logs |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | +0.61 | [-2.26, +3.48] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulativetodelta_exporter | memory utilization | +0.55 | [+0.32, +0.79] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulative | memory utilization | +0.46 | [+0.30, +0.61] | 1 | Logs |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.43 | [+0.18, +0.67] | 1 | Logs bounds checks dashboard |
| ➖ | ddot_logs | memory utilization | +0.35 | [+0.28, +0.42] | 1 | Logs |
| ➖ | ddot_metrics_sum_delta | memory utilization | +0.34 | [+0.15, +0.53] | 1 | Logs |
| ➖ | otlp_ingest_metrics | memory utilization | +0.33 | [+0.17, +0.48] | 1 | Logs |
| ➖ | uds_dogstatsd_20mb_12k_contexts_20_senders | memory utilization | +0.22 | [+0.17, +0.26] | 1 | Logs |
| ➖ | file_tree | memory utilization | +0.16 | [+0.10, +0.21] | 1 | Logs |
| ➖ | file_to_blackhole_500ms_latency | egress throughput | +0.07 | [-0.34, +0.48] | 1 | Logs |
| ➖ | file_to_blackhole_0ms_latency | egress throughput | +0.05 | [-0.48, +0.59] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api | ingress throughput | +0.01 | [-0.19, +0.21] | 1 | Logs |
| ➖ | file_to_blackhole_100ms_latency | egress throughput | +0.01 | [-0.13, +0.15] | 1 | Logs |
| ➖ | file_to_blackhole_1000ms_latency | egress throughput | -0.00 | [-0.45, +0.44] | 1 | Logs |
| ➖ | tcp_dd_logs_filter_exclude | ingress throughput | -0.01 | [-0.10, +0.09] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api_v3 | ingress throughput | -0.03 | [-0.23, +0.16] | 1 | Logs |
| ➖ | docker_containers_memory | memory utilization | -0.10 | [-0.20, +0.00] | 1 | Logs |
| ➖ | otlp_ingest_logs | memory utilization | -0.15 | [-0.25, -0.05] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | -0.15 | [-0.20, -0.10] | 1 | Logs bounds checks dashboard |
| ➖ | ddot_metrics | memory utilization | -0.18 | [-0.38, +0.03] | 1 | Logs |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.29 | [-0.33, -0.25] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | -1.12 | [-2.10, -0.15] | 1 | Logs bounds checks dashboard |
| ➖ | tcp_syslog_to_blackhole | ingress throughput | -1.20 | [-1.40, -0.99] | 1 | Logs |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | docker_containers_cpu | simple_check_run | 10/10 | 709 ≥ 26 | |
| ✅ | docker_containers_memory | memory_usage | 10/10 | 244.49MiB ≤ 370MiB | |
| ✅ | docker_containers_memory | simple_check_run | 10/10 | 694 ≥ 26 | |
| ✅ | file_to_blackhole_0ms_latency | memory_usage | 10/10 | 0.16GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_0ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_1000ms_latency | memory_usage | 10/10 | 0.20GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_1000ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_100ms_latency | memory_usage | 10/10 | 0.17GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_100ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | file_to_blackhole_500ms_latency | memory_usage | 10/10 | 0.19GiB ≤ 1.20GiB | |
| ✅ | file_to_blackhole_500ms_latency | missed_bytes | 10/10 | 0B = 0B | |
| ✅ | quality_gate_idle | intake_connections | 10/10 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 143.47MiB ≤ 147MiB | 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 | 470.64MiB ≤ 495MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 174.45MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 356.39 ≤ 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 | 376.92MiB ≤ 430MiB | 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_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 missed_bytes: 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_metrics_logs, bounds check cpu_usage: 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_logs, bounds check missed_bytes: 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.
59e1214 to
24da229
Compare
c487ba7 to
4a2faf0
Compare
|
🎯 Code Coverage (details) 🔗 Commit SHA: 6f21cc3 | Docs | Datadog PR Page | Give us feedback! |
3252646 to
54eb10e
Compare
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
On-wire sizes (compressed)
|
2be0dac to
7aead3f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aead3faa3
ℹ️ 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".
| "//packages/agent:linux_heroku": [ | ||
| "@curl//:all_files", | ||
| "@gpg-error", | ||
| "@lua//:liblua", | ||
| "@nghttp2", | ||
| ], |
There was a problem hiding this comment.
Keep Heroku license deps aligned with shipped curl stack
Removing curl/nghttp2 from //packages/install_dir:all_deps breaks license collection for Heroku builds: omnibus/config/software/datadog-agent-finalize.rb uses //packages/install_dir:install plus //packages/agent/heroku:license_files_install, and packages/agent/heroku/BUILD.bazel currently defines all_deps as empty, so there is no alternate path that re-adds those licenses. At the same time, Heroku still ships curl transitively via //packages/agent/dependencies:_collected_libs, so this creates a package-content vs license-manifest mismatch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This seems a real problem. -1.15 MiB drop in size.
There was a problem hiding this comment.
This is now fixed (partly by fixing the installed packages from bazel, partly by fixing a stale cache issue - #50161)
We were still requiring an external invocation of configure_fips_tool from omnibus to generate fipsinstall.sh & openssl.cnf
ca8e6cf to
6f21cc3
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
…cies (#49403) ### What does this PR do? Continue the migration toward installing dependencies with dd_collect_dependencies This PR mostly migrates OpenSSL, Python & rtloader. It also contains some changes to our rules: * ` foreign_cc_shared_wrapper`: * stop re-exporting the wrapped target's providers * add lib_filter to allow producing multiple wrappers objects for multiple libraries from a single configure_make target * return DefaultInfo so the wrapper can be used as a data dependency * `dd_cc_packaged`: add `installed_executables` attribute to rpath-patch and install executables/shared modules (engines, ossl-modules, …) alongside the library * `rewrite_rpath`: extract `patchelf_file_action` / `otool_file_action` & introduce `patchelf_dir_action` / `otool_dir_action` as reusable helpers. We now can rpath-patch entire output directories (useful for python modules & openssl engines) ### Motivation Migrating away from omnibus. ### Describe how you validated your changes Local `bazel test //...` and CI ### Additional Notes ~Based on top of #49386 (merged) ~Based on top of #49500 (merged) ~The rules_pkg change is opened upstream at bazelbuild/rules_pkg#1053 (change merged & `rules_pkg` commit bumped in `main`) The preliminary changes to our tooling (`foreign_cc_shared_wrapper`, `dd_cc_packaged`'s `installed_executable` & `rewrite_rpath` refactoring can each be split out in their own PR if it simplifies the review) The heroku size increase is coming from license files that weren't shipped in the package before: ``` - LICENSES/krb5-NOTICE — 64 KB - LICENSES/rpm-COPYING — 44 KB - LICENSES/gstatus-LICENSE — 35 KB - LICENSES/acl-COPYING.LGPL — 26 KB - LICENSES/attr-COPYING.LGPL — 26 KB - LICENSES/gcrypt-COPYING.LIB — 26 KB - LICENSES/systemd-LICENSE.LGPL2.1 — 26 KB - LICENSES/libsepol-LICENSE — 26 KB - LICENSES/freetds-COPYING.LIB — 25 KB - LICENSES/unixodbc-COPYING — 24 KB - LICENSES/cacerts-MPL-2.0.txt — 16 KB - LICENSES/msodbcsql18-LICENSE.txt — 12 KB - LICENSES/xmlsec-Copyright — 7 KB - LICENSES/jmxfetch-LICENSE — 2 KB - LICENSES/dbus-COPYING — 1 KB - LICENSES/popt-COPYING — 1 KB - LICENSES/attr-LICENSE — 1 KB - sources/acl/offer.txt — 128 B - sources/attr/offer.txt — 129 B - sources/freetds/offer.txt — 133 B - sources/libsepol/offer.txt — 131 B - sources/systemd/offer.txt — 130 B ``` Co-authored-by: hugo.beauzee <hugo.beauzee@datadoghq.com>
What does this PR do?
Continue the migration toward installing dependencies with dd_collect_dependencies
This PR mostly migrates OpenSSL, Python & rtloader.
It also contains some changes to our rules:
foreign_cc_shared_wrapper:dd_cc_packaged: addinstalled_executablesattribute to rpath-patch andinstall executables/shared modules (engines, ossl-modules, …) alongside the library
rewrite_rpath: extractpatchelf_file_action/otool_file_action& introducepatchelf_dir_action/otool_dir_actionas reusable helpers. We now can rpath-patch entire output directories (useful for python modules & openssl engines)Motivation
Migrating away from omnibus.
Describe how you validated your changes
Local
bazel test //...and CIAdditional Notes
Based on top of #49386(merged)Based on top of #49500(merged)The rules_pkg change is opened upstream at bazelbuild/rules_pkg#1053(change merged &rules_pkgcommit bumped inmain)The preliminary changes to our tooling (
foreign_cc_shared_wrapper,dd_cc_packaged'sinstalled_executable&rewrite_rpathrefactoring can each be split out in their own PR if it simplifies the review)The heroku size increase is coming from license files that weren't shipped in the package before: