Repository navigation
Lens runs one ingester set per enabled network against a single prom-client registry, but all four ingest metrics were network-blind, so the testnet and mainnet loops wrote to the same series. I added a network label to trades_ingested_total, amm_snapshots_total, price_snapshots_total and last_trade_timestamp, and threaded each ingester's own network argument through all six call sites. The gauge was the real casualty: last_trade_timestamp is a set(), so the testnet loop was silently overwriting mainnet's value for the same pair on every poll interval, leaving Lens's only staleness signal unusable while still looking like a plausible timestamp. - #203
Merged
Miracle656 merged 1 commit intoSep 30, 2026
Merged
Miracle656 merged 1 commit into
Miracle656 merged 1 commit into
Conversation
|
@DevScoopee Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…client registry, but all four ingest metrics were network-blind, so the testnet and mainnet loops wrote to the same series. I added a network label to trades_ingested_total, amm_snapshots_total, price_snapshots_total and last_trade_timestamp, and threaded each ingester's own network argument through all six call sites. The gauge was the real casualty: last_trade_timestamp is a set(), so the testnet loop was silently overwriting mainnet's value for the same pair on every poll interval, leaving Lens's only staleness signal unusable while still looking like a plausible timestamp. The counters were merged rather than wrong, so the two networks simply summed into one number and a per-network ingest rate wasn't queryable. The gauge was worse: because it is last-writer-wins, the value belonged to whichever network ticked last while presenting as network-agnostic, and nothing errored or went missing — the series just quietly meant the wrong thing. The fix added the label in src/metrics.ts and passed the network argument the ingesters already receive at every call site across sdex.ts, amm.ts and snapshot.ts. The deliberate invariant is the loop's own network and never activeNetwork, since that per-process constant is exactly the plausible-looking value that made this bug invisible on a dual-network deployment. The eleven new tests in tests/ingestMetrics.test.ts drive the real ingesters rather than poking the registry, so a call site that forgets the label fails CI. The headline case writes testnet at one timestamp and mainnet at a later one and asserts both series survive, which under the old code read "expected 1 to be 2". I verified the tests bite by reverting the label, which fails seven of the eleven. On cardinality, the series count is pairs times networks for the pair metrics and exactly networks for the snapshot counter, both bounded by WATCHED_PAIRS and ENABLED_NETWORKS. I deliberately added no issuer label, since it would multiply series by every token in the config while pair already identifies the instrument, and no pool label to the trade metrics, since pool ids are only known to the AMM loop. Existing queries keep working, but a recording rule that aggregated across networks will now see one additional series per pair. The new docs/ingest-metrics.md covers the label reference, the cardinality notes and a staleness alert tuned so thin pairs that legitimately stop trading don't page anyone, and I added a patch changeset. Closes Miracle656#167
6 of 11 tasks
Miracle656
added a commit
that referenced
this pull request
Sep 30, 2026
#193 replaced db.getIndexerCursor with getIndexerState, which returns {cursor, ledger} so an ingester can fall back to the last stored ledger when a Horizon trade id will not parse as a TOID. It branched before tests/ingestMetrics.test.ts (#203) and dualNetworkAlerts.test.ts (#205) landed, so those two files still mocked the old name. The merge is clean and tsc passes, but at runtime getIndexerState is undefined and 11 tests fail. Both suites use synthetic paging tokens that carry no TOID, so they now supply a stored ledger and exercise the documented fallback path. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
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
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.
The counters were merged rather than wrong, so the two networks simply summed into one number and a per-network ingest rate wasn't queryable. The gauge was worse: because it is last-writer-wins, the value belonged to whichever network ticked last while presenting as network-agnostic, and nothing errored or went missing — the series just quietly meant the wrong thing. The fix added the label in src/metrics.ts and passed the network argument the ingesters already receive at every call site across sdex.ts, amm.ts and snapshot.ts. The deliberate invariant is the loop's own network and never activeNetwork, since that per-process constant is exactly the plausible-looking value that made this bug invisible on a dual-network deployment. The eleven new tests in tests/ingestMetrics.test.ts drive the real ingesters rather than poking the registry, so a call site that forgets the label fails CI. The headline case writes testnet at one timestamp and mainnet at a later one and asserts both series survive, which under the old code read "expected 1 to be 2". I verified the tests bite by reverting the label, which fails seven of the eleven. On cardinality, the series count is pairs times networks for the pair metrics and exactly networks for the snapshot counter, both bounded by WATCHED_PAIRS and ENABLED_NETWORKS. I deliberately added no issuer label, since it would multiply series by every token in the config while pair already identifies the instrument, and no pool label to the trade metrics, since pool ids are only known to the AMM loop. Existing queries keep working, but a recording rule that aggregated across networks will now see one additional series per pair. The new docs/ingest-metrics.md covers the label reference, the cardinality notes and a staleness alert tuned so thin pairs that legitimately stop trading don't page anyone, and I added a patch changeset.
Closes #167