feat: track json-rpc metrics - #25159
Merged
Merged
Conversation
alexghr
enabled auto-merge
August 10, 2026 15:48
alexghr
force-pushed
the
stack/feat-track-json-rpc-metrics
branch
2 times, most recently
from
August 10, 2026 16:54
3580550 to
60b2228
Compare
alexghr
force-pushed
the
stack/feat-track-json-rpc-metrics
branch
from
August 12, 2026 13:46
60b2228 to
10007f1
Compare
spalladino
approved these changes
Aug 12, 2026
| ); | ||
| if (apiKeyResolution) { | ||
| adminMiddlewares.unshift(getApiKeyAuthMiddleware(apiKeyResolution.apiKeyHash)); | ||
| adminMiddlewares.splice(1, 0, getApiKeyAuthMiddleware(apiKeyResolution.apiKeyHash)); |
Contributor
There was a problem hiding this comment.
Is this because we want the api key auth to run after metrics? If so, let's add it in order in the adminMiddlewares above, this splice(1) seems not very robust. I'm thinking something like:
const adminMiddlewares = [
getOtelJsonRpcServerMetricsMiddleware(),
...(apiKeyResolution ? [getApiKeyAuthMiddleware()] : []),
getOtelJsonRpcPropagationMiddleware(),
getVersioningMiddleware(versions, versioningOpts),
];
Comment on lines
+49
to
+50
| ...(service === undefined ? {} : { [ATTR_JSONRPC_SERVICE]: service }), | ||
| [ATTR_JSONRPC_METHOD]: method, |
Contributor
There was a problem hiding this comment.
Can this be a problem in terms of attribute cardinality if we log every request done, even when the service/method doesn't exist? Eg if I start poking an RPC endpoint with aztec_1, aztec_2, etc, can I screw up prometheus?
Contributor
Author
There was a problem hiding this comment.
the known methods would be routed to rpc.method=unknown
| const method = this.handler[methodName as keyof T]; | ||
| assert(typeof method === 'function', `Method ${methodName} is not a function`); | ||
| const args = await parseWithOptionals(jsonParams, getSchemaParameters(this.schema[methodName])); | ||
| const validationTimer = new Timer(); |
Contributor
There was a problem hiding this comment.
Should we set validationSucceeded to false in these branches? Or do we have something else that collects exceptions and flags the request as failed?
spalladino
reviewed
Aug 12, 2026
spalladino
left a comment
spalladino
left a comment
Contributor
There was a problem hiding this comment.
Claude also flagged the following, meat-proxying:
Two rejection reasons are unreachable, and real internal errors never appear in the metric — telemetry-client/src/json_rpc_server_metrics.ts:119-146. Handler-thrown errors (including BadRequestError) are caught inside processRequest and mapped to -32701/-32702, which getRejectionReasons ignores. The -32000 and -32600 + status 500 branches only fire for errors thrown outside processRequest, which nothing currently does. So an operator alerting on rejection_reason=internal_error never trips it even while every handler is throwing. Conversely, an uncaught rejection inside a batch item comes out -32600 / HTTP 200 and gets mislabeled invalid_request. Either drop the dead reasons or classify from the codes the server actually emits.
alexghr
force-pushed
the
stack/feat-track-json-rpc-metrics
branch
3 times, most recently
from
August 14, 2026 10:54
3b7c6ea to
849195d
Compare
alexghr
force-pushed
the
stack/feat-track-json-rpc-metrics
branch
from
August 14, 2026 12:10
849195d to
36c1dc4
Compare
AztecBot
pushed a commit
that referenced
this pull request
Aug 14, 2026
Add metrics to the JSON-RPC server. Fix A-1677.
Collaborator
|
❌ Failed to cherry-pick to |
alexghr
added a commit
that referenced
this pull request
Aug 14, 2026
Add metrics to the JSON-RPC server. Fix A-1677.
alexghr
added a commit
that referenced
this pull request
Aug 14, 2026
Add metrics to the JSON-RPC server. Fix A-1677.
Merged
aminsammara
added a commit
that referenced
this pull request
Aug 17, 2026
Promotes `v5-next` onto `v5` for the **v5.2.0** release. Frozen at `ee5d2d367e` — the `v5-next` tip at cut time. Both `merge-train/spartan-v5` and `merge-train/fairies-v5` are drained to that same commit, so nothing staged is left behind. ### Testnet validation This branch was cut at `a4db216abf`, which is byte-identical to the `v5.2.0-nightly.20260815` tag — same commit, same tree (`0b22572eace2419ede6bbab173514d6965d4e0d0`). That nightly's CI3 run is green, its artifacts are published to npm and Docker Hub, and it has been **running healthily on testnet since 2026-08-15**: | Component | Image / reported version | |---|---| | validators, prover node, prover broker, prover agents (`testnet` ns) | `aztecprotocol/aztec:5.2.0-nightly.20260815` | | `v5.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | | `canonical.testnet.rpc.aztec-labs.com` | `nodeVersion=5.2.0-nightly.20260815` | Chain advancing normally, no prunes or reorgs, no WARN/ERROR across node pods since rollout. The branch has since been fast-forwarded to `ee5d2d367e` to pick up #25242 (configurable RPC server HTTP timeouts and CORS allowed-headers). That is the only delta from the soaked tree — 8 files, +131/−10, no nightly covers it yet. Its config defaults were checked against Node's built-ins (`keepAliveTimeout` 5000 ms, `headersTimeout` 60000 ms) and match exactly, and the CORS default path resolves to the same `cors()` call as before, so a node that sets none of the new env vars behaves identically. It touches no circuits, protocol contracts, or generated constants. ### Manifest `.release-please-manifest.json` reads `5.2.0` on this branch, which is the released version — correct as-is, no change needed in this PR. `v5-next` moves to `5.3.0` separately in #25240; this branch is frozen and cannot pick that up, so the two can merge in either order. `v5` is a strict ancestor of `v5-next` this cycle, so there was no manifest conflict to pre-resolve. ### Scope 139 commits (86 non-merge, 23 PR-level) spanning 2026-07-14 to 2026-08-17. | Area | Theme | |---|---| | `prover-node` / `prover-client` | Epoch-proving robustness: retry-to-converge with failure declared only at submission-window expiry, ticker-driven expiry, per-checkpoint post-mortem upload, checkpoint-only re-proving, prune-induced fault handling (#24678, #24982, #24983, #24990, #25027) | | `pxe` | Sync performance: hash-pinned node read cache (#24969), anchor-bounded tag log caching (#25074), note/event validation from cached tx data (#25076), constrained tag sync (#24275), sender tagging finalization from log blocks (#25045) | | `p2p` / `validator-client` | Gossip tx validation no longer stalls behind tx-pool finalization (#25148); startup fails when p2p fails to start (#25177); slashing/proposing/health/`sendTx` gated on p2p connectivity (#25185); duplicate time-sensitive proposal validation removed (#25207); `ValidatedProposal` branding (#25222) | | `ethereum` / `aztec-node` | L1 watchers poll `getLogs` instead of `eth_newFilter` (#25176); block stream sync no longer resolves against an earlier pass (#25206) | | `stdlib` / `foundation` | Deserialization bounds hardening (#25026, #25028, #25029, #25109); checkpoint block-shape and block-count validation (#25229); JSON-RPC cookies (#25231) | | `archiver` | Removed-block cleanup and ownership-checked tx-effect deletes (#24765); L2→L1 witness from a single store snapshot (#24754) | | `aztec` | Declarative deployment framework at `@aztec/aztec/deploy` (#24685) | | `slasher` | Own-validator slash-target warnings and metrics (#25058) | | telemetry | JSON-RPC metrics (#25159) | | JSON-RPC server | Configurable HTTP keep-alive / headers timeouts and CORS allowed-headers, defaults preserving current behaviour (#25242) | | toolchain | Noir bumped to `v1.0.0-beta.25` (#24907) | **One breaking change**, inherited from the Noir bump: note types declared directly inside a `contract` module must now be `pub`. Everything else is additive or internal. ### Protocol constants Built from source and compared against `v5` — a cache-free rebuild of every protocol circuit with each ref's own nargo and bb, then regeneration of `vk_tree.ts` and `protocol_contract_data.ts`: - `vkTreeRoot` = `0x2b3b6ea4412b9c8f6457a37f91a2870306f8641e07e16a49b68bda6f8bc02892` — unchanged from v5.1.0 - `protocolContractsHash` = `0x2c075866eafc88a1f6f9addc7e337c6e64e45d1cb7fd7c0d612ebcec72aab2ca` — unchanged from v5.1.0 The Noir beta.24 → beta.25 bump does not reach the circuits: the release build consumes the committed `pinned-build.tar.gz`, which is bit-identical between v5.1.0 and this commit (blob `3bedcb1fd1…`), so the protocol-circuit bytecode is frozen rather than recompiled. The 47 verification keys were recomputed locally from that pinned bytecode with the cache disabled, and all 47 `check_pinned_vk` checks passed. `protocolContractsHash` is likewise backed by the new `pinned-protocol-contracts.tar.gz`, whose three artifacts were byte-compared against the build. Both values were also confirmed inside the published `@aztec/protocol-contracts` and `@aztec/noir-protocol-circuits-types` packages for `5.2.0-nightly.20260815`. That makes v5.2.0 a drop-in upgrade against the current rollup rather than a coordinated one. ✅ **Gate re-run against `ee5d2d367e` (the current head) and passed** — both values reproduced exactly from a build at this commit, with `vkTreeRoot` numerically evaluated rather than inferred. `29556326ce..ee5d2d3` touches no `noir-projects/**`, `l1-contracts/src/**`, `ConstantsGen.sol`, `constants.gen.ts` or `constants.nr`. Scope of the check: it verifies that the pinned circuit bytecode plus locally recomputed VKs agree with the pin — not that a from-source recompile reproduces v5.1.0's bytecode. <details> <summary>Commits added after the original constants check at 2955632</summary> `#25207` `#25222` `#25206` `#25185` `#25176` `#25163` `#25229` `#25159` `#25162` `#25231` `#25224` `#25228` `#25230` `#25242` plus their merge commits. </details> ### Known gap Migration notes carry entries under `## TBD` that arrived with the docs baseline backport (#25017) and describe `next`-line changes not present on the v5 line — protocol contracts removed from `@aztec/noir-contracts.js`, and the `at(wallet)` → `withWallet(wallet)` deprecation. Neither exists on this branch. There is also no `## 5.1.0` heading, and the entries that ship in v5.2.0 sit under `## TBD` rather than a version heading. This ships in the release docs, so it is worth correcting on `v5-next` and backporting to `v5` before the tag, rather than after.
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
3 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.
Add metrics to the JSON-RPC server.
Fix A-1677.