diff --git a/crates/starknet_transaction_prover/README.md b/crates/starknet_transaction_prover/README.md index 2450cb48866..bbba0572439 100644 --- a/crates/starknet_transaction_prover/README.md +++ b/crates/starknet_transaction_prover/README.md @@ -307,6 +307,7 @@ scrapes bypass CORS and JSON-RPC parsing, and the endpoint is unauthenticated. | `prover_prove_transaction_duration_seconds` | histogram | `outcome` | Duration of the whole proving call, covering input validation, the optional blocking check, the virtual OS run and proving. Recorded for failures too, so filter on `outcome` for success-only percentiles. | | `prover_os_run_duration_seconds` | histogram | none | Virtual OS execution time, recorded for successful runs only. | | `prover_stwo_prove_duration_seconds` | histogram | none | STWO proving time, recorded for successful runs only. Emitted only by builds with the `stwo_proving` feature. | +| `prover_panics_total` | counter | none | Process panics caught by the global panic hook. Lets an alert watch the panic rate without a log search. | No user-controlled value becomes a label, so label cardinality stays bounded. @@ -372,14 +373,15 @@ follow-up Ctrl+C would do nothing. ### Panics -A global panic hook catches every panic and logs one `error`-level event with `event="panic"`. The -event carries the panic location, the payload, and a forced backtrace. Only static string literals -reach the log verbatim. A payload built at runtime can hold request or transaction data, so the -hook replaces it with a placeholder. +A global panic hook catches every panic. The hook increments `prover_panics_total` before it logs, +so a recursive panic inside the logging cannot lose the count. It then logs one `error`-level event +with `event="panic"`. The event carries the panic location, the payload, and a forced backtrace. +Only static string literals reach the log verbatim. A payload built at runtime can hold request or +transaction data, so the hook replaces it with a placeholder. -The hook only logs. It does not call `process::abort()` and does not change unwinding behavior, so -the tokio runtime still contains a panic raised inside a request task and the process keeps -serving. +The hook only logs and counts. It does not call `process::abort()` and does not change unwinding +behavior, so the tokio runtime still contains a panic raised inside a request task and the process +keeps serving. A rising `prover_panics_total` is therefore the signal to alert on. ## Limitations diff --git a/crates/starknet_transaction_prover/src/server/metrics.rs b/crates/starknet_transaction_prover/src/server/metrics.rs index 0fc94d417d4..a4f872fbb4f 100644 --- a/crates/starknet_transaction_prover/src/server/metrics.rs +++ b/crates/starknet_transaction_prover/src/server/metrics.rs @@ -67,6 +67,8 @@ const HTTP_DURATION_BUCKETS: &[f64] = pub mod names { /// Build identity. Always 1, labelled with `version` and `git_sha`. pub const BUILD_INFO: &str = "prover_build_info"; + /// Unhandled panics recorded by the global panic hook. + pub const PANICS_TOTAL: &str = "prover_panics_total"; /// Wall-clock duration of the whole `prove_transaction` call (validation, /// blocking check, OS run, proving), labelled by `outcome` so a query can /// separate success latency from failure latency. Bucketed. @@ -112,6 +114,8 @@ pub fn install_exporter(version: &str, git_sha: &str) -> anyhow::Result git_sha.to_string(), ) .set(1.0); + // Pre-register at zero so the series exists in scrapes before the first panic. + metrics::counter!(names::PANICS_TOTAL).increment(0); super::http_metrics::preregister_http_metrics(); Ok(handle) } diff --git a/crates/starknet_transaction_prover/src/server/panic.rs b/crates/starknet_transaction_prover/src/server/panic.rs index 28c4e687e57..c9296b0ac79 100644 --- a/crates/starknet_transaction_prover/src/server/panic.rs +++ b/crates/starknet_transaction_prover/src/server/panic.rs @@ -1,14 +1,17 @@ //! Process-wide panic hook. It replaces the default stderr output with one //! structured `tracing` event carrying the panic location and a backtrace, -//! which log aggregators can index. The hook only logs. It does not change -//! unwinding behavior, so the tokio runtime still contains a panic raised -//! inside a request task and the process keeps serving. +//! which log aggregators can index. It also increments `prover_panics_total`. +//! The hook only logs and counts. It does not change unwinding behavior, so the +//! tokio runtime still contains a panic raised inside a request task and the +//! process keeps serving. use std::backtrace::Backtrace; use std::panic::PanicHookInfo; use tracing::error; +use crate::server::metrics::names::PANICS_TOTAL; + #[cfg(test)] #[path = "panic_test.rs"] mod panic_test; @@ -18,6 +21,8 @@ pub fn install_panic_hook() { } fn panic_hook(info: &PanicHookInfo<'_>) { + // Increment first so a recursive panic in the logging below can't lose the count. + metrics::counter!(PANICS_TOTAL).increment(1); let payload = extract_payload(info); let location = info .location() diff --git a/crates/starknet_transaction_prover/src/server/panic_test.rs b/crates/starknet_transaction_prover/src/server/panic_test.rs index 7c552655399..398cf730b1f 100644 --- a/crates/starknet_transaction_prover/src/server/panic_test.rs +++ b/crates/starknet_transaction_prover/src/server/panic_test.rs @@ -1,13 +1,20 @@ use std::panic::UnwindSafe; +use std::sync::Mutex; use tracing_test::traced_test; +use crate::server::metrics::names::PANICS_TOTAL; use crate::server::panic::install_panic_hook; +use crate::server::test_recorder::{metric_value, shared_handle}; + +/// Serializes the tests that install the global panic hook and read the shared +/// `prover_panics_total` counter, so their before/after deltas don't interleave. +static PANIC_HOOK_TEST_LOCK: Mutex<()> = Mutex::new(()); -// The panic hook is global state, so a single #[test] keeps the captures serial. #[test] #[traced_test] fn logs_structured_event_with_location_payload_and_backtrace() { + let _guard = PANIC_HOOK_TEST_LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); // Recorded right next to the panic, so a hook that stops reading // `info.location()` (or hardcodes it) fails here instead of passing on a // file-only match. @@ -78,6 +85,18 @@ fn logs_structured_event_with_location_payload_and_backtrace() { }); } +#[test] +fn panic_hook_bumps_panics_total_counter() { + let _guard = PANIC_HOOK_TEST_LOCK.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + let handle = shared_handle(); + let before = metric_value(&handle.render(), PANICS_TOTAL); + + catch_panic_under_hook(|| panic!("counter-test panic")); + + let after = metric_value(&handle.render(), PANICS_TOTAL); + assert_eq!(after - before, 1.0); +} + /// Runs `panicking_body` with the service panic hook installed and swallows /// the unwind. Restores whichever hook was installed before. fn catch_panic_under_hook(panicking_body: impl FnOnce() + UnwindSafe) {