feat(metrics): profile local sqlite storage - #5612
Conversation
NathanFlurry
commented
Aug 28, 2026
- Add bounded local SQLite statement, transaction, VFS I/O, and amplification profiling.
- Add named transaction and profiling configuration APIs across Rust, TypeScript, NAPI, and wasm.
- Verify native VFS-to-Prometheus reporting, fingerprint admission, disabled behavior, and API parity.
|
🚅 Deployed to the actors-pr-5612 environment in rivet-frontend
|
Review: feat(metrics): profile local sqlite storageThis is a large, well-tested addition (bounded SQLite statement/transaction/VFS profiling, wired through Rust core -> NAPI -> wasm -> TS). Overall the design is careful about the things CLAUDE.md calls out most (bounded cardinality via admission control, Findings1. Fingerprint/series admission budget is process-global, not per-actor (
Concretely:
If two actor kinds in the same process configure different Given metrics labels are already namespaced by 2. Substantial duplicated profiling boilerplate that's easy to let drift
None of this is wrong, but it's a lot of copy-pasted control flow (not "three similar lines") gating on the same Smaller notes (not blocking)
Positives
|
94b3d24 to
d4a75ea
Compare
|
Review follow-up:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4a75ea36e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "fingerprint_source", | ||
| "transaction_mode", | ||
| "storage_transport", | ||
| "outcome_class", |
There was a problem hiding this comment.
Use
result for operation outcome labels
Rename this label, and the new outcome label on rivetkit_sqlite_outcome_total, to the repository-standard result. As written, the new SQLite operation metrics will not compose with dashboards and alerts that consistently group operation success/failure by result, requiring one-off queries for this subsystem.
AGENTS.md reference: AGENTS.md:L306-L310
Useful? React with 👍 / 👎.
| .buckets(vec![ | ||
| 512.0, | ||
| 4096.0, | ||
| 16_384.0, | ||
| 65_536.0, | ||
| 262_144.0, | ||
| 1_048_576.0, | ||
| ]), |
There was a problem hiding this comment.
Move the response-size buckets into the shared constants
Replace this newly invented bucket vector with an existing constant from engine/packages/metrics/src/buckets.rs, or add an appropriate shared constant there. Keeping a private bucket scheme here makes histogram aggregation and dashboard thresholds inconsistent with the rest of the repository.
AGENTS.md reference: AGENTS.md:L311-L313
Useful? React with 👍 / 👎.
| if set.contains_sync(fingerprint) { | ||
| return (fingerprint.to_owned(), false); | ||
| } |
There was a problem hiding this comment.
Enforce fingerprint caps per profiling configuration
When another actor type has already admitted the same fingerprint into this process-global set, this early return bypasses the current actor's maxTrackedStatementFingerprints or maxTrackedTransactionFingerprints. For example, after actor A admits a query, actor B configured with a cap of zero still creates fingerprint-specific series for it; differing actor configurations therefore behave according to startup history rather than their own limits. Scope admission membership by actor/configuration (as the candidate and tuple keys already partly do), or apply a single explicitly process-global cap.
Useful? React with 👍 / 👎.
ecefbfe to
3d5aeea
Compare
372e417 to
2904bf8
Compare
2904bf8 to
32b9223
Compare
32b9223 to
73458a7
Compare
73458a7 to
b920136
Compare
b920136 to
f2dc39b
Compare
f2dc39b to
21b6075
Compare
21b6075 to
c634df4
Compare
c634df4 to
28682f8
Compare
28682f8 to
799b738
Compare
799b738 to
23e3bcf
Compare
23e3bcf to
bcfc5e7
Compare
bcfc5e7 to
8219ba0
Compare
8219ba0 to
d906ff2
Compare
d906ff2 to
c7d5916
Compare
c7d5916 to
5809bd1
Compare
5809bd1 to
eac581d
Compare