Repository navigation
fix(metrics): bound domain and method label cardinality - #527
Merged
Merged
Conversation
Path metrics reached 63% of PNF's Prometheus TSDB (5.55M of 8.76M series), taking it to 89% of its memory ceiling. The same registry growth was happening inside gateway pods: working set 0.775 -> 6.29 GiB in five days, peaking at 9.49 GiB against a 12 GiB limit. Two label sources leaked unbounded values. `domain` carried raw bech32 supplier addresses -- 4,172 of 4,608 distinct values -- making path_probation_events_total 27% of the entire TSDB. The guard that should have caught this is dead code: reputation/selector.go checks `if err != nil || domain == ""`, but ExtractDomainOrHost does not fail on a bare pokt1 address. A dotless host takes the isPrivateOrInternalDomain branch and is returned verbatim with a nil error, so the address arrives looking like a successful extraction. Fixing that inside ExtractDomainOrHost was rejected: it has 38 call sites and also feeds the operator concentration cap, admin drains, blocked_domains, and reputation keys. Changing it would change endpoint selection. SanitizeDomainLabel sits at the metrics label boundary instead, where it cannot affect routing. Its bech32 test is gated on "no dot" so it can never fire on a real domain: isBech32Like requires the post-`1` remainder to be entirely alphanumeric, and every registrable domain has a dot there. `method` is attacker-controlled -- it carries the JSON-RPC method name or the REST URL path, so any unauthenticated client can mint a series per request. SanitizeMethodLabel already existed and was already wired, which is why this was missed: it bounds the SHAPE of a value but not the SET. Route-shaped segments pass through verbatim, so /aaa, /aab, ... are unbounded. Test_ObservationPipeline_AttackerMethodsAreBounded pins this down -- with the guard reverted, 5,000 probe paths survive the sanitizer untouched. Only the cardinality guard bounds it. The guards matter for pod heap specifically because of eviction, not the cap. promauto counters never evict cold series, so a cap alone freezes the registry without shrinking it; withEviction + DeleteLabelValues is what returns heap without a rolling restart. Changes: - SanitizeDomainLabel on all 18 Record* and 2 Set* helpers that take a domain, plus four sites that bypass those helpers. Two of them, SelectionCandidateTotal and SelectionSelectedTotal, are counters with no Reset(), so leaked operator keys were retained for the pod's lifetime. - Cardinality guards on probation_events_total, observation_pipeline_total, circuit_breaker_events_total and rpc_type_fallback_total. - Dropped the `supplier` label from rpc_type_fallback_total. It carried 3,289 values against the metric's own 9 domains x 12 service_ids. The argument is kept and ignored so no caller changes, matching the existing RecordHealthCheck pattern. - normalizeRESTPath now maps any segment outside the RFC 3986 unreserved set to :var and caps path depth, so injection payloads never reach a label. Real Cosmos routes are unaffected. Sanitization runs after RecordCircuitBreakerEvent's empty-domain check, not before: SanitizeDomainLabel maps "" to DomainUnknown, which would silently defeat the documented skip. All five regression tests assert through the production emit helper and read the label values the collector actually received, and each was revert-checked to confirm it fails without its fix.
…l checks Unrelated to the cardinality fix; carried on this branch for convenience. moonbeam, moonriver and celo ran health checks at 10s intervals while health checks were 78%, 71% and 87% of each service's total relays respectively (measured 2026-08-07; celo was the worst ratio on the fleet at 22.9 HC/s against 3.3 user relays/s). Per-check rate is ~98/interval_seconds and is flat in endpoint count, so the interval is the only real lever. A 10s detection window buys nothing at that traffic level. moonbeam_archival and celo_archival returned minor_error on 100% of runs. Both expected balances were re-verified as CORRECT against rpc.api.moonbeam.network and forno.celo.org, so this was genuine archival-supply absence rather than a stale rule value. A penalty applied uniformly to every endpoint distorts no ranking and alerts nobody, so each spent a third of its service's health-check budget for zero signal. gnosis_archival (66% fail / 34% ok) and eth_archival (14% fail) are kept -- those discriminate between endpoints. Both removed checks are commented with their address and block so they can be restored if archival supply appears.
Clears Dependabot alert GHSA-hrxh-6v49-42gf (high). No known exposure in PATH. The advisory bundles three issues, and all three require a configuration PATH does not run: - xDS RBAC authorization bypass via Metadata / RequestedServerName matchers -- requires xDS; PATH uses none. - xDS RBAC panic on NOT rules wrapping unsupported fields -- same. - HTTP/2 Rapid Reset mitigation bypass (DoS) -- affects the server-side HTTP/2 transport. PATH is a gRPC client dialing Shannon full nodes and exposes no gRPC server; the only grpc.NewServer() in the tree is in network/grpc/grpc_test.go. Taken anyway because it is a single patch version with no API change. go mod tidy also promotes github.com/prometheus/client_model from indirect to direct. That is not part of the bump: the cardinality regression test added earlier on this branch imports client_model/go to read back the label values the collector actually received. It was already a transitive dependency, so nothing new enters the build. Still open and NOT addressed here: github.com/getkin/kin-openapi (1 critical, 1 medium) in portal-db/sdk/go and portal-db/sdk/go/example. Those are separate Go modules -- `go list ./...` resolves zero portal-db packages, so none of it links into the PATH binary. It is still published as an SDK by .github/workflows/auto-version-bump.yml, so it remains a downstream concern and wants its own decision: bump, or stop releasing it.
oten91
added a commit
that referenced
this pull request
Aug 25, 2026
… gate, batch responses, plus cardinality/solana/archival follow-ups (#528) Follow-up to #527. The branch grew past its original scope: it now carries the F5/F6 cardinality follow-up plus four other lines of work that were deployed and validated together over the last two weeks. **Every commit here is live on canary and mainnet as `sha-040ae42-rc` and has been validated in production** (earlier cuts of the branch ran as `sha-064bc62-rc` and `sha-45e00a2-rc`; sections 6–8 below cover what landed since). That is the main argument for reviewing it as one unit — these commits were never exercised separately, so splitting them would put untested combinations into `main`. --- ## 1. Metric cardinality (the branch's original purpose) `e3f5c7e2` — bound histogram labels, remove `supplier` from aggregate metrics. Two rounds of a production cardinality incident converged on one rule: **only a label's value set bounds it.** A sanitizer bounds a value's *shape*, never the set. A cardinality guard bounds the *live registry*, never the number of distinct series Prometheus retains — `path_supplier_signal_total` sat at 26% of its cap while being one of the two largest series sources in the job (6,523 tuples live in 10 minutes against 60,674 distinct over one pod's 7.7h life). A label on a histogram costs roughly 12× what it costs on the counter beside it. `path_relay_latency_seconds_bucket` was 31.6% of all gateway series because it carried `status_code` × `reputation_signal`, a 20× pair no dashboard ever queried from the histogram. Outcome taxonomy now lives on the counter; the histogram keeps topology labels. Per-supplier questions are served by `GET /ready/<service>?detailed=true` — a point lookup instead of ~74K retained timeseries. Three metrics keep `supplier` deliberately, where the address is the actionable payload rather than a way of naming an operator. `Test_SupplierLabelIsGone` enforces the rest. ## 2. Solana sync allowance and health checks `2304bc9b` `2d49d8e5` `ad729c3f` `5c1c8d60` `0e856cc4` `068ff990` Solana's `ValidateEndpoint` had no sync allowance at all — `sync_allowance: 750` was configured but never implemented. During the 2026-08-18 surge (200 → 2500 rps, ~2.5 blocks/s) only the freshest endpoint stayed valid, producing near-total lock-in on one operator while others sat at score 100 with no traffic. Health checks cannot correct this; they run at the same per-endpoint rate on every operator. Also here: kava CometBFT checks routed to `comet_bft`, slower xrplevm websocket probes, a missing solana health observation no longer scored as a fault, and relays that never received an HTTP status no longer counted as successes. ## 3. Go, dependencies, CI `d3e0273c` `e6e5530e` `fa146acd` `c10e5be6` — poktroll v0.1.35, Go directive to 1.26.6 for the stdlib security fixes, drop the libsecp CGO variant, build release platforms concurrently. The Go bump takes `govulncheck` from 12 reachable findings to 6, all of which are `Fixed in: N/A` (three withdrawn lib/pq advisories, openpgp, two cosmos x/crisis init-only). The `net/http` CVE was genuinely reachable via `router.go` `ListenAndServe`. The Dockerfile Go patch is deliberately left floating so future patches are picked up; the directive sets the floor. ## 4. Heuristic and reputation detectors `dd710d38` `4931c6f4` `6ef6ca1a` `22e9dff1` An endpoint returning zero-length payloads at ~0.2% of its traffic held a reputation score of 100 all day, and **neither existing mechanism could reach it**: - Additive scoring is outvoted by volume. At one violation per 1000 requests the endpoint earns +998 and loses −25, so the score returns to its ceiling however long the behaviour continues. Raising the per-event penalty to FATAL (−50) does not change the sign. - The critical-rate detector is tuned for "unambiguously broken" (30%), and more fundamentally `CriticalRateEWMAAlpha`'s ~20-request memory **cannot represent a sub-1% rate at all** — the EWMA can only be 0 or ~0.05 there. No threshold change to that detector could have worked. So `22e9dff1` adds a second detector rather than retuning the first, with a much longer window (alpha 0.001, ≈1000 requests) and a threshold three orders of magnitude lower. Two classifier bugs fixed alongside it: a `"(method=X)"` suffix made every exact-match case in `classifyHeuristicErrorAsSignal` unreachable, so an empty payload degraded to `unknown_payload_error` (MINOR) — only the `HasPrefix("error_indicator_")` case survived, which is why it hid. And `getProgramAccounts` returning an empty array is a valid success, not a fault. ## 5. Archival promotion and health-check contamination (today) ### `54659fb6` — geth PBSS pruned state Geth's path-based state scheme reports `metadata is not found, <block>`. Every archival pattern in PATH used hash-based-scheme wording (`missing trie node`, `state has been pruned`), so a PBSS node's honest "I do not retain that state" matched **nothing**, at four separate sites. The deeper defect: `IsArchival` returned true for **any** successful `eth_getBalance` / `eth_call` / `eth_getCode` / `eth_getStorageAt` / `eth_getTransactionCount` **without reading the block parameter**. Those are also the ordinary way to read current state, and a pruned node answers them perfectly — so the archival pool was polluted by construction and marked archival for 8h. `targetsHistoricalBlock` now gates the promotion. Known residual: the `DataExtractor` interface carries no perceived chain tip, so a numeric block a few blocks back still reads as archival. Closing that needs an interface change across all four extractors. ### `d8f4c3c1` — health-check probes were feeding both rate detectors Both volume-independent rate detectors are wrapped in `if !signal.IsHealthCheck`, so a probe cannot bench an endpoint on its own — a strict or flaky check must not cool an endpoint that serves user reads perfectly. **That guard was intact. The stamp was not.** `IsHealthCheck` was set at only three call sites, all in the health-check executor. One probe also reaches reputation through the protocol layer twice more — the relay itself via `requestContext`, and `Apply{HTTP,WebSocket}Observations` on that same relay's observations — and neither stamped it. The field doc on `requestContext.isHealthCheck` stated outright that it "does not affect reputation signals or observations", which is why the omission read as deliberate. The result is a self-sustaining loop rather than a one-off penalty: a benched endpoint receives no user traffic, so probes become its only signal, so its rate EWMAs are entirely probe-derived, so it re-benches itself on the next probe failure. This predates the new detector. The critical-rate detector has been contaminated since it shipped; `22e9dff1` only made it visible by tripping at a much lower threshold. `Apply{HTTP,WebSocket}Observations` now take `isHealthCheck` as a required parameter rather than defaulting it — the receiving protocol layer cannot distinguish synthetic observations from real ones, so each caller states it at compile time. Scope is narrow on purpose: only the rate detectors exclude probes. A probe still moves the additive score — that is how a benched endpoint recovers when it receives no user traffic — and still increments the counters, so no rate's denominator changes shape. `TestHealthCheckSignals_StillMoveTheAdditiveScore` guards that. ### `064bc628` — each rate cooldown escalates against its own history `Score.InvalidRateCooldownCount` is documented as kept separate from `RateCooldownCount` so the two detectors escalate independently. The counters were separate; the timestamp they escalated against was not — both compared against the shared `Score.CooldownUntil`, which the strike system also writes. Note the sign: `time.Since()` on a cooldown still in force is negative, hence always below `DefaultMaxCooldown`. Any bench in force, from any mechanism, made the next trip of either detector read as consecutive. Each detector now records the end of the cooldown it set and escalates against that. `CooldownUntil` is unchanged and remains the only field selection reads. ### `b395d183` — the "historical state" pruned-state wordings Found by probing rather than by reading. Sending a block 27M deep to endpoints PATH had marked archival returned two wordings that missed every pattern in `archivalErrorIndicators` by a single word: ``` gnosis: "historical state is not available" -- "state not available" misses on "state IS not" poly: "historical state <hash>" -- "historical data" misses on "historical STATE" ``` Both fell through to the "some other error" branch, which returns an error rather than false, so an endpoint that had just failed an archival query was never demoted out of the archival pool. The bare `"historical state"` prefix covers both, and is already present in `qos/heuristic/indicators.go` — the two catalogues had drifted, so this realigns them. ### `31617122` — an unverified archival mark no longer outlives a verified one The two sources of archival status had drifted 16× apart, in the damaging direction: ``` health-check mark 30m user-traffic mark 8h ``` The health-check path pins an exact expected historical value in the rules file, so a node that ignores the block parameter and answers from current state fails it. The user-traffic path cannot pin a value — the query is whatever a client sent — so it grants archival status on any successful archival-method call, and `54659fb6` notwithstanding it still trusts a successful response, which is what a fabricating node always produces. Measured in production: four endpoints on one operator were marked archival while returning current state for every block asked, including one 256× past the chain tip. The archival health-check rule for the service they served had been deleted for failing every endpoint — which was the rule working correctly, that service has no archival nodes — leaving only the unverified 8h path to promote them. Both paths now share `gateway.ArchivalStatusTTL`. The old comment on the 8h constant claimed it "matches health check archival TTL"; it did not, and the false comment is probably why the drift went unnoticed. The test reads the stored expiry back through `UpdateFromExtractedData` rather than comparing constants, so re-hardcoding a duration at the call site fails it. Bootstrapping is unaffected: promotion still happens via requests naming a numeric block within the archival-required threshold, which route freely rather than being filtered to already-archival endpoints. --- ## Production validation Deployed to canary at 07:18 UTC and mainnet at 08:57 UTC on 2026-08-20. Because the two environments flipped at different times, the same metric collapsing twice — each time following the build — rules out pod age and traffic composition. | | before | after | |---|---|---| | mainnet `rate_cooldown_total` | 0.178/s | **0.0006/s** (297×) | | canary `rate_cooldown_total` | 0.005/s | 0.0012/s | | `pool_collapse_guard{solana}` on canary | 143,090 / 14h | ~35 / 85m | | invalid-rate trips | 10 services | **solana only**, both envs | Guardrails flat throughout, judged against 6h ranges rather than point readings: fleet success 95.7% mean (min 92.5, max 97.3), solana pool size unchanged, no cooldown spike. Redis confirms the escalation fix directly. Mainnet DB2 went from **0 of 5,901** keys carrying `rate_cooldown_until` to 292 of 297 sampled within five minutes of the flip, and two solana endpoints that tripped post-deploy each recorded `invalid_rate_cooldown_count = 1` with their own timestamp — a first offence, unescalated. For contrast, the pre-flip distribution had 84 keys above zero with **35 at ≥ 6**, i.e. benched the full hour every time, topping out at 98. Archival counts from `/ready/<svc>?detailed=true` moved as intended. The old build pinned five services at exactly 100% — every endpoint of every operator archival, which is not a plausible ground truth. The fixed build discriminates: on gnosis it keeps one operator at 12/12 while dropping two other operators to 0/17 and 4/17, where the old build had all three at 100%. ## Testing `go build`, `go vet` and `golangci-lint` clean. Unit tests pass; `reputation/storage` needs Docker for testcontainers-Redis and fails without it. Every call site in the two reputation fixes was revert-checked individually — seven reverts, seven confirmed test failures. Two traps found while writing those tests, both recorded in comments: - The first escalation test **passed against the revert**. A fresh key plus a foreign bench in force does not discriminate, because incrementing a zero counter yields 1 — exactly what a correct reset yields. The discriminating case needs stale non-zero history *and* a foreign bench. - `runAtRate(4000, 100)` trips the detector five times, not once: a trip resets the EWMA and the loop continues. Any first-offence assertion has to drive one signal at a time and stop at the first trip. The pre-existing test never noticed because it only asserted `IsInCooldown()`. Tests assert on the signal reputation *receives*, from the production caller, rather than on the flag the caller set — the flag was already true and proved nothing. CI note: the `xrplevm` HTTP E2E check has been failing on this repo independently of this branch. Please confirm it also fails on `main` before treating it as a blocker here. ## Known open items 1. **moonbeam genuinely has no archival nodes.** Answered by direct probing: `moonbeam_archival` was deleted from the rules file on 2026-08-07 for failing 100% of runs — which was the rule working correctly — and the single endpoint still marked archival there is one that ignores the block parameter. Impact is small: moonbeam draws 1.7–3.5 `archival_required` rejections/s against poly's 761/s. 2. **Endpoints that ignore the block parameter cannot be detected by any success-based check**, including the `targetsHistoricalBlock` gate added in `54659fb6`, which verifies the request named a historical block and then trusts a successful response. Measured on one operator across two services: identical balances at block 1, block 5,000,000 and block 4,294,967,295. A design for detecting this is written up in `DESIGN_NEGATIVE_HEALTH_CHECK.md` and deliberately **parked** — the two commits above cover most of the exposure without a new check type. The cheapest future detector is `eth_getBlockByNumber(H)` asserting `result.number == H`, which is self-verifying and needs no external reference. 3. **Only 22 of 69 services have an archival health-check rule.** On the rest, archival status comes exclusively from the unverified user-traffic path. 4. **`poly_archival` and `xrplevm_archival` assert `expected_response_contains: "0x0"`**, a substring matching a large share of hex values. Weak, and in the external rules file rather than this repo. 5. **`InvalidRateThreshold = 0.005` was sized from one hour of data with a contaminated denominator.** Now that the denominator is honest it fires ~31/hour on solana, correctly confined but not near-silent. Worth re-deriving from the real per-key rate distribution. 6. **User relays record a reputation signal twice** — once in `context.go` and once via the observation path. Pre-existing double-count, not addressed here. --- ## 6. Circuit-breaker failure-rate gate `db817520` `aef31dd6` `1a9ca5cc` `a7063b59` `d82b1e1d` **Hysteresis.** The gate used one threshold to break a domain and nothing but TTL expiry to restore it, so a host whose true failure rate sat just above the line was removed every time it was let back in, with escalation holding it out longer each cycle. Measured on six relay-miner hosts behind one operator: the four marginal ones (within 1.7 points of the 80% line) spent 69–92% of a six-hour window removed from the pool, identically in both environments, while answering 40 consecutive probes with zero errors at the same latency as the host carrying the service. A domain that broke recently must now be clearly worse to break again (threshold + 0.15). **The denominator was blind to hedge-race successes.** Failures reach the gate from every path, but a success only counts where the returning path calls `RecordSuccess`, and the hedge-race branch never did. With a hedge delay configured *every* first attempt returns through it — including the overwhelming majority where the hedge never fires — so the gate saw 5% of a high-volume operator's successes and 26% fleet-wide, and read a low-volume host at 30–66% failure where the relay counters read ~21%. No threshold, hysteresis included, can hold against a rate inflated past it by construction. Tested through the real retry loop. **`path_circuit_breaker_outcome_total{service_id, domain, outcome}`** exposes both sides of the fraction the gate actually computes, per hostname — the gate keys on hostname while `path_relays_total` keys on eTLD+1, and the blended figure cost four wrong hypotheses in one investigation. Two labels by design, registered with the cardinality guard. ## 7. Traffic shape and classifier fixes `d7e4d81a` `7ace2a41` `45e00a24` **`GET /admin/request-sample`.** Every quality signal rewards whoever answers fastest, and an endpoint fronted by a cache answers a *repeated* request in sub-millisecond time without touching a node. Whether a fast operator is fast or merely cached has to be read from the traffic, and nothing recorded its shape. One request in N is fingerprinted on method + compacted params (ids excluded) and counted per service in fixed windows; the endpoint reports uniqueness, top-1 share, and per-method uniqueness — block-height calls are legitimately repetitive, account lookups are not. Bounded table, per-pod, two gauges keyed on `service_id` only. **Solana's account-index exclusion is a capability limit**, not a fault: `-32010 "<key> excluded from account secondary indexes"` is node configuration, and another operator serves the identical call from its index. It matched nothing in the catalogue, so a dapp polling three `getProgramAccounts` queries continuously charged one operator a breaker failure and a reputation penalty on every poll. Added as an allowlisted phrase, deliberately not an archival pattern. **`path_observation_pipeline_total` mislabelled every no-fault error as `major_error`** — it re-derived `reputation_signal` from the observation's error type instead of the signal actually recorded. `path_relays_total`, labelled from the real signal, disagreed about the same relays; two observables disagreeing about one state, and the pipeline label was the wrong one. ## 8. Batch responses and client cancellations (2026-08-24/25) `9137cdbb` `da13f402` `796ffc7d` `51ffb44a` `43abf62e` `040ae428` Started from an operator asking why they were "broken" for `batch_transport` on one service. The label misleads: PATH never sends a multi-request relay — batches are split and each item is its own relay. `batch_transport` means one item of a client batch failed before any HTTP response existed. **Every one of those breaks was `context canceled`** — the client hung up mid-batch, and the batch item loop stamped the abort of *our* request on whichever supplier was holding the item. It hit every operator serving the service at once. The single-request loop already returned on a done context before reaching `MarkBroken`; the batch loop now does the same (`9137cdbb`). Measured: breaks 116/h → 0 on canary with the control unchanged. Worth recording honestly: the user-facing effect over 40 minutes was **neutral**. The cancels are not uniform across operators — they track p99 tail latency (0.23s on the best operator, 1.0–2.3s on the worst), because `context canceled` is the *client's* timeout firing on the supplier's tail — so the old behaviour was accidentally benching the one slow operator for ~20 minutes at a time. Reputation does not bench it because these errors are no-fault. The signal belongs in reputation's latency path, not the breaker; that is the follow-up, and this A/B is its evidence. **A failed batch item threw away the whole batch.** An item that failed every attempt has no body; the assembler dropped it, the length check saw N−1 for N, and the client got a single `id:null -32603 "batch response length mismatch"` in place of the N−1 answers already relayed and paid for. Every retained log line of the failure was that shape. Per JSON-RPC 2.0 each request object gets a response object, so the item nothing answered now gets an error object carrying its own typed id, next to the successes (`da13f402`, EVM and Cosmos via the shared validator; `796ffc7d` for NoOp, which had the quiet form — the item simply vanished from the array with HTTP 200). Three more shapes surfaced on canary and are fixed in the same helper: - **A one-element batch that retries** runs down the single-request path, which records every response it saw; the batch assembler collected all of them, saw 2 for 1, and replaced a request that had *succeeded* with a 500. Keep the latest per id (`51ffb44a`). - **An id-less item is not a notification** once it has been through PATH — it is relayed alone with `"id":null` written out and the node answers it. `da13f402` had excluded such items from the expected count and rejected every batch carrying one with "expected N, got N+1"; caught on canary within the hour, reproduced with a two-item probe (500 on canary, 200 on the control), fixed in `43abf62e`. A test written from the spec instead of from what suppliers actually return; the replacement test uses the production shape. - **A response that is not a well-formed `Response`** — an error given as a bare string, an id type the parser rejects — is still an answer to some item. Attribution now reads only the id; an unreadable id is a wildcard like a null id (`040ae428`). The mismatch error now lists the response ids, bounded, so any survivor names its shape in the log. ### Validation Canary ran each cut against mainnet as an untouched control, then mainnet followed after a 12-hour soak. Marshal-failure log lines in the retained tail: **242 on the control, 0 on canary** over 12h; **0 on both** since mainnet rolled. Fleet relay success 0.926 vs 0.927, 5xx and rps equal, 0 restarts, heap on the known ~30 MiB/h curve. Probed end-to-end, tallied by the serving environment: an id-less two-item batch 8/8 → 200 with both answers; a one-element batch forced to retry (an archival call most suppliers refuse) → 3 of 5 500s on the control, 200 with the supplier's own error on the fix. Two shapes are not forceable from outside — an item with *no* body, and an unreadable id — and rest on the log evidence plus the new ids diagnostic. Every fix has a test that fails with the fix reverted; each was checked. ### Also worth knowing - `path_requests_total{status_code="error"}` is derived from the *endpoint* observation (backend status 0 with an error set) — per-relay transport failure, not what the client received. No metric records the client-facing batch outcome; the Error-level log line is the only evidence, and a counter for synthesized batch errors (`service_id` only) would be a reasonable addition. - A batch where *every* item fails still returns the empty-batch response (nothing, 200) — the emptiness check runs before the fill. Rare; separate item. - Solana's batch context already answered per id; it turns numeric-looking string ids into ints and says "malformed response" for a missing body. Cosmetic, unchanged. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Path metrics reached 63% of PNF's entire Prometheus TSDB (5.55M of 8.76M series),
taking it to 89% of its memory ceiling. The same registry growth was happening inside
gateway pods: working set 0.775 → 6.29 GiB in five days, peaking at 9.49 GiB against a
12 GiB limit, with
heap_inuseat 7.23 GiB againstGOMEMLIMIT=10GiB.Prometheus-side relabelling could only have fixed PNF's half. Two details confirmed this
was registry growth rather than load: CPU was flat across the whole window, and beta grew
6.8× while essentially idle.
Report: PNF Ops,
gateway-metric-cardinality-report-20260812.md.Root cause
Two label sources leaked unbounded values.
domaincarried raw bech32 supplier addresses — 4,172 of 4,608 distinct values —making
path_probation_events_total27% of the whole TSDB. The guard that should havecaught this is dead code:
reputation/selector.gochecksif err != nil || domain == "",but
ExtractDomainOrHostdoes not fail on a barepokt1…. A dotless host takes theisPrivateOrInternalDomainbranch and is returned verbatim with a nil error, so theaddress arrives looking like a successful extraction.
methodis attacker-controlled — it carries the JSON-RPC method name or the REST URLpath, so any unauthenticated client can mint a series per request. PNF found live TSDB
values including CRLF header injection, XSS, path traversal and template injection
payloads, currently being produced by commodity scanners.
The part worth reviewing carefully
SanitizeMethodLabelalready existed and was already wired atprometheus_reporter.go:135. That is why this was missed. It bounds the shape of avalue but not the set: route-shaped segments pass through verbatim, so
/aaa,/aab, …are unbounded.
Test_ObservationPipeline_AttackerMethodsAreBoundedpins this down — with the guardreverted, 5,000 probe paths survive the sanitizer untouched (
"5000" is not less than or equal to "100"). Only the cardinality guard bounds it. Same class as the 2026-06-15UTF-8 fix, which sanitized characters and never bounded the set.
For pod heap specifically, the guards matter because of eviction, not the cap:
promautocounters never evict cold series, so a cap alone freezes the registry withoutshrinking it.
withEviction+DeleteLabelValuesis what returns heap without a restart.Why not fixed in
ExtractDomainOrHostIt has 38 call sites and also feeds the operator concentration cap
(
domain_diversity.go), admin drains (reputation.go:129),blocked_domains(
domain_blocklist.go:182) and reputation keys (key.go:87). Changing it would changeendpoint selection.
SanitizeDomainLabelsits at the metrics label boundary instead,where it cannot affect routing.
Its bech32 test is gated on "no dot" so it can never fire on a real domain:
isBech32Likerequires the post-
1remainder to be entirely alphanumeric, and every registrable domainhas a dot there. Bare IPs and dotless internal hostnames are deliberately preserved.
Changes
SanitizeDomainLabelon all 18Record*and 2Set*helpers that take a domain, plusfour sites that bypass those helpers. Two of them —
SelectionCandidateTotalandSelectionSelectedTotal— are counters with noReset(), so leaked operator keys wereretained for the pod's lifetime.
probation_events_total,observation_pipeline_total,circuit_breaker_events_total,rpc_type_fallback_total.supplierlabel fromrpc_type_fallback_total. It carried 3,289 valuesagainst the metric's own 9 domains × 12 service_ids. The argument is kept and ignored so
no caller changes, matching the existing
RecordHealthCheckpattern.normalizeRESTPathmaps any segment outside the RFC 3986 unreserved set to:varandcaps path depth. Real Cosmos routes are unaffected
(
/cosmos/base/tendermint/v1beta1/blocks/latestpasses through intact).Sanitization runs after
RecordCircuitBreakerEvent's empty-domain check, not before:SanitizeDomainLabelmaps""toDomainUnknown, which would silently defeat thedocumented skip.
Validation — deployed as
sha-d052cf1-rc, measured ~2h after rolloutheap_inuse(fleet)heap_inuse(fleet)probation_events_totalactive seriesobservation_pipeline_totalrpc_type_fallback_totalScrape payload is now below the pre-regression baseline (57,062 vs 63,290).
Guard drop rate is zero on every metric — the label fixes alone brought cardinality
under the cap, so the four new guards sit as pure backstop. The concern that probation
might saturate the 25k cap did not materialise: the real tuple count is 1,364, not the
234k theoretical cross product.
Reading note for anyone re-checking this:
/api/v1/label/domain/valuesstill lists all4,172
pokt1values and head series still climbs immediately after the fix, because bothinclude stale series awaiting head truncation. Judge with instant
count()(5mlookback = active series only). The
supplier_addrsentinel appearing in domain values,and
:var-shaped methods appearing, are the positive signals.Testing
Five regression tests, all asserting through the production emit helper and reading
the label values the collector actually received — not the sanitizers directly. A
sanitizer that is never called on the emit path passes its own unit tests perfectly, which
is exactly how the three drain bugs shipped green (CLAUDE.md, "Testing Changes That Affect
Routing").
Each was revert-checked: fix removed → test fails.
go test ./...clean,golangci-lint0 issues.Also on this branch
d052cf18— health-check interval and dead archival-check cleanup from the 08-07 work.Unrelated to cardinality; separated into its own commit rather than buried in the fix.
14033352— grpc 1.82.0 → 1.82.1, clearing Dependabot GHSA-hrxh-6v49-42gf (high). Noknown exposure: all three sub-issues need xDS (PATH uses none) or the server-side HTTP/2
transport (PATH is a gRPC client and exposes no server).
Not closed here
opt-in. Four guards were wired by hand, so a sixth metric can still ship unguarded.
path_relay_latency_seconds_bucket(639,670 series) gets the domain fix viaRecordRelaybut is deliberately unguarded: it is a histogram at ~12 series pertuple, and the guard's own docs record a 100k-tuple cap producing 945k series in the
2026-04-28 audit. It needs a deliberate tighter per-metric limit, not the 25k default.
portal-db/sdk/go. Separate Go modules —go list ./...resolves zero portal-db packages, so none of it links into the PATHbinary. Still published as an SDK by
auto-version-bump.yml, so it wants its owndecision: bump, or stop releasing it.