inject observer metrics for rate limiting

[ajtmccarty]

Why

The API backpressure layer wired its Prometheus sinks by handing components a mutable observer slot
after construction — PrioritySlotPool.set_observer(), RetryAfterPolicy.set_observer(), and a
ReferenceQueryLoadTracker.set_observer() called from the tracker's own module. That inverted the
dependency direction three ways:

• The primitives owned their sinks. database/load_signal.py imported database/metrics.py at
  module level, so importing the stress-signal primitive the admission gate codes against pulled
  Prometheus gauge registration into the import chain.
• Consumers mutated injected collaborators. AdmissionController.__init__ reached back into the
  pool and the policy it had just been handed to install callbacks defined in controller.py. The
  pool called into its consumer at runtime while the consumer imported it at compile time, and
  the wiring was invisible at the application entry point.
• One sink per component. A single mutable slot meant two controllers sharing a pool silently
  clobbered each other's observer, and no component could have a second sink.

Goal: every component that feeds a metric receives its sinks as a required constructor argument
and pushes values to them, so the concrete sinks are named only where the object graph is wired and
Prometheus stays out of the primitives' import chain.

Non-goals: no change to the shed decision itself — the tiers, thresholds, CoDel behaviour, slot
accounting, Retry-After computation, and every metric name and label are unchanged. This is a
dependency-direction refactor plus configuration hardening, not a behaviour change.

What changed

Behavioral changes

• Non-finite (inf) values are now rejected for every backpressure float setting. Previously
  inf passed the gt=0/ge=1 bounds and then crashed startup with OverflowError inside
  int(base * multiplier), disabled window eviction entirely (horizon = now - inf), or silently
  exempted a priority class from shedding (ratio / inf → tier 0).
• The backstop multipliers and shed-stress ratios must now be ordered by priority. A misordered
  config used to invert the whole feature — HIGH backstop-rejected while LOW kept queueing, or HIGH
  shedding at a lower stress ratio than LOW — and startup accepted it silently.
• Worker processes now honour backpressure_stress_window_seconds. The window was applied by
  mutating the tracker inside the admission factory, which only runs on API servers, so any process
  without an admission gate kept the hard-coded 20s default and published stress gauges computed
  over the wrong window.
• Observer failures are contained per observer, on all three components. A raising sink can no
  longer corrupt admission state, shed a request that would have been admitted, fail the database
  query that fed an observation, or skip the sinks behind it.

Implementation notes

• Four observer protocols, one per component: AdmissionObserver (four events — offered, admitted,
  rejected, sojourn), SlotPoolObserver, RetryPolicyObserver, LoadSignalObserver. Each pushes
  values as arguments, so a sink never reads back into the component it observes. AdmissionObserver
  is one interface with four events rather than one per metric, because they describe a single
  request's passage through one decision.
• Concrete sinks live in api/admission/observers.py and database/load_signal_metrics.py. Nothing
  under api/admission/ outside the entry point imports them: server.py passes them to
  build_admission_controller, which takes them as arguments rather than choosing them.
• build_admission_controller moved out of controller.py into a new factory.py. That leaves the
  decision logic importing nothing from its own package at runtime.
• The process-global tracker moved out of load_signal.py into load_signal_registry.py, which
  reads the window from settings at construction. window_seconds is now a read-only property —
  resizing a live window would reinterpret the samples already in it.
• RejectionReason became a StrEnum in constants.py, so the three reason strings exist in one
  place and both the controller and the sink import the type from a leaf module.

What stayed the same

• Metric names, labels, and semantics — infrahub_admission_* and infrahub_db_reference_query_*
  are byte-for-byte identical, so dashboards and alerts are unaffected.
• The 429 response envelope, Retry-After values, excluded paths, and the CORS-preflight bypass.
• Default values for every setting. The new constraints only reject configurations that were already
  incoherent.

How to review

Suggested order:

1. slot_pool.py, retry_policy.py, load_signal.py — the primitives. Each gained a protocol and a
   required observers argument, and lost its set_observer. Note the failure guard sits inside
   the per-observer loop; moving it outside would silently downgrade isolation from per-observer to
   per-event.
2. controller.py — largest diff. Six inline metric calls became four _observe_* methods over the
   injected list, each guarding per observer via the _contained_observer_failure() context manager.
3. factory.py, server.py, load_signal_registry.py — the wiring, and the only places that name a
   concrete sink.
4. config.py — the new constraints and the validate_shed_stress_ratios_ordered_by_priority
   validator.
5. Tests. Recording and failing adapter sinks live in tests/unit/api/admission/helpers.py; no
   mocks.

Worth extra scrutiny:

• middleware.py:94 still increments MISSING_PRIORITY_TOTAL directly. It is the one metric in
  the package written outside a sink — deliberate, since it fires during header parsing before a
  controller is involved, but it means the package is not 100% consistent.
