Do not count short-circuited requests as search errors

🤖 AI-authored change.

What does this MR do and why?

Requests that short-circuit before a search is ever attempted are being counted as search errors, at a constant 100% error ratio. An authorization failure, a validation failure, an unsupported scope or a missing parameter is recorded as a failed search that never happened, and in the global_search SLI those populations are indistinguishable from genuine search failures.

The fix sets a flag immediately before the benchmarked search call on each path and guards the error recording on it: a request that never attempted a search is not a search. All four recording sites are fixed rather than only the API path the report came from, because they share the bug class. Requests that did search behave exactly as before, and no labels are added.

Measured on gprd, 30-day window: every degenerate population has apdex_total == 0 — not one request in 30 days reached the success path. This may let ZoektSearchErrorRateHigh's label scoping be dropped, pending a post-deploy measurement window.

Reviewer focus: whether guarding on an instance variable set at the call site is the right mechanism versus threading a return value out of the benchmark block — and whether fixing all four sites in one MR is the scope you want.

References

• Parent epic: &23480
• Originating issue: #627960
• Measurement and call-site detail: $6055219

Mechanism, citations and verification

Why now

Measured on gprd over a 30-day window, four zoekt search populations — global blobs, project wiki_blobs, project milestones and project issues — have apdex_total at exactly 0 while their total and error counts are equal, i.e. every request in 30 days short-circuited. gitlab-com/runbooks!11489 (closed) scopes ZoektSearchErrorRateHigh to search_scope="blobs" at search_level=~"project|group" specifically to exclude those rows; that scoping is a containment, not a fix, and can be dropped once this ships.

What changed

All four record_error_rate call sites in the search request path share the bug class, so all four are fixed: the API's forbidden!/bad_request! and disallowed-project-scope exits, SearchController#count's ParameterMissing, #autocomplete's blank term, and EE::SearchController#aggregations' invalid term. haml_search_results has no short-circuit above its benchmark, so the flag there only makes the invariant uniform.

Per-population figures and the per-site breakdown: $6055219

What is deliberately unchanged

Requests that did attempt a search behave exactly as before:

• An exception raised inside the benchmark block still leaves the duration nil and is still recorded as an error.
• A 4xx returned after a completed search (e.g. search_results.failed?) is still recorded as a non-error.
• The search_type / search_level / search_scope labels are untouched, so there is no cardinality change and no new label to plumb through the metrics catalog.

The alternative in #627960 — a new label distinguishing short-circuits — was not taken: it would add a dimension to every series to describe requests that carry no search-latency information at all.

Verification

At 45bfadbd90cf, spec/requests/api/search_spec.rb ran 28 examples and spec/controllers/search_controller_spec.rb 11 examples, 0 failures on both, with RuboCop clean on the five changed files. Reverting only the guard turns three API examples and two controller examples red, and the controller output shows the reverted behaviour recording error: true with all three labels nil — the degenerate shape the SLI has been carrying in production.

What I did not verify

• The gprd figures are read from Mimir queries, not reproduced end-to-end. I did not replay production traffic against the patched code, so the predicted post-fix series shape is inference from the query results, not a measurement of the new behaviour.
• I did not verify the fix in a live environment — all spec runs are local, and the head commit here is not the commit the runs were made at.
• I did not check every other record_error_rate caller outside the search request path (e.g. background/Sidekiq recorders) for the same short-circuit class.
• I did not confirm that dropping the ZoektSearchErrorRateHigh scoping after this ships is safe; that needs a post-deploy measurement window and belongs to the runbooks change, not this one.
• I did not measure whether any existing SLO dashboard or error-budget burn figure will visibly step when the degenerate populations stop being counted.

🤖 Automated change. Mention @johnmason for feedback, or reply #human to escalate to John.