0xMiden / 0xMiden/node

Telemetry: record gRPC status codes on request spans; reserve span errors for internal faults

Aperta
#2,507 1 commento 0 reazioni 1 assegnatario Rivendicata da @sergerad Vedi su GitHub
Lingua principale
Rust
Stelle
104
Fork
138
Merge medio
1g 13h
PR unite (30g)
56

Descrizione

## Problem

User-caused RPC failures (invalid proofs, malformed or stale transactions, mempool
rejections) are recorded the same way as internal faults: the span gets OTel `error`
status via `set_error`. In Honeycomb both look identical — `error` exists on the
event — so client noise is indistinguishable from node failures.

So a burst of bad client submissions on devnet:

- floods error-based alerts (our "High Failure Rate" Honeycomb trigger had to
exclude `rpc.Api` root spans as a stopgap, blinding it to genuine faults on
that surface);
- pollutes error-rate views used for debugging real incidents;
- propagates error status up the span tree, so one invalid tx produces several
"error" events (`rpc.Api` root, internal sequencer submit span, mempool span).

## Proposal

Two independent changes to request-span telemetry:

**1. Always record the gRPC status code on request root spans.**
`grpc_trace_fn` (`crates/utils/src/tracing/grpc.rs`) creates the root span per
request but never records the response status. Add `rpc.grpc.status_code`
(OTel RPC semconv) to every request span — `0` (OK) on success, the actual
code on failure. The field is always present; its value distinguishes outcomes.

**2. Only mark spans as errors for codes that indicate a node fault.**

| gRPC status code | Example in miden-node | Span error status? |
|---|---|---|
| `OK` (0) | successful submit | no |
| `INVALID_ARGUMENT` | malformed tx, proof verification failure | no — client's fault |
| `FAILED_PRECONDITION` | expired tx, stale auth height, mempool state conflict | no — client's fault |
| `RESOURCE_EXHAUSTED` | mempool `CapacityExceeded`, rate limiting | no — but see note below |
| `INTERNAL` / `UNKNOWN` | DB failure, proving failure, bugs | **yes** — `set_error` as today |
| `UNAVAILABLE` | store unreachable, shutdown races | **yes** |

The same rule applies to internal spans along the request path: handling an
invalid tx is *successful rejection*, not an application error, so mempool /
sequencer spans processing it should not be error-marked either.

Note on `RESOURCE_EXHAUSTED`: not a node bug, so no error mark — but it signals
load. It stays alertable via the always-present status code (and mempool
telemetry already tracks capacity pressure directly).

## Resulting query semantics (Honeycomb)

- "users are failing to submit" → `rpc.grpc.status_code != 0` on `rpc.Api` spans
- "the node is misbehaving" → `error exists` (now meaningful again)
- "mempool is saturated" → `rpc.grpc.status_code = RESOURCE_EXHAUSTED`

## Touchpoints

- `crates/utils/src/tracing/grpc.rs` — record `rpc.grpc.status_code` on the
root span when the response status is known
- `crates/utils/src/tracing/span_ext.rs` — `ErrorSpanExt::set_error` callers on
user-input failure paths (RPC handlers, block-producer submit API, mempool
submission errors) stop error-marking client-caused failures

## Downstream (infrastructure repo)

- "High Failure Rate" trigger — drop the `rpc.service != rpc.Api` stopgap filter
- "Transaction Submission Failures" trigger — switch from `error exists` to
`rpc.grpc.status_code != 0`

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.