Evaluate database_capture flag outside the query execution path

What does this MR do and why?

The Capture query analyzer and the adapter log patch each call Feature.enabled?(:database_capture) from inside the query execution/notification path. A cold-cache evaluation issues its own Flipper gate query, whose sql.active_record notification re-enters the flag check while the first evaluation is still on the stack, producing a self-recursion error; the same check can also run mid-configure_connection before the connection's type map is ready, producing nil type map errors. All three are handled, but they recur per instance and land as ERROR-level noise in exceptions_json.log and Sentry.

The flag is now evaluated once per request/job: QueryAnalyzers::Capture.enabled? includes Gitlab::Database::Capture.enabled?, so the framework samples it when analyzers are enabled at request/job start, outside the notification path, mirroring how PreventCrossDatabaseModification handles its own flag. The adapter patch gates on the analyzer's thread-local context instead of calling Feature.enabled? directly, and analyze no longer touches the flag at all. Net effect: the flag is now sampled once per request/job rather than per query, the same freshness class as Flipper's own per-process cache.

Resolves #628047 (closed).

How to set up and validate locally

  1. Run bin/rspec spec/lib/gitlab/database/query_analyzers/capture_spec.rb spec/lib/gitlab/patch/postgresql_adapter_returning_values_spec.rb — 18 examples, all green.
  2. Look at the new "with a real feature flag store" example: it drives the same chain as production with a real, unstubbed Flipper. Revert the lib changes and it fails with RecursionError: self recursion on a single INSERT ... RETURNING, confirming the regression test actually catches the bug.
Supporting evidence

On unmodified master, enabling the analyzer with a real Flipper (cold cache) and running one INSERT ... RETURNING reproduces RecursionError: self recursion exactly: patch flag check triggers Flipper gate SQL, which fires sql.active_record, which re-enters the analyzer's flag check. With this fix applied, the identical scenario tracks zero errors and the statement is still captured with its RETURNING values.

The type-map errors (NoMethodError: undefined method 'lookup' for nil, TypeError: wrong argument type nil (expected PG::TypeMap)) are timing races during connection setup and weren't reproduced live, but they share the same root cause — Feature/Flipper access from inside the notification path — which this change removes entirely. A new spec asserts analyze never evaluates the flag.

Checked for fallout: returned_values has no consumer other than the Capture analyzer, nothing else calls Gitlab::Database::Capture.enabled?, and no docs describe the old per-query behavior. Related suites also pass: spec/lib/gitlab/database/query_analyzer_spec.rb (39 examples) and spec/initializers/database_capture_spec.rb (2 examples). RuboCop is clean on all five changed files.

With the flag off, which is its default everywhere, runtime behavior is unchanged apart from the errors disappearing: the patch does a cheap thread-local nil check per query and the analyzer stays disabled.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Merge request reports

Loading
Loading