Replace prom-client with hand-rolled Prometheus text rendering - #1441
Conversation
Delete Prometheus.res and the prom-client default registry usage. Counters now live on IndexerState (batch/handler/storage/rollback stats), ChainState (block-range fetch, reorg, latency), SourceManager (status seconds, source heights) and EffectState (per-effect call/cache/queue stats), while gauges are derived from live state at scrape time. Metrics.res renders the /metrics text with declarative builder helpers: blank line between metrics, values rounded to 3 decimals. prom-client remains only for /metrics/runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces direct Prometheus updates with runtime state counters, adds callback-based storage timing, updates indexing and source instrumentation, and renders synchronous Prometheus text from collected ChangesState-backed metric contracts
Runtime instrumentation migration
Synchronous metric rendering
Validation and compatibility
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant IndexerState
participant Metrics
participant MetricsEndpoint
Runtime->>IndexerState: Record runtime measurements
MetricsEndpoint->>IndexerState: toMetrics
MetricsEndpoint->>Metrics: collect(metrics)
Metrics->>Metrics: renderMetrics
Metrics-->>MetricsEndpoint: Return Prometheus text
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ae65dc5e2
ℹ️ 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".
| ->Array.map((s: IndexerState.handlerStat) => ( | ||
| `{contract="${s.contract}",event="${s.event}"}`, | ||
| s, | ||
| )) |
There was a problem hiding this comment.
When a configured contract/event/effect/source name contains ", \, or a newline, these interpolated labels produce invalid Prometheus exposition text (for example, a quote terminates contract early), causing Prometheus to reject the /metrics scrape. The previous prom-client label renderer escaped these values; route all dynamic label values through a Prometheus label-escaping helper before assembling the label string.
Useful? React with 👍 / 👎.
| state->IndexerState.recordStorageWrite( | ||
| ~storage=persistence.storage.name, | ||
| ~timeSeconds=timerRef->Performance.secondsSince, | ||
| ) |
There was a problem hiding this comment.
With a ClickHouse sink configured, this records one storage="postgres" sample for the whole composite PgStorage.writeBatch call, including any time spent awaiting the sink, and no longer emits the sink's own storage="clickhouse" count or duration. Previously PgStorage.writeBatchMethod timed the sink promise and Postgres write independently, so dashboards can no longer distinguish a slow/failing sink from Postgres (and a successful sink followed by a Postgres failure is now uncounted).
Useful? React with 👍 / 👎.
| b->seriesOpt( | ||
| ~name="envio_effect_cache", | ||
| ~help="The number of items in the effect cache.", | ||
| ~kind="gauge", | ||
| ~entries=effects, | ||
| ~value=s => s.cacheCount > 0 ? Some(s.cacheCount->Int.toFloat) : None, |
There was a problem hiding this comment.
For an existing but empty persisted effect-cache table, makeFromDbState now seeds cacheCount with zero, but this predicate suppresses its sample. Before this change, the startup loop called EffectCacheCount.set for every discovered cache table, including zero-row tables, so dashboards could distinguish an empty persistent cache from an effect with no cache metric at all.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/envio/src/ChainState.res (1)
819-846: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign comments with the state-backed metric flow. The implementation now stores metric state for scrape-time rendering; these comments either describe the old direct-emission behavior or restate obvious structure.
packages/envio/src/ChainState.res#L819-L846: change “Emits the per-chain progress metrics” to describe updating metric state, or remove that clause.packages/envio/src/ChainState.res#L32-L32: remove the downstream-renderer narration.packages/envio/src/ChainState.res#L371-L371: remove the redundant section comment.As per coding guidelines,
**/*.rescomments must not restate code, identify callers, or narrate a refactor.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/ChainState.res` around lines 819 - 846, Update the comments in packages/envio/src/ChainState.res at lines 819-846, 32, and 371: revise the applyBatchProgress comment to say it updates metric state rather than emits metrics, and remove the downstream-renderer narration and redundant section comment at the two sibling sites; do not change the implementation.Source: Coding guidelines
packages/envio/src/EventProcessing.res (1)
173-207: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlways finalize preload metrics when the handler fails.
A synchronous throw or rejected promise skips
endPreloadHandler, permanently leakingpreloadPendingCount. Subsequent preload wall-clock metrics for this handler will never close.Proposed fix
let timerRef = indexerState->IndexerState.startPreloadHandler( ~contract=contractName, ~event=eventName, ) + let finish = () => + indexerState->IndexerState.endPreloadHandler( + timerRef, + ~contract=contractName, + ~event=eventName, + ) promises->Array.push( handler({ event: item->Ecosystem.getItemEvent(~ecosystem=config.ecosystem), context: UserContext.getHandlerContext({ item, indexerState, loadManager, persistence, checkpointId, isPreload: true, chains, isResolved: false, config, }), }) - ->Promise.thenResolve(_ => { - indexerState->IndexerState.endPreloadHandler( - timerRef, - ~contract=contractName, - ~event=eventName, - ) - }) - ->Utils.Promise.silentCatch, + ->Promise.thenResolve(_ => finish()) + ->Promise.catch(_ => finish()), ) } catch { - | _ => () + | _ => + indexerState->IndexerState.endPreloadHandler( + timerRef, + ~contract=contractName, + ~event=eventName, + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/EventProcessing.res` around lines 173 - 207, Update the preload handler flow around startPreloadHandler and handler so endPreloadHandler is invoked for both synchronous throws and rejected promises, not only successful Promise.thenResolve completion. Ensure each started timerRef is finalized exactly once while preserving the existing silent error handling.
🧹 Nitpick comments (1)
packages/envio/src/Metrics.res (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the refactor narration.
This comment describes the module purpose and the removal of
prom-client, rather than a non-obvious constraint. As per coding guidelines, “Never narrate the refactor itself.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/Metrics.res` around lines 1 - 3, Remove the top-of-file comment in Metrics.res that narrates the refactor and mentions prom-client; leave the implementation unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/envio/src/Metrics.res`:
- Around line 20-31: The metrics pipeline currently treats labels as opaque
strings, allowing delimiters and special characters to corrupt Prometheus output
and parsing. In packages/envio/src/Metrics.res:20-31, update series and related
rendering to accept structured labels and centrally escape quotes, backslashes,
and newlines; in scenarios/test_codegen/test/helpers/MockIndexer.res:625-655,
parse quoted and escaped label values instead of splitting directly on commas or
equals signs; in scenarios/test_codegen/test/lib_tests/Metrics_test.res:16-27,
add whole-output regression cases covering escaped labels and quoted delimiters.
In `@packages/envio/src/PgStorage.res`:
- Around line 1782-1788: Preserve the sink-specific timer metrics in the
writeBatch path instead of returning only the exception from the Promise chain.
Update the sink handling around sink.writeBatch and the corresponding
IndexerState accumulation to return or pass the sink duration/count recording,
while retaining the existing Some(exn) failure behavior and outer
persistence.storage.name metrics.
In `@packages/envio/src/sources/SourceManager.res`:
- Around line 126-128: Update the idleSeconds, waitingForNewBlockSeconds, and
queryingSeconds accessors to include the elapsed time from the current
statusStart via Performance.secondsSince, in addition to each stored counter.
Ensure scrape results reflect the active interval before the next status
transition.
In `@scenarios/test_codegen/test/lib_tests/Metrics_test.res`:
- Around line 16-27: Add whole-output assertions in the metrics rendering test
around the existing b.out expectation, covering label values containing quotes,
backslashes, newlines, commas, and equals signs. Verify the rendered output uses
the shared escaping and delimiter contract while preserving the existing metric
lines and values.
---
Outside diff comments:
In `@packages/envio/src/ChainState.res`:
- Around line 819-846: Update the comments in packages/envio/src/ChainState.res
at lines 819-846, 32, and 371: revise the applyBatchProgress comment to say it
updates metric state rather than emits metrics, and remove the
downstream-renderer narration and redundant section comment at the two sibling
sites; do not change the implementation.
In `@packages/envio/src/EventProcessing.res`:
- Around line 173-207: Update the preload handler flow around
startPreloadHandler and handler so endPreloadHandler is invoked for both
synchronous throws and rejected promises, not only successful
Promise.thenResolve completion. Ensure each started timerRef is finalized
exactly once while preserving the existing silent error handling.
---
Nitpick comments:
In `@packages/envio/src/Metrics.res`:
- Around line 1-3: Remove the top-of-file comment in Metrics.res that narrates
the refactor and mentions prom-client; leave the implementation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10c52092-334d-4fd0-855c-50354dd57d2d
📒 Files selected for processing (23)
packages/envio/src/ChainFetching.respackages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/CrossChainState.respackages/envio/src/CrossChainState.resipackages/envio/src/EffectState.respackages/envio/src/EffectState.resipackages/envio/src/EventProcessing.respackages/envio/src/FetchState.respackages/envio/src/IndexerState.respackages/envio/src/IndexerState.resipackages/envio/src/LoadLayer.respackages/envio/src/Main.respackages/envio/src/Metrics.respackages/envio/src/PgStorage.respackages/envio/src/Prometheus.respackages/envio/src/PruneStaleHistory.respackages/envio/src/Rollback.respackages/envio/src/Writing.respackages/envio/src/sources/SourceManager.respackages/envio/src/sources/SourceManager.resiscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/Metrics_test.res
💤 Files with no reviewable changes (1)
- packages/envio/src/Prometheus.res
…conds Escape quotes/backslashes/newlines in dynamic Prometheus label values; restore per-storage write samples by threading an onWrite callback through storage.writeBatch so a configured sink is timed separately from postgres; emit a zero envio_effect_cache sample for existing empty persisted caches; include the in-progress status interval in idle/waiting/querying seconds at scrape time; parse escaped label values in the MockIndexer metric helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Merge the metrics snapshot into ChainState.chainData so the status API, TUI and metrics renderer share one toData view. Cumulative metric counters (block-range events/blocks/count, handler counts, storage load count/where/ size, effect call count) become floats since int32 is unsafe for them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31cfe5cb29
ℹ️ 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".
| ->Utils.Dict.forEach((s: EffectState.effectStats) => | ||
| entries->Array.push(( | ||
| `{effect="${s.effectName->escapeLabelValue}",scope="${s.scope->Internal.EffectCache.scopeToString}"}`, | ||
| s, |
There was a problem hiding this comment.
For an effect configured with the same name on multiple chain scopes, this shared entry list now adds scope to envio_effect_cache_invalidations and envio_effect_queue_wait_seconds. Those metrics previously had only the effect label (and their values were aggregated across scopes), so existing PromQL/dashboard selectors such as {effect="foo"} no longer match and aggregate totals become separate series. Render scope-qualified entries only for the metric families that previously used that label.
Useful? React with 👍 / 👎.
| // Prometheus floats keep at most 3 decimals; integral values render without a | ||
| // fractional part. | ||
| @inline | ||
| let formatValue = (value: float) => (Math.round(value *. 1000.) /. 1000.)->Float.toString |
There was a problem hiding this comment.
Prometheus accepts full-precision floating-point samples, but this rounds every metric—not just the previously hand-rendered source-request duration—to milliseconds. Consequently, fast handler/load/effect operations under 0.0005 seconds are emitted as 0, and longer-running counters lose accumulated fractional time on every scrape; this makes short-window rates and latency-derived dashboards materially inaccurate compared with the prior prom-client output. Render the original float value instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The 3-decimal rounding is intentional for this PR — it's an explicit requirement of the change to keep the /metrics payload compact. Sub-millisecond samples rounding to 0 is an accepted trade-off; the affected counters accumulate at full precision in state and only the rendered text is rounded, so no accumulated time is lost between scrapes.
Generated by Claude Code
envio_effect_cache_invalidations and envio_effect_queue_wait_seconds keep their effect-only label set, aggregated across scopes, matching the previous prom-client series identity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/envio/src/EffectState.resi (1)
15-20: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep
invalidationsCountas a float counter.
queueCountis a gauge, sointis fine there.invalidationsCountis the monotonic metric that can still overflow over long runs; widen that field and its increment sites tofloat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/EffectState.resi` around lines 15 - 20, Change EffectState’s invalidationsCount field from int to float, and update every increment or mutation site for invalidationsCount to use float arithmetic. Leave queueCount as int and preserve the existing counter behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/envio/src/EffectState.resi`:
- Around line 15-20: Change EffectState’s invalidationsCount field from int to
float, and update every increment or mutation site for invalidationsCount to use
float arithmetic. Leave queueCount as int and preserve the existing counter
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 60d3289f-5b00-4e75-b668-dcf09428a86b
📒 Files selected for processing (10)
packages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/EffectState.respackages/envio/src/EffectState.resipackages/envio/src/IndexerState.respackages/envio/src/IndexerState.resipackages/envio/src/LoadLayer.respackages/envio/src/Main.respackages/envio/src/Metrics.respackages/envio/src/tui/Tui.res
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/envio/src/IndexerState.resi
- packages/envio/src/LoadLayer.res
- packages/envio/src/EffectState.res
- packages/envio/src/Metrics.res
- packages/envio/src/IndexerState.res
- packages/envio/src/ChainState.res
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd4df38360
ℹ️ 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".
| ->SourceManager.getSourceHeightSamples | ||
| ->Array.forEach((s: SourceManager.sourceHeightSample) => | ||
| entries->Array.push(( | ||
| `{source="${s.sourceName->escapeLabelValue}",chainId="${s.chainId->Int.toString}"}`, |
There was a problem hiding this comment.
When a chain config contains two RPC sources with the same host (for example, primary and fallback URLs that differ only by path or credentials), both are named RPC (<host>). Once both have observed a height, this loop emits two envio_source_known_height samples with identical {source,chainId} labels, which causes Prometheus to reject the scrape as duplicate samples. The previous prom-client gauge coalesced these updates into one series; aggregate or select one sample per label set before rendering.
Useful? React with 👍 / 👎.
Two sources can share a name (e.g. primary and fallback RPC urls on the same host); aggregate request counts/seconds and take the max height per label set so the scrape never contains duplicate samples. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2a568ee25
ℹ️ 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".
| } | ||
|
|
||
| let getHandlerStat = (state: t, ~contract, ~event) => { | ||
| let key = contract ++ ":" ++ event |
There was a problem hiding this comment.
When configured contract/event names contain :, distinct handler pairs such as (A:B, C) and (A, B:C) both map to the same key. The new stateful metric collector then combines their processing/preload counters and renders all of them with the labels from whichever pair was first observed, whereas the prior prom-client handler metrics kept the two label dimensions distinct. Contract and event names come from user configuration, so use a collision-free composite key (or nested dictionaries).
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Metrics.res now defines the metrics snapshot types (chain/handler/effect/ storage/source records plus Metrics.t) and renders prometheus text from the snapshot instead of reaching into indexer state. IndexerState.toMetrics is the single read-only window into the mutable counters: the stat records are hidden behind the IndexerState and EffectState interfaces (effect stat mutations go through startCall/endCall/queue*/recordInvalidation/ commitCacheCount), and the TUI, MockIndexer and the /metrics endpoint all consume Metrics.t. The console/state HTTP response keeps exactly the previous chainData field set for RACE compatibility, mapped from the snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/envio/src/Metrics.res (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove comments that narrate the module and helper implementations.
These comments restate structure or behavior already clear from the types and function names; retain only comments documenting non-obvious constraints.
As per coding guidelines, “Don't write a comment that restates what the code already says” and “Never narrate the refactor itself.”
Also applies to: 157-158, 174-175, 194-194
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/envio/src/Metrics.res` around lines 1 - 5, Remove the introductory module comment in Metrics.res and the comments at the referenced helper locations that merely describe the module structure or implementation behavior. Retain only comments conveying non-obvious constraints, without adding replacement narration.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/envio/src/ChainState.res`:
- Around line 735-737: Remove the caller/purpose narration above
ChainState.toMetrics in packages/envio/src/ChainState.res (lines 735-737). At
packages/envio/src/ChainState.res lines 810-814, replace the stale emission
claim with only the readiness/timestamp invariant. In
packages/envio/src/EffectState.resi lines 36-49, remove the generic “Metric
mutations” and “Metric reads” headings while preserving the rollback and restart
invariants.
---
Nitpick comments:
In `@packages/envio/src/Metrics.res`:
- Around line 1-5: Remove the introductory module comment in Metrics.res and the
comments at the referenced helper locations that merely describe the module
structure or implementation behavior. Retain only comments conveying non-obvious
constraints, without adding replacement narration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 102e26df-f1b4-4118-a90d-e66e0480458e
📒 Files selected for processing (13)
packages/envio/src/ChainState.respackages/envio/src/ChainState.resipackages/envio/src/EffectState.respackages/envio/src/EffectState.resipackages/envio/src/IndexerState.respackages/envio/src/IndexerState.resipackages/envio/src/LoadLayer.respackages/envio/src/Main.respackages/envio/src/Metrics.respackages/envio/src/Writing.respackages/envio/src/tui/Tui.resscenarios/test_codegen/test/helpers/MockIndexer.resscenarios/test_codegen/test/lib_tests/Metrics_test.res
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/envio/src/tui/Tui.res
- packages/envio/src/Writing.res
- scenarios/test_codegen/test/lib_tests/Metrics_test.res
- packages/envio/src/EffectState.res
- scenarios/test_codegen/test/helpers/MockIndexer.res
- packages/envio/src/IndexerState.res
- packages/envio/src/LoadLayer.res
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
…ndexer-state-0ezqlb
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd65c4539d
ℹ️ 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".
| | 1 => "minor" | ||
| | 2 => "major" | ||
| | 4 => "incremental" | ||
| | 8 => "weakcb" | ||
| | _ => "unknown" |
There was a problem hiding this comment.
When /metrics/runtime observes any non-minor GC on the supported Node 22+ runtime, perf_hooks reports kind values as 1=minor, 4=major, 8=incremental, and 16=weakcb. This mapping instead exports major pauses as kind="incremental", incremental pauses as kind="weakcb", and weak-callback pauses as unknown, so GC dashboards or alerts broken down by kind become misleading. Fresh evidence in this version is the new gcKindName renderer that now emits GC metrics but uses the wrong constants.
Useful? React with 👍 / 👎.
Node perf_hooks reports GC kinds as MINOR=1, MAJOR=4, INCREMENTAL=8, WEAKCB=16. The previous mapping mislabeled major pauses as incremental, incremental as weakcb, and dropped weakcb to unknown, skewing GC dashboards broken down by kind. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
…abel - Escape the effect scope label value like every other label so an unusual scope can't break the scrape. - Length-prefix the storage-load stat key so a storage name containing the separator can't collide, matching the handler-stat key. - Compute the active-resource label string once per resource. Adds a collect test asserting both effect and scope labels are escaped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Cover comma and equals in label values (passed through unescaped) alongside the quote/backslash/newline escaping, in both the escapeLabelValue unit test and the whole-output collect render. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
| // Highest height observed for this source, rendered into | ||
| // envio_source_known_height. Kept apart from knownHeight, which drives the | ||
| // height-subscription wait logic. | ||
| mutable reportedHeight: int, |
There was a problem hiding this comment.
Isn't knownHeight is exactly what we want to report?
| ~blockNumber=newHeight.contents, | ||
| ) | ||
| if newHeight.contents > sourceState.reportedHeight { | ||
| sourceState.reportedHeight = newHeight.contents |
There was a problem hiding this comment.
We don't need to report anything. We can simply read a known hate directly from source state.
Drop the separate reportedHeight field and the reportActiveSourceHeight path: write the observed height back into sourceState.knownHeight (which already exists for the height-wait logic) and read it for the envio_source_known_height metric. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8a9672a5d
ℹ️ 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".
| ~name="nodejs_gc_duration_seconds_sum", | ||
| ~help="Cumulative garbage collection pause time by kind, one of major, minor, incremental or weakcb.", | ||
| ~kind="counter", | ||
| ~entries=gcEntries, | ||
| ~value=s => s.seconds, | ||
| ) |
There was a problem hiding this comment.
When users rely on prom-client's default GC histogram (for example histogram_quantile(..., rate(nodejs_gc_duration_seconds_bucket[5m]))), this replacement emits only _sum and _count counter families and never emits nodejs_gc_duration_seconds_bucket samples under the base histogram metric. Fresh evidence in this version is that the current renderer adds GC sum/count here but still omits the bucket series, so existing GC latency quantile dashboards disappear even though GC metrics appear to be present.
Useful? React with 👍 / 👎.
| chainState->ChainState.recordBlockRangeFetch( | ||
| ~totalTimeElapsed=stats.totalTimeElapsed, | ||
| ~parsingTimeElapsed=stats.parsingTimeElapsed->Option.getOr(0.), | ||
| ~numEvents=parsedQueueItems->Array.length, |
There was a problem hiding this comment.
When a block-range response advances knownHeight during a long backfill, we now only record fetch stats here; envio_source_known_height is rendered from SourceManager's per-source knownHeight, which is updated by waitForNewBlock rather than by these query responses. The previous code updated the source-height gauge from this response path, so the per-source height can remain stale until the chain catches up and waits for another block.
Useful? React with 👍 / 👎.
| ~name="nodejs_version_info", | ||
| ~help="Node.js version info.", | ||
| ~kind="gauge", | ||
| ~entries=[(`{version="${NodeJs.Process.version}"}`, ())], |
There was a problem hiding this comment.
When dashboards or alerts filter nodejs_version_info by major, minor, or patch, this replacement only emits the version label. prom-client's default runtime collector exposed all four labels, so existing selectors like {major="22"} stop matching even though the metric name is still present; split NodeJs.Process.version and render the legacy labels too.
Useful? React with 👍 / 👎.
| ~name="process_start_time_seconds", | ||
| ~help="Start time of the process since unix epoch in seconds.", | ||
| ~kind="gauge", | ||
| ~value=Date.now() /. 1000. -. NodeJs.Process.uptime(), |
There was a problem hiding this comment.
When /metrics/runtime is scraped over a wall-clock adjustment or enough timing jitter crosses the 3-decimal rounding boundary, recomputing process_start_time_seconds from Date.now() and process.uptime() on every scrape can make this standard start-time gauge change without a restart. Dashboards and alerts that use changes in process_start_time_seconds to detect restarts can then false-positive; compute this once when the runtime collectors start and reuse it.
Useful? React with 👍 / 👎.
- Compute process_start_time_seconds once when the runtime collectors start and reuse it, so clock jitter across the rounding boundary can't make it read as a phantom restart. - Emit major/minor/patch labels on nodejs_version_info alongside version, matching prom-client's default collector so existing label selectors keep matching. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
During a long backfill the wait loop doesn't run, so the per-source known height (rendered into envio_source_known_height) would stay stale. Advance sourceState.knownHeight from block-range response heights too, keeping the single-field design while restoring backfill freshness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Move the per-source height update from ChainFetching into executeQuery, where the exact source the response came from is in scope. Drops the "assume it's the active source" approximation and removes the reportActiveSourceHeight indirection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
| rateLimits: dict<effectRateLimitState>, | ||
| // Metric counters keyed by the same name. Survive rollback: prometheus | ||
| // counters must stay monotonic. | ||
| stats: dict<effectStats>, |
There was a problem hiding this comment.
How is it different from the stats we already have on effect cache and memory tables?
| res->endWithData( | ||
| Metrics.collect(~metrics=getIndexerState()->Option.map(IndexerState.toMetrics)), |
There was a problem hiding this comment.
Returning None looks like an issue. Let's return an empty string, or a fallback - give me suggestions
| it("Hand-rolls prometheus gauge text with one sample per chain", t => { | ||
| let rendered = Metrics.renderGauge( | ||
| describe("Metrics rendering helpers", () => { | ||
| it("Renders metrics separated by a blank line, keeping 3 decimals after the point", t => { |
There was a problem hiding this comment.
Let's have a test with all metric values present
Assert the complete /metrics output for a Metrics.t with every field and array populated, covering all metric families and the 3-decimal rounding end to end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
2 participants
Summary
Removes the
prom-clientlibrary dependency and replaces it with hand-rolled Prometheus text format rendering. Metrics are now computed from live indexer state at scrape time rather than imperatively updated through a global registry, eliminating the need for external metric management.Key Changes
Prometheus.res: Deleted 789 lines of prom-client wrapper code includingSafeCounter,SafeGauge, and all metric modulesMetrics.res: Replaced pull-based prom-client rendering with hand-rolled Prometheus text format generationblock,sample,series,seriesOpt,single) for composing metricsIndexerStateand its sub-states at scrape timeIndexerState:handlerStat,storageLoadStat,storageWriteStat,historyPruneStattypesChainState: Added per-chain metric counters (blockRangeFetchSeconds,blockRangeParseSeconds,reorgCount, etc.)EffectState: AddedeffectStatstype with call/queue/cache metrics; removedqueueCountfrom rate limit stateSourceManager: AddedreportedHeightand cumulative time counters (idleSeconds,waitingForNewBlockSeconds,queryingSeconds)EventProcessing,ChainFetching,LoadLayer,PgStorage,Rollback,PruneStaleHistory,FetchState, andCrossChainStateMockIndexer; updatedMetrics_test.resto test new builder APIImplementation Details
hyperindex_synced_to_headmetrichttps://claude.ai/code/session_01FfXVEyPmzDojxvrbfv3Ups
Summary by CodeRabbit
/metricsto render from a precomputed indexing-state snapshot, adding richer timing/count metrics for handler execution, storage reads/writes, effect/queue activity, rollbacks, reorg detection, and history pruning.writeBatchcallback usage.