Skip to content

fix: record edge_prefilter_decisions_total at the policiesActiveFor fork - #295

Open
detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-record-edge-prefilter-decisions-total-at-the-p-ebe160
Open

detail-app[bot] wants to merge 1 commit into
masterfrom
detail/bug-fix/fix-record-edge-prefilter-decisions-total-at-the-p-ebe160

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 16, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Fixes ENG-696

Bug

edge_prefilter_decisions_total{route_kind,decision} was permanently 0 on HEAD. PR #187 deleted src/proxy/server.zig, which held the only production caller of RuntimeMetrics.recordPrefilterDecision, and the replacement policiesActiveFor-based frontends never re-wired the metric. The counter still scraped as well-formed (initializeStaticSeries seeds all 24 zero series at boot), so it silently degraded from correct (pre-v1.20.0) to identically zero with no error signal, removing operator visibility into how often the edge bypasses the policy engine.

Fix

Re-wire recordPrefilterDecision at every policiesActiveFor fork so each piped request records exactly one decision:

  • src/frontend/exec.zig (processBuffered, shared by both frontends' buffered pipes) — records fast_path/policy_path; adds routeKindLabel(signal, format) and bufferedRouteKindLabel(kind) mappers that derive RouteKindLabel from the existing routeLabel/bufferedRouteLabel (mirroring the pre-feat: use latest policy libs and upgrade to 0.16.0 #187 toRouteKindLabel).
  • src/frontend/httpz/server.zig and src/frontend/stdio/conn.zig (execPipeStream in each) — record on both branches of the fork.

Decision maps 1:1 to policiesActiveFor's boolean (falsefast_path, truepolicy_path). By-design-zero dimensions (.none, health, passthrough, prometheus_metrics) stay zero, matching the pre-regression behavior for piped traffic.

Testing

  • Unit tests: 5 added (counter machinery, the two mappers, and two processBuffered wiring guards that drive a real request and assert the counter increments); zig build test = 524 pass / 1 skip / 0 fail. Verified the two wiring tests fail on an unwired tree (proving they're genuine regression guards, not tautologies).
  • Build/lint: zig build and zig build -Doptimize=ReleaseSafe exit 0; zig fmt --check and ziglint clean.
  • End-to-end (httpz): started edge against an echo upstream with a no-policy config then a keep-all log+metric file. Confirmed datadog_logs/otlp_logs/datadog_metrics increment on fast_path with no policy and on policy_path with one loaded; sibling edge_policy_records_evaluated_total increments on the policy path; no double-count; .none/health/passthrough/prometheus_metrics stay 0.
  • End-to-end (stdio): re-ran the streaming paths (/api/v2/logs, /v1/logs) on a stdio-frontend build; increments match httpz (parity).
  • No-regression smoke: edge_requests_total/_responses_total/_duration_seconds/_upstream_attempts_total all increment as before.
  • Fail-open: invalid JSON to /api/v2/series (buffered) still returns 202 raw-forwarded, and datadog_metrics,policy_path is recorded before the transform fails (the recorded decision is not retracted).
  • datadog-log-bench --list builds; upstream-pool-harness passes 10/10 against the fixed edge (retry/eviction unchanged).

Could not run

  • stdio buffered e2e (/api/v2/series): the stdio-frontend buffered path crashes in execPipeBufferedopenUpstream (conn.zig:182) on an unreachable. Reproduced identically on an unmodified baseline build, so it is a pre-existing stdio bug outside this fix's scope (the stdio change here is only in execPipeStream). The buffered wiring is shared exec.processBuffered, covered by the httpz e2e and the unit wiring test.
  • task test:s3-e2e (MinIO): the environment's Docker registry allows only the official library/* namespace (alpine/hello-world pull; minio/minio is denied with "pull access denied"), so the MinIO container the task starts cannot be pulled. The s3-dump code path is untouched by this fix.

Automatic Fixes PRs can be configured here.

@macroscopeapp

macroscopeapp Bot commented Sep 16, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved at 27158b5

Macroscope's review found this PR approvable — This is a localized observability bug fix that records one existing metric at each prefilter decision without changing forwarding or policy-processing behavior. The added tests cover buffered and streaming paths, route labels, both decisions, and double-count prevention.

You can add or adjust custom eligibility rules. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant