feat(autoinstrumentation): compile SSI targets to policies and wire the mutator to the policy engine#52489
feat(autoinstrumentation): compile SSI targets to policies and wire the mutator to the policy engine#52489iamluc wants to merge 6 commits into
Conversation
|
Files inventory check summaryFile checks results against ancestor aced8ce7: Results for datadog-agent_7.82.0~devel.git.180.5acd305.pipeline.120124111-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
22 successful checks with minimal change (< 2 KiB)
|
415b90f to
adfa114
Compare
Add a behavioral characterization (golden master) of the target matcher, exercised exclusively through the public TargetMutator API (NewTargetMutator + getMatchingTarget) so the same suite runs unchanged against the current label-selector implementation. It pins first-match-wins precedence, pod matchLabels (ANDed), pod matchExpressions (In/NotIn/Exists/DoesNotExist, including the "absent key matches NotIn" rule), namespace matchNames/matchLabels/ matchExpressions, combined namespace+pod selectors, the empty-target-matches-all case, and disabled-namespace short-circuiting. These tests are added before the policy-engine refactor so the refactor can be shown to preserve matching behavior.
…licies Add the shared github.com/DataDog/dd-policy-engine/go evaluator as a dependency and lower the static "targets" SSI configuration into its policy model (policiesFromTargets). This is the static-configuration counterpart of the remote-config dd-wls parser: both produce policies.Policy, the single shape the matcher understands, so Kubernetes SSI targeting shares one rule semantics with the host injector. The dependency is pinned to a pseudo-version of the dd-policy-engine PR branch (DataDog/dd-policy-engine#71); repin to the released version once that PR merges. golang.org/x/sys is bumped 0.45.0 -> 0.46.0 as required by the dependency's go.mod. Only the target->policy compiler lands here; the mutator wiring and the RC subscription follow in a separate PR.
…ator Wire the target->policy compiler into the TargetMutator: configuration targets are lowered once into policies at construction time and pod matching is delegated to the native policy engine (policyMatcher) instead of k8s label selectors. The matcher and internal targets stay aligned by index, so the first matching policy resolves directly to its injection config, preserving first-match-wins semantics. NewTargetMutator keeps its current signature; no remote-config path is wired here (that follows in a separate PR), so callers and existing tests are unchanged. The characterization suite added earlier passes unchanged against this implementation. One intentional behavior change is pinned in target_matching_change_test.go: when a pod's namespace is absent from the workloadmeta store and a target reads namespace labels, the matcher now declines injection consistently (the legacy matcher was order-dependent and could still match an earlier pod-only target).
…f aborting Evaluate policies strictly in order (first match wins). When a pod's namespace is absent from the workloadmeta store, a rule that reads namespace labels cannot be evaluated; ignore only that rule and keep evaluating the rest, so an unresolvable namespace rule never prevents an otherwise-matching rule from injecting. This replaces the legacy "abort all matching" fail-safe, which declined injection entirely as soon as it reached a rule whose namespace could not be resolved.
Add com_github_datadog_dd_policy_engine_go to the go_deps use_repo list (via `bazel mod tidy`) so the new dependency is exposed to Bazel. Without it, `bazel mod tidy` reports an out-of-date use_repo list and CI fails the `git diff --exit-code` check.
dfd9ed0 to
b92303a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b92303aaa4
ℹ️ 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".
| "com_github_datadog_datadog_go_v5", | ||
| "com_github_datadog_datadog_operator_api", | ||
| "com_github_datadog_datadog_traceroute", | ||
| "com_github_datadog_dd_policy_engine_go", |
There was a problem hiding this comment.
Commit the Bazel module lock update
bazel/codereview_guideline.md requires MODULE.bazel.lock to be committed whenever a module extension invocation changes. This adds a new go_deps repo exposure for the new Go module, but the commit leaves MODULE.bazel.lock untouched, so Bazel module resolution can run with stale lock state; please run bazel mod tidy/the repo’s Bazel dependency update flow and commit the lockfile change.
Useful? React with 👍 / 👎.
| github.com/DataDog/datadog-go/v5 v5.8.3 | ||
| github.com/DataDog/datadog-operator/api v0.0.0-20260515125012-8e158b708444 | ||
| github.com/DataDog/datadog-traceroute v1.0.17 | ||
| github.com/DataDog/dd-policy-engine/go v0.0.0-20260619124759-08b73482a87e |
There was a problem hiding this comment.
Regenerate the third-party license manifest
Adding this Go module also changes go.sum, but LICENSE-3rdparty.csv was not updated. The GitLab lint_licenses job runs dda inv -- -e lint-licenses, and tasks/go.py compares that CSV against the vendored dependency set from go.sum, so this dependency addition will leave the license manifest stale until dda inv generate-licenses is run and committed.
Useful? React with 👍 / 👎.
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: aced8ce Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_idle | memory utilization | +0.15 | [+0.08, +0.21] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | +0.11 | [+0.07, +0.15] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | +0.06 | [-1.02, +1.14] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | -0.07 | [-0.32, +0.18] | 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 | 3 ≤ 4 | bounds checks dashboard |
| ✅ | quality_gate_idle | memory_usage | 10/10 | 143.56MiB ≤ 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 577.95KiB ≤ 819.20KiB | 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 | 479.99MiB ≤ 495MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 0.89MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 4 ≤ 6 | bounds checks dashboard |
| ✅ | quality_gate_logs | memory_usage | 10/10 | 180.87MiB ≤ 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.21MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 325.24 ≤ 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 | 390.72MiB ≤ 430MiB | 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.86GiB ≤ 1.04GiB | 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".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
CI Pass/Fail Decision
✅ Passed. All Quality Gates 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_metrics_logs, bounds check intake_connections: 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 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 total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle, 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 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 total_bytes_received: 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_idle_all_features, bounds check intake_connections: 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: 5acd3052e4
ℹ️ 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".
| "com_github_datadog_datadog_go_v5", | ||
| "com_github_datadog_datadog_operator_api", | ||
| "com_github_datadog_datadog_traceroute", | ||
| "com_github_datadog_dd_policy_engine_go", |
There was a problem hiding this comment.
Commit the updated Bazel lockfile
Adding com_github_datadog_dd_policy_engine_go changes the go_deps Bzlmod extension output, but MODULE.bazel.lock was not updated and still contains no entry for this repo (checked with rg com_github_datadog_dd_policy_engine MODULE.bazel.lock). The Bazel review guideline in bazel/codereview_guideline.md requires committing the refreshed lockfile for module-extension changes; otherwise Bazel lockfile-mode checks/reproducible fetches can fail before this new dependency is available. Please run the Bazel dependency refresh and commit the resulting MODULE.bazel.lock update.
Useful? React with 👍 / 👎.
What does this PR do?
Evolves Kubernetes SSI targets matching from k8s label selectors to the
shared Go policy engine (
github.com/DataDog/dd-policy-engine/go), so thecluster-agent shares one rule semantics with the host injector.
Three commits, in order:
matcher, exercised only through the public
TargetMutatorAPI(
NewTargetMutator+getMatchingTarget). Added before the refactor so itruns unchanged against the current label-selector implementation.
dd-policy-engine/godependency and lowers thestatic
targetsconfig into policies (policiesFromTargets).TargetMutatormatches pods via the policy engine(
policyMatcher) instead of label selectors; targets and policies stayaligned by index, preserving first-match-wins.
Scope is intentionally minimal: no remote-config path is wired here (that
follows in a separate PR).
NewTargetMutatorkeeps its signature, so callersand existing tests are unchanged.
Matching coverage (characterization)
first-match-wins precedence · pod
matchLabels(ANDed) · podmatchExpressionsIn/NotIn/Exists/DoesNotExist (incl. "absent key matches NotIn") · namespace
matchNames/matchLabels/matchExpressions· combined namespace+pod (ANDed) ·empty-target-matches-all · disabled-namespace short-circuit.
The suite passes against both the legacy label-selector implementation
(verified on
main) and the policy-engine implementation, proving behavior ispreserved.
Intentional behavior change
target_matching_change_test.gopins one deliberate change: when a pod'snamespace is absent from the workloadmeta store and a target reads namespace
labels, the matcher now declines injection consistently (the legacy matcher
was order-dependent and could still match an earlier pod-only target). This test
is red on the legacy impl and green on the new one, documenting the change.
Describe how you validated your changes
Characterization tests were additionally run against
main(legacy impl) via agit worktree to confirm equivalence.
Additional Notes
branch (feat(go): native Go policy evaluator for Kubernetes SSI targeting dd-policy-engine#71, draft). Repin to the released version
once that PR merges — this PR is stacked on it and stays in draft until then.
golang.org/x/sysis bumped0.45.0 -> 0.46.0as required by thedependency's
go.mod(MVS).