• get_reference_query_load_tracker() now raises InitializationError if settings are unloaded.
  Unreachable in practice (the tracker is only reached from the reference query, which requires a
  configured database), and chosen over a silent fallback that would diverge from config.
• inf is no longer a way to express "never shed this class." A large finite ratio still is. If
  that needs to be first-class, it deserves an explicit representation.

Checklist

• Tests added/updated
• Changelog entry — n/a, API backpressure is unreleased and this refactors it before ship
• External docs updated — n/a, the generated config reference is unchanged (constraints do not
  appear in it)
• Internal .md docs updated (dev/knowledge/backend/api-backpressure.md)
• I have reviewed AI generated content

---

Summary by cubic

Inject metrics via observers across the rate limiting path to decouple Prometheus sinks from admission logic and keep failures contained. Moves controller wiring to a factory, shares the DB load-signal via a registry, and tightens backpressure config validation.

• Refactors
  • Introduced observer interfaces: AdmissionObserver, RetryPolicyObserver, SlotPoolObserver, LoadSignalObserver.
  • Added metric sinks: AdmissionMetricsObserver, SlotPoolMetricsObserver, SustainedLoadMetricsObserver, LoadSignalMetricsObserver.
  • AdmissionController, PrioritySlotPool, RetryAfterPolicy, and ReferenceQueryLoadTracker now take observers and isolate sink failures.
  • Moved build_admission_controller to factory.py; server.py wires observers at startup.
  • New load_signal_registry.py provides the shared tracker; load_signal_metrics.py publishes its gauges; window length comes from settings.
  • Replaced string reasons with RejectionReason enum (used as the reason metric label).
  • stress_shed_fraction() now takes a tier; tests updated accordingly.
  • Backpressure settings guarded: shed stress ratios must be ordered by priority, non‑finite floats are rejected, and backstop multipliers are bounded (low ≤ 1 ≤ high).

^{Written for commit 333ad80. Summary will update on new commits.}

[codspeed-hq]

Merging this PR will not alter performance

✅ 12 untouched benchmarks

---

_{Comparing ajtm-rate-limiting-metric-injection (333ad80) with develop (ea6e4cd)}

[cubic-dev-ai]

4 issues found across 22 files

Confidence score: 3/5

• In backend/infrahub/api/admission/factory.py, non-finite backstop multipliers can make admission-controller construction fail during server startup, creating a concrete availability risk from otherwise valid-looking config overrides — add ApiSettings validation to reject NaN/inf values before conversion.
• In backend/infrahub/api/admission/factory.py, missing cross-field ordering checks for HIGH/LOW multipliers and thresholds can invert shedding priority so HIGH traffic is rejected before LOW under pressure, which undermines expected service behavior — enforce LOW ≤ MID ≤ HIGH (or the intended ordering) at settings validation time.
• In backend/infrahub/database/load_signal_registry.py, the docstring promises an InitializationError path that the function does not implement, so callers may rely on error handling that never triggers and diagnostics become confusing — either implement the documented exception flow or update the docstring to match runtime behavior.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/infrahub/api/admission/factory.py">

<violation number="1" location="backend/infrahub/api/admission/factory.py:56">
P2: An infinite high or low backstop multiplier can make server startup fail while the admission controller is built. The corresponding `ApiSettings` fields could reject non-finite values before this conversion, matching the existing validation on `backpressure_max_concurrency_factor`.</violation>

<violation number="2" location="backend/infrahub/api/admission/factory.py:58">
P2: Positive but improperly ordered backstop multipliers can cause HIGH traffic to be rejected before LOW traffic under queue pressure. Cross-field validation could preserve the intended priority relationship (`high_multiplier >= 1 >= low_multiplier`, or an equivalent high-capacity ordering).</violation>

<violation number="3" location="backend/infrahub/api/admission/factory.py:61">
P2: A valid-looking settings override can reverse priority shedding: HIGH requests begin shedding at a lower stress ratio than LOW requests when the thresholds are not ordered. Cross-field validation could enforce LOW ≤ MEDIUM ≤ HIGH before this map is constructed.</violation>
</file>

<file name="backend/infrahub/database/load_signal_registry.py">

<violation number="1" location="backend/infrahub/database/load_signal_registry.py:20">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**

The docstring claims this function raises `InitializationError` if the configuration has not been loaded yet, but the implementation never raises, imports, or defines that exception—it simply accesses `config.SETTINGS.api.backpressure_stress_window_seconds` directly. This fabricated documentation will mislead callers about what errors to handle. Either add the guard and raise the exception, or remove the `Raises` block from the docstring.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Positive but improperly ordered backstop multipliers can cause HIGH traffic to be rejected before LOW traffic under queue pressure. Cross-field validation could preserve the intended priority relationship (high_multiplier >= 1 >= low_multiplier, or an equivalent high-capacity ordering).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/api/admission/factory.py, line 58:

