[ABLD-430] Add options to dda test to use bazel for migrated tests - #49294
[ABLD-430] Add options to dda test to use bazel for migrated tests#49294aiuto wants to merge 29 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds experimental Bazel integration to dda inv test so migrated Go tests can be skipped from go test, optionally exported as labels, and/or executed via bazel test to benefit from Bazel caching.
Changes:
- Introduces
--skip-bazel-tests,--save-bazel-tests,--run-bazel-tests, plusEXPERIMENTAL_USE_BAZEL_TESTSenv opt-in intasks/gotest.py. - Extends the Bazel invocation helper (
tasks/libs/build/bazel.py) to optionally hide stderr noise or capture stderr when needed. - Updates test result processing to return
(success, stats)and adjusts the E2E runner callsite accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tasks/new_e2e_tests.py | Updates caller to unpack the new (success, stats) return shape from process_test_result. |
| tasks/libs/build/bazel.py | Enhances bazel() wrapper to support hiding/capturing stderr for Bazel output parsing use-cases. |
| tasks/gotest.py | Implements Bazel target discovery, go-test exclusion, optional Bazel execution, and combined reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| target_path = os.path.join(module.path, target) | ||
| if not target_path.startswith('./'): | ||
| target_path = f'./{target_path}' | ||
| if recursive and not target_path.endswith('/...'): | ||
| pattern = f'{target_path}/...' | ||
| else: | ||
| pattern = target_path | ||
| with ctx.cd(module.path): | ||
| res = ctx.run( | ||
| f'go list -buildvcs=false -tags "{tag_str}" {pattern}', |
There was a problem hiding this comment.
expand_packages_excluding changes into module.path but still builds pattern using os.path.join(module.path, target). For non-root modules this doubles the path (e.g. cd into comp/foo then run go list ./comp/foo/...), which will generally return no packages and can cause dda inv test --skip-bazel-tests to silently skip running tests for those modules. Build the go list pattern relative to the current module directory (use target directly), or run go list from repo root without ctx.cd(module.path) so paths stay consistent.
There was a problem hiding this comment.
Indeed, this looks wrong, can you check what happens with non-root modules?
There was a problem hiding this comment.
Yes. It was a problem. I gave claude the copilot comments and a reproduction case, and it came up with a fix.
Files inventory check summaryFile checks results against ancestor 141f1206: Results for datadog-agent_7.79.0~devel.git.726.a596172.pipeline.107660434-1_amd64.deb:Detected file changes:
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 141f120 Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | docker_containers_cpu | % cpu utilization | +1.10 | [-1.88, +4.07] | 1 | Logs |
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_logs | % cpu utilization | +2.14 | [+0.49, +3.80] | 1 | Logs bounds checks dashboard |
| ➖ | docker_containers_cpu | % cpu utilization | +1.10 | [-1.88, +4.07] | 1 | Logs |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.69 | [+0.66, +0.72] | 1 | Logs bounds checks dashboard |
| ➖ | ddot_metrics | memory utilization | +0.68 | [+0.51, +0.86] | 1 | Logs |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.58 | [+0.36, +0.81] | 1 | Logs bounds checks dashboard |
| ➖ | tcp_syslog_to_blackhole | ingress throughput | +0.38 | [+0.22, +0.54] | 1 | Logs |
| ➖ | docker_containers_memory | memory utilization | +0.26 | [+0.18, +0.34] | 1 | Logs |
| ➖ | uds_dogstatsd_20mb_12k_contexts_20_senders | memory utilization | +0.22 | [+0.16, +0.29] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulativetodelta_exporter | memory utilization | +0.07 | [-0.15, +0.30] | 1 | Logs |
| ➖ | file_tree | memory utilization | +0.07 | [+0.00, +0.13] | 1 | Logs |
| ➖ | quality_gate_idle | memory utilization | +0.04 | [-0.01, +0.09] | 1 | Logs bounds checks dashboard |
| ➖ | file_to_blackhole_1000ms_latency | egress throughput | +0.03 | [-0.40, +0.47] | 1 | Logs |
| ➖ | file_to_blackhole_100ms_latency | egress throughput | +0.03 | [-0.08, +0.14] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api_v3 | ingress throughput | +0.01 | [-0.20, +0.23] | 1 | Logs |
| ➖ | tcp_dd_logs_filter_exclude | ingress throughput | -0.00 | [-0.11, +0.11] | 1 | Logs |
| ➖ | ddot_metrics_sum_delta | memory utilization | -0.01 | [-0.17, +0.15] | 1 | Logs |
| ➖ | uds_dogstatsd_to_api | ingress throughput | -0.01 | [-0.22, +0.19] | 1 | Logs |
| ➖ | file_to_blackhole_500ms_latency | egress throughput | -0.02 | [-0.43, +0.39] | 1 | Logs |
| ➖ | otlp_ingest_metrics | memory utilization | -0.05 | [-0.22, +0.11] | 1 | Logs |
| ➖ | file_to_blackhole_0ms_latency | egress throughput | -0.06 | [-0.58, +0.47] | 1 | Logs |
| ➖ | ddot_logs | memory utilization | -0.13 | [-0.20, -0.06] | 1 | Logs |
| ➖ | ddot_metrics_sum_cumulative | memory utilization | -0.44 | [-0.58, -0.30] | 1 | Logs |
| ➖ | otlp_ingest_logs | memory utilization | -0.48 | [-0.57, -0.38] | 1 | Logs |
Bounds Checks: ✅ Passed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | docker_containers_cpu | simple_check_run | 10/10 | 635 ≥ 26 | |
| ✅ | docker_containers_memory | memory_usage | 10/10 | 275.48MiB ≤ 370MiB | |
| ✅ | docker_containers_memory | simple_check_run | 10/10 | 695 ≥ 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 | 10/10 | 176.90MiB ≤ 181MiB | 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 | 501.01MiB ≤ 550MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 3 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 207.77MiB ≤ 220MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 355.08 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 3 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 415.62MiB ≤ 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_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.
- 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 cpu_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 memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
alopezz
left a comment
There was a problem hiding this comment.
It's a lot of code so I haven't gone through the whole thing yet, but copilot's comments are also probably worth addressing.
| rel = target[2:] if target.startswith('./') else target | ||
| rel = rel.rstrip('/') |
There was a problem hiding this comment.
This can be done a bit simpler:
| rel = target[2:] if target.startswith('./') else target | |
| rel = rel.rstrip('/') | |
| rel = target.removeprefix('./').rstrip('/') |
| _bazel_env = os.environ.get("EXPERIMENTAL_USE_BAZEL_TESTS", "") | ||
| if _bazel_env not in ("", "0"): | ||
| skip_bazel_tests = True | ||
| run_bazel_tests = True |
There was a problem hiding this comment.
This looks very confusing.
- The naming of the flags/variables is very unfortunate: skip_bazel_tests means skipping tests in go, run_bazel_tests means running tests in bazel.
- What is the intended use case for the True-False and False-True combinations of the flags? It looks like at the very least you'd never want to just skip tests with this.
- Having a special environment variable override the behavior of multiple flags is another point of weirdness. I'd advocate for not having the environment variable and driving it entirely via a command line flag.
A possible suggestion is to instead use an enum-style flag such as:
--bazel-tests=off # default: go test handles everything
--bazel-tests=run # go test + also run bazel test (comparison mode)
--bazel-tests=replace # go test hands off to bazel test (equivalent of setting the environment variable in the current implementation)
There was a problem hiding this comment.
The naming is absolutely bad. The multiple values is intentional.
- Default behavior should be to run it the old way
- I want the ability to do the equivalent of replace.
- This must be easy to opt in without users having to learn new flags that are essentially temporary. That is why the I definitely want the environment variable.
- I also want to split the work in CI.
dda inv -- test <args to select packages> --skip_bazel_test, --write_bazel_tests=bazeltests.txt.
cat bazeltests.txt | xargs bazel test
The last case is
- so that we can collect sub metrics on the bazel performance.
- to eventiually hook it up with capturing the build event stream and reporting finer grained test stats to push up to ddci. I want that code standalone rather than in dda.
How about... we change the flags to
--skip_tests_covered_by_bazel This is just a rename of the --skip_bazel_tests
--prefer_bazel_testing. This is the equivalent of --skip_bazel_tests + --run_bazel_tests
--write_bazel_test_list. replaces --save_bazel_tests
Now, we can also put "experimental" at the beginning of each. I don't want users to presume they are permanent parts of their workflows.
| prefix = '' if module.path == '.' else module.path + '/' | ||
| for target in module.test_targets: | ||
| # Prepend module path for non-root modules | ||
| full_target = f'./{prefix}{target.lstrip("./")}' if module.path != '.' else target |
There was a problem hiding this comment.
I suspect the intention is:
| full_target = f'./{prefix}{target.lstrip("./")}' if module.path != '.' else target | |
| full_target = f'./{prefix}{target.removeprefix("./")}' if module.path != '.' else target |
It probably works either way due to the inputs we get to this function, but target.lstrip would remove any number of . or / at the beginning of the string.
There was a problem hiding this comment.
Changed.
TBH... If was building something for production, I would verify any packages and modules from the command line against the file system, and come in with a set of normalized paths.
But that is a large change that is independent of this PR.
| target_path = os.path.join(module.path, target) | ||
| if not target_path.startswith('./'): | ||
| target_path = f'./{target_path}' | ||
| if recursive and not target_path.endswith('/...'): | ||
| pattern = f'{target_path}/...' | ||
| else: | ||
| pattern = target_path | ||
| with ctx.cd(module.path): | ||
| res = ctx.run( | ||
| f'go list -buildvcs=false -tags "{tag_str}" {pattern}', |
There was a problem hiding this comment.
Indeed, this looks wrong, can you check what happens with non-root modules?
| # In non-verbose mode filter Bazel's progress/info noise. | ||
| if not verbose and line.strip().startswith(_NOISY_PREFIXES): | ||
| continue |
There was a problem hiding this comment.
I'm not sure this does much, because right below _parse_bazel_test_line(line) and the following condition already filter the resulting parsed_results to lines that start with //.
There was a problem hiding this comment.
What is missing is that it should print the lines which are not test out but not noisy.
But that's not a big deal right now.
The verbosity is something to explore. I don't know what the minimal acceptable threshold is yet.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
### What does this PR do? Automatically enables a subset of actions to be enabled in the private action runner if its enabled ### Motivation We want some actions to be available out of the box ### Describe how you validated your changes Started the PAR and the "status" command without an explicit allowlist / with `private_action_runner.default_actions_enabled: false` ### Additional Notes This is a behavior customers can opt out of by specifying `default_actions_enabled: false` in the config file Discussing with the team the exact list of actions we want to auto enable. Co-authored-by: gabriel.plassard <gabriel.plassard@datadoghq.com>
This reverts commit 6b7f049.
### What does this PR do? Update OpenSSL to 3.5.6 ### Motivation Keeping our dependencies up to date ### Describe how you validated your changes Local build for linux x86_64, the CI will check the rest ### Additional Notes Co-authored-by: hugo.beauzee <hugo.beauzee@datadoghq.com>
…49285) - comp/core/fxinstrumentation - comp/core/log - comp/core/lsof - comp/core/pid - comp/updater/daemonchecker/def - comp/updater/ssistatus/def - comp/updater/telemetry/def - pkg/pidfile ### Motivation Another of the large scale change to move the build to bazel. Co-authored-by: joseph.gette <joseph.gette@datadoghq.com>
### What does this PR do? Sequential PR after [#49112](#49112): this branch is based on that L9/L10 Bazel work, so the Git diff may include those commits until `main` contains them. **The new work in this PR is the L11 migration only.** - Remove root `gazelle:exclude` for `comp/core/config`, `comp/core/secrets/impl`, `pkg/trace/stats`, `pkg/trace/config`, `pkg/trace/sampler`, `pkg/trace/semantics`, `pkg/trace/version`, and `pkg/trace/watchdog`; replace the `pkg/trace` root stub with a Gazelle-generated `go_library` on `doc.go`. - Add Gazelle-generated `BUILD.bazel` for all of the above packages. - `comp/core/config`: `gotags = ["test"]` on `config_test` so `//go:build test` mock helpers (`NewMock`, `NewMockFromYAMLFile`, etc.) compile. - `comp/core/secrets/impl`: `gotags = ["test"]` on `impl_test` for the same reason (`testutil.IsolatedGoBuildCmd`, `filesystem.SetCorrectRight`, `telemetryimpl.NewMock`). - `pkg/trace/watchdog`: `gotags = ["test"]` on `watchdog_test` for `log.NewBufferLogger`. - `deps/go.MODULE.bazel` updates from `bazel mod tidy` for the new module wiring. #### Go code changes - **`comp/core/secrets/impl/fetch_secret_test.go`**: `getBackendCommandBinary` now detects Bazel via `TEST_SRCDIR` and uses the pre-built `test_command` `go_binary` from runfiles instead of invoking `go build` at runtime (which fails because `go` is not on PATH in Bazel's sandbox). The binary is copied to a temp dir with `0700` permissions via `filesystem.SetCorrectRight` to satisfy the agent's secret-backend permission checks. The existing `go test` path is unchanged. - **`pkg/trace/watchdog/info_test.go`**: `doTestMemHigh` adds 0.1% slack to the `MemStats.Alloc` delta assertion. The test is inherently racy — `Alloc` tracks live bytes, so small objects freed by concurrent GC between the two `Mem()` calls can reduce the delta below the exact allocation size. Under Bazel the different binary layout makes this flake consistently (~200-300 bytes short on a 10M allocation). ### Motivation [ABLD-431](https://datadoghq.atlassian.net/browse/ABLD-431) — migrate layer-11 modules to Bazel after L9/L10 land via #49112. ### Describe how you validated your changes Please rely on CI and local `bazel test` on the touched targets. ### Additional Notes - **Merge order:** land #49112 first, then this PR (or rebase this branch onto `main` after #49112 merges to shrink the diff to L11-only). - If `bazel run //:gazelle` is re-run, keep `# keep` on manual `gotags` and `data` lines per the migration rule. [ABLD-431]: https://datadoghq.atlassian.net/browse/ABLD-431?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: aiuto <tony.aiuto@datadoghq.com> Co-authored-by: joseph.gette <joseph.gette@datadoghq.com>
### What does this PR do? It further cleans up variables in sysconfigdata file produced by the Python build such that sandbox paths as well as other output root (`bazel-out/...`) paths are replaced by placeholders into which later the final install location can be injected (which it is done using a slightly tweaked replace_prefix rule for now). It also adds a test covering some of the previous replacement logic as well as the new one. ### Motivation Spotted while looking at [this](#48184 (comment)). I realized, looking at the file, that there were lots of references that don't make sense to keep. This is mostly dead weight, so we may as well claim those bytes back, and it's also a reproducibility concern. ### Describe how you validated your changes - Passing tests. - Passing integrations-core test pipeline against a build from this branch: https://github.com/DataDog/integrations-core/actions/runs/24349158368 ### Additional Notes Rather than be selective and list variables manually (there are lots of them), this change leverages the knowledge of sandbox build dir used by rules_foreign_cc as well as a basic heuristic around `bazel-out` to replace paths throughout. The current choice of replace_prefix is merely tactical – it's less of a jump than trying to use the `install_dir` config flag value. We may end up replacing this as we refine our strategy around the whole path replacement/patching business in general. Co-authored-by: alex.lopez <alex.lopez@datadoghq.com>
### What does this PR do? Nothing more than what is indicated in the title ### Motivation ### Describe how you validated your changes ### Additional Notes Co-authored-by: kevin.fairise <kevin.fairise@datadoghq.com>
### What does this PR do? Adds fakeintake endpoints to simulate the PAR OPMS backend (dequeue, publish, health-check, heartbeat) and test control endpoints (enqueue, result, flush, stats). Also adds Go client methods and fixes the CI rule so `FAKEINTAKE_IMAGE_OVERRIDE` is set when fakeintake changes. ### Motivation Prerequisite for #49065 (PAR K8s e2e tests). Once merged, `fakeintake:latest` will have the PAR endpoints and #49065 can rely on it without image overrides. ### Describe how you validated your changes Fakeintake module compiles cleanly. CI will run `new-e2e-privateactionrunner` with the new image. ### Additional Notes Co-authored-by: valeri.pliskin <valeri.pliskin@datadoghq.com>
…9132) ## Summary - Add `TestGetBool` to `runtime_settings_test.go` — `GetBool` had no tests despite `GetInt` being covered - Add `runtime_profiling_test.go` with `TestExecWithRuntimeProfilingSettings` covering no-op, set-and-restore, get-error, and set-error paths using an inline `mockClient` ## Test plan - [x] `dda inv test --targets=./pkg/config/settings` — all 11 tests pass - [x] `dda inv linter.go --targets=./pkg/config/settings` — 0 issues Closes AGENTCFG-59. _Created by [Auto-JIRA](https://github.com/DataDog/datadog-agent/blob/main/.claude/skills/auto-jira/SKILL.md)._ Co-authored-by: rahul.kaukuntla <rahul.kaukuntla@datadoghq.com>
### What does this PR do?
Decouples the CWS (Cloud Workload Security) consumer and the SBOM event monitor by introducing two new types:
- CommandServer: extracts the shared gRPC command server from CWSConsumer, so it can be reused by both CWS and SBOM without coupling.
- UsageConsumer: a new, minimal event consumer that starts the security probe (including the SBOM resolver) without the CWS rule engine. It is registered
when runtime_security_config.sbom.enabled = true but runtime_security_config.enabled = false.
- SBOMAPIServer: moves the SBOM gRPC server implementation (GetSBOMStream, collectSBOMS) out of APIServer into its own type, registered only by
UsageConsumer.
Additionally, system-probe is now enabled when sbom.enabled is set, independent of CWS.
### Motivation
Customers using SBOM enrichment (e.g., container image scanning) had to also enable CWS, because the SBOM resolver was embedded inside CWS. This PR makes SBOM a standalone capability that can run independently.
### Describe how you validated your changes
- Verified the event monitor initializes UsageConsumer when only sbom.enabled = true and CWSConsumer when runtime_security_config.enabled = true
- Verify we do not have ruleset_loaded events and metrics sent when only enabling SBOM resolver.
### Additional Notes
Co-authored-by: sylvain.baubeau <sylvain.baubeau@datadoghq.com>
### What does this PR do? Compatibility attribute was missing for `include_header` and `integrity` go code generators so we were triggering those on Windows as well, even though there is no need for that as all of the .go sources in the corresponding bazel packages are guarded with `linux_bpf` build tag. So it wasn't harmful but was indeed wasteful work. Co-authored-by: joseph.gette <joseph.gette@datadoghq.com>
Go Package Import DifferencesBaseline: 141f120
|
| "hopper": {nvml.DEVICE_ARCH_HOPPER, 9, 0}, | ||
| "ada": {nvml.DEVICE_ARCH_ADA, 8, 9}, | ||
| "blackwell": { | ||
| nvml.DeviceArchitecture(10), // nvml.DEVICE_ARCH_BLACKWELL in newer go-nvml |
There was a problem hiding this comment.
Weird merge? This change is already on main
| // Resolve ENC[] secrets after dogtelextension config is applied so that | ||
| // secret_backend_command set via extensions.dogtel is visible here. | ||
| // Check both secret_backend_command (custom script) and secret_backend_type | ||
| // (native backend via secret-generic-connector, e.g. aws.secrets, k8s.secrets). | ||
| if pkgconfig.GetString("secret_backend_command") != "" || pkgconfig.GetString("secret_backend_type") != "" { | ||
| secretResolver := secretsimpl.NewEnabledResolver(noopsimpl.GetCompatComponent()) | ||
| if resolveErr := pkgconfigsetup.ResolveSecrets(pkgconfig, secretResolver, "agent_config"); resolveErr != nil { | ||
| return nil, fmt.Errorf("failed to resolve secrets: %w", resolveErr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Looks like bad merge? These few lines are merged to mainline already
| return ctx.Err() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
what is the reason for having this change as part of the bazel migration? Not opposed to it, but would like to better understand the rationale.
|
Bad merge. starting fresh |
What does this PR do?
Adds new flags for
dda inv testto allow users to use bazel for tests that have been migrated.This is a first cut implementation. I need to hear suggestions and problems from real users who are willing to try it.
--skip-bazel-tests
Queries Bazel for all go_test targets within the same scope as the current test run, then excludes those packages from go test. Each package is tested by exactly one system.
--save-bazel-tests=
Writes the list of Bazel-covered test targets (one label per line) to without changing which tests run. The intended future use is from gitlab jobs, where we may want to call dda and bazel separately.
--run-bazel-tests
After go test completes, runs the Bazel-covered targets with bazel test --keep_going. Results are printed in the same manner as the go tests.
EXPERIMENTAL_USE_BAZEL_TESTS environment variable
If this is set to a non-empty, non-zero value, dda test will implicitly set --skip-bazel-tests, and --run-bazel-tests. This will allow users to easily add themselves to a forward group of testers.
Motivation
Speed up
dda testinvocation on the command line by taking advantage of bazel test caching.Describe how you validated your changes
Playing with it at the command line. We need some flight time with real users to find more requirements and what the behavior should really be like.
Open questions for the future