feat: instrument HTTP surface in Prometheus (#189) - #212
Merged
Miracle656 merged 2 commits intoSep 30, 2026
Merged
Conversation
|
@collinsezedike 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! 🚀 |
collinsezedike
force-pushed
the
feat/http-metrics-prometheus
branch
from
September 29, 2026 21:40
c740907 to
969464d
Compare
Mounted after them, the HTTP middleware never saw the requests those two reject, so 429s and invalid-?network= 400s were absent from http_requests_total. Moved to the top of the chain, right after cors(). Also documents the two new metrics in the README table and records why the route label must stay req.route.path: req.baseUrl is the matched mount path, and src/api/accounts.ts mounts a router at "/:address/transfers", so baseUrl carries the real address and would make the label unbounded. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Miracle656
approved these changes
Sep 30, 2026
Miracle656
left a comment
Miracle656
left a comment
Owner
There was a problem hiding this comment.
Approved. I went through every label, since an unbounded one here is a memory leak in the metrics process, not just noisy graphs:
route—req.route?.path, the Express route pattern./transfers/incoming/GABC…reportsroute="/transfers/incoming/:address", and an unmatched request reportsroute="unknown"rather than the raw URL. Bounded by the number of routes in the source. This is the one that matters and you got it right.method— bounded by what Node's HTTP parser will accept; an unrecognised method never reaches Express.status— bounded by the codes we emit.
No address, contract id, txhash, query value or raw path reaches a label. Worth saying explicitly: I checked whether req.baseUrl + req.route.path would be an improvement — it would give /search and /webhooks distinct labels instead of both reporting route="/" — and it is a trap. req.baseUrl is the matched mount path rather than the pattern, and src/api/accounts.ts:29 does router.use("/:address/transfers", …), so for that route baseUrl is /accounts/GAAA…AWHF/transfers with the real address embedded. I ran it to be sure. One label value per address is exactly the unbounded leak. Your simpler version is the correct one, and I've added a comment saying so, so nobody 'fixes' it later.
Excluding /metrics from its own counters, and testing that exclusion, is the detail most versions of this PR miss.
One thing I changed rather than sent back (c8a9f9a on your branch): the middleware was mounted after networkMiddleware and limiter, so it never saw what those two reject — a 429 storm or a flood of invalid ?network= values would have been entirely absent from http_requests_total, which is precisely the traffic a request counter exists to show. Moved it to the top of the chain, right after cors(). I also added the two metrics to the README /metrics table, which the repo keeps current.
Verified after that change: npx tsc --noEmit clean, full jest suite 39 suites / 448 tests passing, metrics.test.ts 13/13.
Nice, tight PR.
Miracle656
added a commit
to krsnak/wraith
that referenced
this pull request
Sep 30, 2026
Two mechanical integrations with changes that landed on main after this branch was cut: - src/api.ts: import-block conflict only. Miracle656#212 added httpRequestsTotal / httpRequestDurationSeconds to the ./metrics import; this branch added getCachedTokenDecimals and ./amount. Taken as a union — the two touch nothing in common. - src/__tests__/routes/transfers.test.ts: Miracle656#218 made hasMore a required field on the queryTransfers return type, so the new 6-decimal $select test's mocked result no longer satisfied the cast. Added hasMore: false. The queryTransfers body auto-merged: Miracle656#218's includeTotal/totalField and this branch's prismaSelect contractId + derived displayAmount are disjoint, and both survive. Verified on the merge result: tsc --noEmit and tsc -p tsconfig.test.json both clean, jest 47 suites / 562 tests all passing (main baseline 46/557). Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Miracle656
added a commit
that referenced
this pull request
Sep 30, 2026
* Use token decimals for display amounts * docs(wave): W094-W098 batch draft (published as #203-207) Five issues, 800 points. The headline is W094: `src/indexer.ts:491` switches between pollOnce and pollParallel on INGEST_WORKERS, and `indexer/parallel.ts` imports only fetchEventsSafe, parseEvents, upsertTransfers, setLastIndexedLedger and emitTransfer — no NFT parsing, no metadata, no account summaries. Raising the worker count for throughput silently stops indexing whole categories, with no error to notice. Also: /offramp/orders/:orderId serves an order with no authorization, NFT metadata is fetched one serial round trip at a time, and every paged query pays for a full COUNT. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB * Docs: Mainnet deployment guide (#166) (#211) * Docs: Mainnet deployment guide (#166) * docs(mainnet): correct the native SAC ids, fix the backup link, add DIRECT_DATABASE_URL Review fixes applied on top of #211: - The mainnet block used CDLZFC3SY…, which is the *testnet* native XLM SAC (Asset.native().contractId(Networks.TESTNET)). Mainnet is CAS3J7GY… (Networks.PUBLIC). The testnet block used CDMLFMKMM…, which is not the native SAC on either network. Both corrected, with the derivation inlined so the next reader can check rather than trust. - Added a warning that SAC_CONTRACT_IDS must be set explicitly on mainnet, because the built-in fallback in src/indexer.ts is the wrong address. - ../W076 did not resolve to anything; pointed at ./backup-restore.md. - Added DIRECT_DATABASE_URL to both env blocks — prisma/schema.prisma declares directUrl, and boot-time schema sync fails without it. - Linked ./DUAL_NETWORK.md for the NETWORKS=testnet,mainnet option. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --------- Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * fix: narrow XDR error classification (#209) * fix(offramp): require a bearer token to read an order (#216) * fix(offramp): require a bearer token to read an order GET /offramp/orders/:orderId served any order to whoever held its id: the bank payout amount, the deposit address and the rate. Orders now get a random public id (ofr_ + 128 bits) in place of the provider's, and the creator is handed a bearer token (oft_ + 256 bits) once, in the create response. Only its SHA-256 is stored. Lookups need Authorization: Bearer; a missing header is 401, and an unknown id, a malformed id and a wrong token are all the same 404. Failed lookups are rate-limited separately from the app-wide limiter (10 per 15 minutes per IP by default); successful polling does not count. Replaying a creation with the same idempotencyKey now also needs the same walletAddress (409 otherwise) and re-issues the token, since only its hash is kept. The lookup response no longer names the provider (source: live) or returns its id, and a database failure returns a generic 500 instead of reaching the global handler. * test(offramp): use a valid strkey for the second wallet fixture GBBB...SAM was 56 characters but failed StrKey.isValidEd25519PublicKey. The test passes either way today, because the route does not validate the address, but a fixture that is not a real strkey breaks the moment it is handed to anything that decodes one. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --------- Co-authored-by: blockchain-maxis <267648998+blockchain-maxis@users.noreply.github.com> Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * fix: skip malformed events in parseEvents instead of wedging the indexer (#197) Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * fix(indexer): share one per-batch pipeline between the single and parallel ingest paths (#219) Co-authored-by: DevTobis <232918735+DevTobis@users.noreply.github.com> * Bring docker-compose.yml in line with env contract (#194) (#220) * Bring docker-compose.yml in line with env contract (#194) - Remove obsolete version: "3.9" key - Add all missing env keys from .env.example: - DIRECT_DATABASE_URL - STELLAR_NETWORK - SOROBAN_RPC_URL (with STELLAR_RPC_URL as backward-compat alias) - NETWORKS - SAC_CONTRACT_IDS (and per-network variants) - NFT_CONTRACT_IDS (and per-network variants) - RETENTION_DAYS - CACHE_ENABLED and all Redis cache config - TOMBSTONE_CHECK_EVERY_CYCLES - LP_POOL_CONTRACT_IDS variants - SKIP_INDEXER - Keep CONTRACT_IDS as documented backward-compat alias - Add optional redis service with cache profile for CACHE_ENABLED support - Update README to use SAC_CONTRACT_IDS and add cache profile instructions - Add test to validate compose file meets requirements * test(compose): derive the env-contract check from .env.example The drift test built envKeys from .env.example and then never used it, asserting against a hand-maintained list instead — so a key added to .env.example and forgotten in docker-compose.yml still passed, which is the one failure #194 is about. The assertions were also substring matches on the whole file: toContain("SAC_CONTRACT_IDS") is satisfied by SAC_CONTRACT_IDS_TESTNET, and toContain("CONTRACT_IDS") by either, so they held on a file declaring none of them. Now it parses the wraith service's own environment block (anchored on the service, since db has an environment block too) and compares declared keys against every uncommented key in .env.example, with an explicit, empty exclusion list. Verified it fails on an added key and passes without one. Also: the docker compose config case ran unconditionally and called fail(), which is not defined under jest-circus, so on a machine or CI runner without Docker it failed with a ReferenceError. Unit tests here do not require Docker (the integration suite is vitest + Docker), so it now skips instead. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --------- Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * Fix/cache network key (#202) * fix(cache): include resolved network in Redis cache key (#182) defaultKeyFn now prepends req.network (set by networkMiddleware) to the cache key so mainnet and testnet requests never collide, even when the network is supplied via the X-Network header rather than ?network=. The ?network= query param is filtered from the query segment since it is already captured by req.network, ensuring ?network=mainnet and X-Network: mainnet produce identical keys. Existing keys change shape and will expire naturally over their TTL. * fix(cache): include resolved network in Redis cache key (#182) * chore: drop the unrelated package-lock.json change The branch re-resolved fsevents and dropped its "dev": true marker. Nothing in this PR touches dependencies, so restore the lockfile to main's. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --------- Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * Add npm run db:seed with deterministic fixtures (#221) - Create shared fixture module in src/fixtures.ts with deterministic test data across two addresses (ALICE, BOB, CAROL) and two contracts (CONTRACT_A, CONTRACT_B) - Add seed script at scripts/seed.ts with --network flag support (testnet/mainnet) - Add db:seed script to package.json - Update integration tests to use shared fixture module instead of local copy - Update README Quick Start with seed step and real sample output - Add fixture validation test to verify data structure and coverage The seed script uses skipDuplicates on unique constraints, making it safe to run multiple times without duplicating rows. Fixtures include TokenTransfer, NftTransfer, and AccountSummary rows with deterministic eventId values. Resolves #193 * fix(seed): accept --network mainnet as well as --network=mainnet Follow-up to #221: this fix was pushed to the PR branch but did not make it into the squash. The docstring advertised the space-separated form while parseArgs matched only --network=, so 'npm run db:seed -- --network mainnet' silently seeded testnet. Both spellings now parse; an unknown value still exits 1. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB * Fix token decimals review issues * perf(api): drop the default COUNT from list queries, add hasMore and opt-in includeTotal (#218) Co-authored-by: royalTreasure <295874283+royalTreasure@users.noreply.github.com> * feat: instrument HTTP surface in Prometheus (#189) (#212) * feat: instrument HTTP surface in Prometheus * fix(metrics): instrument before networkMiddleware and the rate limiter Mounted after them, the HTTP middleware never saw the requests those two reject, so 429s and invalid-?network= 400s were absent from http_requests_total. Moved to the top of the chain, right after cors(). Also documents the two new metrics in the README table and records why the route label must stay req.route.path: req.baseUrl is the matched mount path, and src/api/accounts.ts mounts a router at "/:address/transfers", so baseUrl carries the real address and would make the label unbounded. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB --------- Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> * fix(indexer): bound NFT metadata lookups with a worker pool and per-cycle budget (#217) Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> --------- Co-authored-by: Miracle656 <iupacnumen2020@gmail.com> Co-authored-by: Gloria <glorious217@gmail.com> Co-authored-by: Jemimah <ekongjemimah@gmail.com> Co-authored-by: blockchain-maxis <blockchainmaxis@gmail.com> Co-authored-by: blockchain-maxis <267648998+blockchain-maxis@users.noreply.github.com> Co-authored-by: Apulupie <167634780+Frun1na@users.noreply.github.com> Co-authored-by: Tobiz <deborahayoola2000@gmail.com> Co-authored-by: DevTobis <232918735+DevTobis@users.noreply.github.com> Co-authored-by: BOA <97275013+boalambo@users.noreply.github.com> Co-authored-by: Bathoul Mohammed <funds0033@gmail.com> Co-authored-by: royaldev <chiditreasure15@gmail.com> Co-authored-by: royalTreasure <295874283+royalTreasure@users.noreply.github.com> Co-authored-by: Collins Ezedike-egwom <62267326+collinsezedike@users.noreply.github.com> Co-authored-by: That guy <120946193+ezedike-evan@users.noreply.github.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
2 participants
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.
Summary
Instruments the HTTP surface in Prometheus with request count and latency metrics per method, route pattern, and status.
Related issue
Closes #189
Type of change
Checklist
npx tsc --noEmitpassesnpm run buildpasses