Remove redundant gazelle exclusions (**/build) and fix setup inconsistencies#48279
Conversation
Files inventory check summaryFile checks results against ancestor 4035f3a4: Results for datadog-agent_7.79.0~devel.git.240.472f7ed.pipeline.104804549-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates 31 successful checks with minimal change (< 2 KiB)
On-wire sizes (compressed)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 8b5150a Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | -4.50 | [-7.39, -1.60] | 1 | Logs |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | tcp_syslog_to_blackhole | ingress throughput | +1.51 | [+1.36, +1.67] | 1 | Logs |
| ➖ | uds_dogstatsd_20mb_12k_contexts_20_senders | memory utilization | +0.36 | [+0.30, +0.42] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +0.25 | [+0.20, +0.30] | 1 | Logs bounds checks dashboard |
| ➖ | ddot_metrics | memory utilization | +0.19 | [+0.00, +0.37] | 1 | Logs |
| ➖ | file_tree | memory utilization | +0.07 | [+0.01, +0.13] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulativetodelta_exporter | memory utilization | +0.05 | [-0.17, +0.28] | 1 | Logs |
| ➖ | file_to_blackhole_100ms_latency | egress throughput | +0.01 | [-0.07, +0.09] | 1 | Logs |
| ➖ | tcp_dd_logs_filter_exclude | ingress throughput | +0.00 | [-0.11, +0.11] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api_v3 | ingress throughput | -0.01 | [-0.21, +0.20] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api | ingress throughput | -0.01 | [-0.21, +0.19] | 1 | Logs |
| ➖ | file_to_blackhole_1000ms_latency | egress throughput | -0.01 | [-0.43, +0.40] | 1 | Logs |
| ➖ | docker_containers_memory | memory utilization | -0.01 | [-0.09, +0.06] | 1 | Logs |
| ➖ | file_to_blackhole_500ms_latency | egress throughput | -0.02 | [-0.40, +0.37] | 1 | Logs |
| ➖ | ddot_logs | memory utilization | -0.05 | [-0.11, +0.01] | 1 | Logs |
| ➖ | file_to_blackhole_0ms_latency | egress throughput | -0.12 | [-0.62, +0.38] | 1 | Logs |
| ➖ | ddot_metrics_sum_delta | memory utilization | -0.20 | [-0.36, -0.03] | 1 | Logs |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.22 | [-0.26, -0.18] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.22 | [-0.46, +0.01] | 1 | Logs bounds checks dashboard |
| ➖ | otlp_ingest_metrics | memory utilization | -0.37 | [-0.54, -0.21] | 1 | Logs |
| ➖ | otlp_ingest_logs | memory utilization | -0.46 | [-0.56, -0.37] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulative | memory utilization | -0.57 | [-0.71, -0.43] | 1 | Logs |
| ➖ | quality_gate_logs | % cpu utilization | -1.54 | [-3.17, +0.09] | 1 | Logs bounds checks dashboard |
| ➖ | docker_containers_cpu | % cpu utilization | -4.50 | [-7.39, -1.60] | 1 | Logs |
Bounds Checks: ❌ Failed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | docker_containers_cpu | simple_check_run | 10/10 | 692 ≥ 26 | |
| ✅ | docker_containers_memory | memory_usage | 10/10 | 271.90MiB ≤ 370MiB | |
| ✅ | docker_containers_memory | simple_check_run | 10/10 | 682 ≥ 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.23GiB ≤ 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 | 3 = 3 | bounds checks dashboard |
| ❌ | quality_gate_idle | memory_usage | 9/10 | 175.88MiB > 175MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 3 = 3 | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | memory_usage | 10/10 | 490.78MiB ≤ 550MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 209.85MiB ≤ 220MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 347.27 ≤ 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 | 417.50MiB ≤ 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
❌ Failed. Some Quality Gates were violated.
- 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.
- quality_gate_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_metrics_logs, bounds check missed_bytes: 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: 9/10 replicas passed. Failed 1 which is > 0. Gate FAILED.
- 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.
gazelle:excludes awaiting Bazel 9gazelle:excludes awaiting Bazel 9 [NOT YET]
Now lifted by #48455.
f0c6115 to
472f7ed
Compare
gazelle:excludes awaiting Bazel 9 [NOT YET]gazelle setup and remove redundant **/build exclusions
gazelle setup and remove redundant **/build exclusions**/build exclusions and fix gazelle setup inconsistencies
**/build exclusions and fix gazelle setup inconsistenciesgazelle exclusions on **/build and fix setup inconsistencies
Gitlab CI Configuration ChangesModified Jobsbazel:run-gazelle bazel:run-gazelle:
after_script:
- "if [ $CI_JOB_STATUS = failed ]; then\n echo >&2 \"\U0001F4A1 To fix the above\
\ problem, please consider running the following command:\"\n echo >&2 \"$FIX_COMMAND\"\
\nfi"
cache:
- key:
files:
- .bazelversion
prefix: bazelversion-$CI_RUNNER_DESCRIPTION
paths:
- .cache/bazelisk
- .cache/bazel/*/install
policy: pull$BAZEL_CACHE_POLICY_SUFFIX
when: on_success
- key:
files:
- .go-version
- .python-version
prefix: bazel-$CI_JOB_NAME
paths:
- .cache/bazel/*/cache
- .cache/go
- .cache/ms-go
- .cache/pip
policy: pull$BAZEL_CACHE_POLICY_SUFFIX
when: on_success
image: registry.ddbuild.io/ci/datadog-agent-buildimages/linux$CI_IMAGE_LINUX_SUFFIX:$CI_IMAGE_LINUX
needs: []
script:
- - bazel run //:gazelle -- -mode=diff -strict
? --------
+ - bazel run //:gazelle -- -mode=diff
stage: lint
tags:
- arch:amd64
- specific:true
variables:
BAZELISK_HOME: $XDG_CACHE_HOME/bazelisk
FIX_COMMAND: bazel run //:gazelle
KUBERNETES_CPU_REQUEST: 4
KUBERNETES_MEMORY_LIMIT: 12Gi
KUBERNETES_MEMORY_REQUEST: 12Gi
XDG_CACHE_HOME: $CI_PROJECT_DIR/.cacheChanges Summary
ℹ️ Diff available in the job log. |
gazelle exclusions on **/build and fix setup inconsistenciesgazelle exclusions (build) and fix setup inconsistencies
gazelle exclusions (build) and fix setup inconsistenciesgazelle exclusions (**/build) and fix setup inconsistencies
| # gazelle:exclude tools/host-profiler | ||
| # gazelle:exclude tools/retry_file_dump | ||
| # gazelle:exclude tools/windows | ||
| # gazelle:exclude .gitlab |
There was a problem hiding this comment.
Seems like it is worth keeping because we should never have code in .gitlab.
There was a problem hiding this comment.
Good news: gazelle honors both the good old .bazelignore and the more recent ignore_directories in REPO.bazel (Bazel 8.0), see:
func newIgnoreFilter(repoRoot string) *ignoreFilter {
bazelignorePaths, err := loadBazelIgnore(repoRoot)
if err != nil {
log.Printf("error loading .bazelignore: %v", err)
}
repoDirectoryIgnores, err := loadRepoDirectoryIgnore(repoRoot)
I'll therefore file a PR to extend the 2nd as a preferred point of maintenance (supports globs):
Lines 1 to 6 in 8b5150a
There was a problem hiding this comment.
### What does this PR do? Replace the single `.cache` entry in `ignore_directories` with a `.*` glob that matches all "hidden" root directories at once. ### Motivation Several "hidden" directories live at the repo root, such as: - .cache [1.] - .claude - .cursor - .dda - .ddqa - .git - .github - .gitlab [2.] - .idea - .mypy_cache - .ruff_cache - .run - .vscode - etc. Each one risks confusing `bazel`'s directory watcher and/or `gazelle`'s scope, and the list keeps growing as new tools drop directories there. The `.*` glob covers all of them without requiring future additions, including: 1. `.cache`, which was already listed, 2. `.gitlab`, thus honoring #48279 (comment)
f0055cf
into
main
### What does this PR do? Replace the single `.cache` entry in `ignore_directories` with a `.*` glob that matches all "hidden" root directories at once. ### Motivation Several "hidden" directories live at the repo root, such as: - .cache [1.] - .claude - .cursor - .dda - .ddqa - .git - .github - .gitlab [2.] - .idea - .mypy_cache - .ruff_cache - .run - .vscode - etc. Each one risks confusing `bazel`'s directory watcher and/or `gazelle`'s scope, and the list keeps growing as new tools drop directories there. The `.*` glob covers all of them without requiring future additions, including: 1. `.cache`, which was already listed, 2. `.gitlab`, thus honoring #48279 (comment) The exclusions are of course honored by `bazel` itself, but also by `gazelle` ([source](https://github.com/bazel-contrib/bazel-gazelle/blob/4a7caee10fbd2da9bb059f3ce7c2cac8d0050e53/walk/config.go#L211-L217)) and probably others.
### What does this PR do? Replace the single `.cache` entry in `ignore_directories` of `REPO.bazel` with a `.*` glob that matches all "hidden" directories at the root of the workspace. ### Motivation Several "hidden" directories live at the repo root, such as: - .cache [1.] - .claude - .cursor - .dda - .ddqa - .git - .github - .gitlab [2.] - .idea - .mypy_cache - .ruff_cache - .run - .vscode - etc. Each one risks confusing `bazel`'s directory watcher and/or `gazelle`'s scope, and the list keeps growing as new tools drop directories there. The `.*` glob covers all of them without requiring future additions, including: 1. `.cache`, which was already listed, 2. `.gitlab`, thus honoring #48279 (comment) ### Additional Notes Such exclusions are of course used by `bazel` itself, but also by `gazelle` ([source](https://github.com/bazel-contrib/bazel-gazelle/blob/4a7caee10fbd2da9bb059f3ce7c2cac8d0050e53/walk/config.go#L211-L217)) and probably other tools of the Bazel ecosystem. Co-authored-by: regis.desgroppes <regis.desgroppes@datadoghq.com>
What does this PR do?
Remove
gazelleexclusions that had to be put in place pending a fix forgazelleconfused bybuilddirectories on "broken" case-insensitive filesystem stacks (docker/desktop-feedback#251).Doing so, I realized there were inconsistencies in the
gazellesetup and took the opportunity to fix them.Motivation
#48455 indeed allows to get rid of the following exclusions:
Nearby inconsistencies in the
gazellesetup:build_tagsis passed by attribute (build_tags = ALL_BUILD_TAGS) => OK (canonical),externalwas passed byargs(-external=static) => pass by attribute (external = "static"),prefixwas passed by directive (# gazelle:prefix github.com/DataDog/datadog-agent) => pass by attribute (prefix = "github.com/DataDog/datadog-agent"),argshappens to be a compatibility shim => useextra_args(canonical) when there's no corresponding attribute yet:build_file_namewas passed byargsand doesn't have a corresponding attribute yet => pass byextra_args,strictwas passed from a single GitLab job and doesn't have a corresponding attribute yet => pass byextra_args.Describe how you validated your changes
Kind assistance of @alopezz and/or @JSGette who run Linux containers on their macOS-powered laptop.