<comment>Positive but improperly ordered backstop multipliers can cause HIGH traffic to be rejected before LOW traffic under queue pressure. Cross-field validation could preserve the intended priority relationship (`high_multiplier >= 1 >= low_multiplier`, or an equivalent high-capacity ordering).</comment>

<file context>
@@ -0,0 +1,99 @@
+    backstop_max_waiters = {
+        Priority.HIGH: max(1, int(base_backstop * settings.api.backpressure_backstop_high_multiplier)),
+        Priority.MEDIUM: base_backstop,
+        Priority.LOW: max(1, int(base_backstop * settings.api.backpressure_backstop_low_multiplier)),
+    }
+    stress_thresholds = {
</file context>

[ajtmccarty]

addressed with new setting validation

[cubic-dev-ai]

P2: A valid-looking settings override can reverse priority shedding: HIGH requests begin shedding at a lower stress ratio than LOW requests when the thresholds are not ordered. Cross-field validation could enforce LOW ≤ MEDIUM ≤ HIGH before this map is constructed.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/api/admission/factory.py, line 61:

<comment>A valid-looking settings override can reverse priority shedding: HIGH requests begin shedding at a lower stress ratio than LOW requests when the thresholds are not ordered. Cross-field validation could enforce LOW ≤ MEDIUM ≤ HIGH before this map is constructed.</comment>

<file context>
@@ -0,0 +1,99 @@
+        Priority.LOW: max(1, int(base_backstop * settings.api.backpressure_backstop_low_multiplier)),
+    }
+    stress_thresholds = {
+        Priority.HIGH: settings.api.backpressure_shed_high_stress_ratio,
+        Priority.MEDIUM: settings.api.backpressure_shed_medium_stress_ratio,
+        Priority.LOW: settings.api.backpressure_shed_low_stress_ratio,
</file context>

[ajtmccarty]

addressed with new setting validation and tests

[cubic-dev-ai]

P2: An infinite high or low backstop multiplier can make server startup fail while the admission controller is built. The corresponding ApiSettings fields could reject non-finite values before this conversion, matching the existing validation on backpressure_max_concurrency_factor.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/api/admission/factory.py, line 56:

<comment>An infinite high or low backstop multiplier can make server startup fail while the admission controller is built. The corresponding `ApiSettings` fields could reject non-finite values before this conversion, matching the existing validation on `backpressure_max_concurrency_factor`.</comment>

<file context>
@@ -0,0 +1,99 @@
+
+    base_backstop = settings.api.backpressure_backstop_max_waiters
+    backstop_max_waiters = {
+        Priority.HIGH: max(1, int(base_backstop * settings.api.backpressure_backstop_high_multiplier)),
+        Priority.MEDIUM: base_backstop,
+        Priority.LOW: max(1, int(base_backstop * settings.api.backpressure_backstop_low_multiplier)),
</file context>

[ajtmccarty]

addressed with new settings validation and test

[cubic-dev-ai]

P2: Custom agent: Flag AI Slop and Fabricated Changes

The docstring claims this function raises InitializationError if the configuration has not been loaded yet, but the implementation never raises, imports, or defines that exception—it simply accesses config.SETTINGS.api.backpressure_stress_window_seconds directly. This fabricated documentation will mislead callers about what errors to handle. Either add the guard and raise the exception, or remove the Raises block from the docstring.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/database/load_signal_registry.py, line 20:

<comment>The docstring claims this function raises `InitializationError` if the configuration has not been loaded yet, but the implementation never raises, imports, or defines that exception—it simply accesses `config.SETTINGS.api.backpressure_stress_window_seconds` directly. This fabricated documentation will mislead callers about what errors to handle. Either add the guard and raise the exception, or remove the `Raises` block from the docstring.</comment>

<file context>
@@ -0,0 +1,30 @@
+    rather than at import — keeps importing this module free of side effects, and defers the
+    settings read until the configuration is loaded.
+
+    Raises:
+        InitializationError: If the configuration has not been loaded yet.
+
</file context>

[ajtmccarty]

this can raise an initialization error indirectly if the config is not loaded yet

[cubic-dev-ai]

That still doesn’t justify documenting InitializationError specifically. If config can fail indirectly here, the docstring should name the real failure mode or remove the Raises block.

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Shadow auto-approve: would not auto-approve. Auto-approval blocked by 3 unresolved issues from previous reviews.

Re-trigger cubic</sub>

[polmichel]

LGTM. Clean DIs and test suites

[dgarros]

thank you for improving this part of the code, it looks much better

what are we missing in the current rules / guidelines to ensure that Claude will do a better job next time ?

[ajtmccarty]

@dgarros

what are we missing in the current rules / guidelines to ensure that Claude will do a better job next time ?

tried to add something that covers the changes from this PR to the docs in #10065. as I say in that PR, not sure if a guide would be better or not