feat(sdk): x402Metrics wrapper for Prometheus observability - #53
Conversation
Eras256
left a comment
There was a problem hiding this comment.
Thanks for the fast turnaround on this — the wrapper's shape (opt-in, doesn't touch the payment path) is right, but there's a classification bug that inverts the core metric this PR is built around.
@x402/core's createPaymentRequiredResponse always sets error on the very first, no-payment-yet 402 (e.g. error: "Payment required") — so isChallenge = !capturedBody?.error is backwards for the most common event in the system: real challenges get counted as verify_fail, and vice versa. Same root cause makes x402_revenue_total empty in production unless options.routes is passed by hand (the README's own example doesn't), and makes real settlement failures (which come back as an empty-body 402, not 5xx, per @x402/express) also get miscounted as fresh challenges. 403 rejections aren't counted at all.
None of this touches payment verification/settlement itself — it's metrics-only, so no money-safety issue — but the flagship metric doesn't measure what it claims to right now. Can you re-check the classifier against a real (or schema-faithful) 402 response body from @x402/core instead of the hand-built fixtures in metrics.test.ts? Happy to re-review once that's addressed.
🤖 Generated with Claude Code
Update: addressed review feedbackThe original classifier assumed
Fix:
Also separated infrastructure errors from settlement failures. Traced Revenue extraction now reads pricing from the decoded header's Fixtures in Tests: 41 passing (up from 28), including new coverage for settlement-failure detection, Known limitations (updated):
|
|
Reviewed the actual implementation and ran the test suite locally: 41/41 passing. This is solid work — x402Metrics wraps x402Serve() as a pure observer (only patches res.json/res.setHeader, never res.send/res.end, avoiding conflicts with @x402/express's internal buffering), and classifies outcomes by decoding the real PAYMENT-REQUIRED header rather than guessing from status code alone. The test fixtures are validated against @x402/core's own schema (parsePaymentRequired), not just internally-consistent mocks — good practice. Only blocker right now is that this branch is behind main and has a merge conflict in packages/sdk/package.json / package-lock.json (both PRs touched devDependencies). Could you rebase/merge main into feat/x402-metrics-wrapper and push? Once that's resolved this is ready to merge. |
On it now , will update branch immediately |
…pper # Conflicts: # packages/sdk/package-lock.json # packages/sdk/package.json
Resolve merge conflict in package-lock.json by regenerating from combined devDependencies (ours + main's @types/ws@^8.18.1 upgrade). Lock ws@^8.18.0 in package.json.
|
@Eras256 updated the branch as you requested |
- Kept main's CommonJS/ts-jest tsconfig (already proven with nirium-protocol#53's tests) instead of this branch's NodeNext config, to avoid destabilizing what's already merged. - Dropped an unused `viem` dependency this branch had added (Ethereum library, never referenced anywhere in resilient-ws.ts) — same class of leftover already caught and removed in PR nirium-protocol#58. - test/ws-resilient.test.ts uses Node's built-in test runner (node:test), not Jest — it was never going to be picked up by jest.config.js's `roots: ['<rootDir>/src']` regardless of this merge. Wired it into `npm test` via `node --experimental-strip-types --test test/*.test.ts` so it actually runs going forward instead of silently never executing.
…sh & deduplication (#61) Resolved the merge conflict against current main (package.json/tsconfig.json/package-lock.json — jest/ts-jest infra added by #53 after this branch was opened). Also dropped an unused viem dependency this branch had picked up, and fixed something the conflict exposed: test/ws-resilient.test.ts uses Node's built-in test runner, not Jest, so it was never actually being executed by `npm test` — wired it in properly. Verified: 43/43 tests passing (41 Jest + 2 real WebSocket reconnect/dedup tests via node --test).
Summary
Adds an opt-in metrics wrapper for
x402Serve()that exposes Prometheus counters and histograms for challenges, verifications, settlements, revenue, and latency — without modifying any payment-verification behavior.Closes #41
What's included
New
packages/sdk/src/metrics.ts—x402Metrics(x402Serve(config), options?), a pure wrapper around an existingx402Servehandler. Returns{ handler, metricsHandler, snapshot, reset }.handler— drop-in replacement for the wrappedx402Serveinstance; observes outcomes without altering them.metricsHandler—GET /metricsroute helper in Prometheus text exposition format, mountable independently of any paid route and requiring no payment itself.snapshot/reset— for tests and diagnostics.packages/sdk/src/metrics.test.ts(28 tests) — mockedx402Servehandler covering challenge/verify/settle counting, revenue correctness across a scripted paid/failed sequence, multi-asset revenue, latency histogram, Prometheus output format, and PII exclusion.packages/sdk/src/x402serve-smoke.test.ts(7 tests) — mocks the ESM-only deps (ws,@x402/fetch,@x402/stellar,mppx) to provex402Serve's own validation and behavior are unchanged when wrapped byx402Metrics.@x402/corePaymentRequiredV2SchemaviaparsePaymentRequired()(installed as a devDependency only, never imported at runtime), so the fixtures can't silently drift from what the dependency actually emits on the wire.packages/sdk/jest.config.js, roottsconfig.json(was missing from git —packages/sdk/tsconfig.json'sextends: "../../tsconfig.json"was pointing at a file that didn't exist).Changed
packages/sdk/src/index.ts— two-line addition exportingx402Metricsand its types. No changes tox402Serveor any payment logic.packages/sdk/README.md— new "x402 Metrics" section with a usage example,curlexample scraping/metrics, and sample Prometheus output.packages/sdk/package.json— addedjest,ts-jest,@types/jest,@types/node,@x402/core(test-only) to devDependencies.Metrics exposed
x402_challenges_totalroutex402_verify_success_totalroutex402_verify_fail_totalroutex402_settle_success_totalroutex402_settle_fail_totalroutex402_revenue_totalroute,assetx402_settlement_latency_secondsrouteNo payer addresses or other PII are included — aggregate counters/histograms only.
How classification works
Outcomes are read from the real x402 v2 protocol shape rather than inferred from status codes alone:
errorfield → challenge issued (fresh payment request)errorpresent → verify failureRevenue uses the
routesconfig passed tox402Metricswhen provided (reliable, since price is known upfront); when not provided, it falls back to parsing the price out of the 402 challenge body (accepts[0].amount, with a defensive|| accepts[0].maxAmountRequiredfallback for v1-shaped responses).Testing
npm run test)npm run build)rm -rf node_modules && npm ci --legacy-peer-deps && npm run build && npm run test@x402/express/@x402/core/@x402/stellarpayment logic is exercised in tests — everything is mocked at the handler boundary, so tests never hit the live OpenZeppelin facilitator.@x402/core's schema module is used only to validate that mock fixtures match its real shape, not to run any payment flow.Known limitations
@x402/coreemits by default (x402Version: 2,resourceas an object,amountfield). A v1-shaped response (maxAmountRequired, flat stringresource) is handled defensively in the revenue fallback but isn't exercised by a dedicated test.accepts/route config, not an independently-verified on-chain settlement amount — there's no way to observe the actual settled amount from outside the middleware without deeper integration.@x402/expressmiddleware end-to-end (only mocked at the handler/schema boundary), per the constraint of not installing/exercising real payment-verification packages in CI.Note on peer dependencies
packages/sdk/package.jsonpins@stellar/stellar-sdk@^14.5.0directly, but@stellar/mppdeclares a peer dependency on@stellar/stellar-sdk@^15.1.0. This predates this PR butnpm installin this package currently requires--legacy-peer-depsto complete; flagging here in case it's worth a separate fix.