Skip to content

feat(shadow_metrics): Remove shadow metrics emission - #4732

Open
wbj-cb wants to merge 1 commit into
mainfrom
remove-shadow-metrics-emission
Open

feat(shadow_metrics): Remove shadow metrics emission#4732
wbj-cb wants to merge 1 commit into
mainfrom
remove-shadow-metrics-emission

Conversation

@wbj-cb

@wbj-cb wbj-cb commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Dismantles the shadow-metrics background pipeline, leaving the crate as the read-only block-explorer API over persisted shadow blocks. Removes all Prometheus metric emission and the polling reader together with its cursor infrastructure (the reader existed only to poll shadow_blocks via a cursor and emit metrics).

Changes

  • shadow-metrics: Delete ShadowMetrics and all metric recording; delete ShadowMetricsReader. /readyz now only checks that shadow_blocks is readable. Drop the base-metrics, metrics, and tokio deps and the metrics feature.
  • shadow-indexer-db: Drop ShadowBlockCursor, ShadowMetricsCursorRepo, ShadowBlockRepo::{list_reorged_since, max_cursor}, and ShadowBlockRow::cursor().
  • bin: Drop reader spawning, liveness tracking, and poll-interval/max-rows args. The Prometheus metrics initializer is left in place.
  • System tests: Keep the DB reconciliation tests (renamed to shadow_blocks_reconciliation.rs), remove the reader/cursor tests, drop the unused base-shadow-metrics dev-dependency.

Notes

  • The shadow_metrics_cursor table and its migrations are intentionally left in place; no code references them.

Testing

  • cargo clippy clean across base-shadow-indexer-db, base-shadow-metrics, base-shadow-metrics-bin
  • Unit tests pass; shadow_blocks_reconciliation system test compiles

@cb-heimdall

Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@depot-code-access

depot-code-access Bot commented Aug 27, 2026

Copy link
Copy Markdown

✅ All benchmarks green — 14 within ±2% (deterministic instruction counts). View run

Benchmark details (14)
Benchmark Base (target) Head (this PR) Δ instructions
batch_queue/drain/drain_cached_span_batches 242,027 242,027 +0.0%
batch_transaction/encode_in_place/encode_in_place 4,199,759 4,199,759 +0.0%
batch_transaction/temporary_frame_buffers/temporary_frame_buffers 8,408,350 8,408,350 +0.0%
flashblock_decode/decode/brotli 3,296,484 3,296,484 +0.0%
flashblock_decode/decode/plain_json 2,280,194 2,280,194 +0.0%
flz/compress_len/real_contract_call 43,148 43,148 +0.0%
flz/compress_len/synthetic_0 38,205 38,205 +0.0%
flz/compress_len/synthetic_1 54,682 54,682 +0.0%
flz/compress_len/synthetic_2 147,976 147,976 +0.0%
flz/data_gas 43,059 43,059 +0.0%
flz/tx_estimated_size 43,056 43,056 +0.0%
frame_parse/decode/single_4kib 1,031 1,031 +0.0%
frame_parse/parse_frames/few_large 1,053,062 1,053,062 +0.0%
frame_parse/parse_frames/many_small 154,763 154,763 +0.0%

@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

Clean removal of the shadow-metrics polling reader and Prometheus emission pipeline. The deletion is thorough: types (ShadowBlockCursor, ShadowMetricsCursorRepo, ShadowMetricsReader, ShadowMetrics), their re-exports, Cargo dependencies, and the reader system tests are all removed consistently. No dangling references to the deleted types remain in code. The retained reconciliation tests were correctly extracted into a standalone file. The check_schema_ready simplification (dropping cursor-table privilege checks) is appropriate given the cursor infra is gone.

Findings (nits — all in unchanged lines, so inline comments could not be posted):

  1. Stale "reader" references in doc comments — Several doc comments in files touched by this PR still reference "the reader" which no longer exists:

    • crates/execution/shadow-indexer-db/src/models.rs:21"stall the reader"
    • crates/execution/shadow-indexer-db/src/repo.rs:74"keeps a reader from observing"
    • crates/execution/shadow-indexer-db/src/repo.rs:214"A row is never emitted until…" (emission was the reader's job)
    • crates/execution/shadow-indexer-db/src/repo.rs:220"not the reader's"

    These are not blocking but would be good to clean up in a follow-up to avoid confusing future readers of the code.

No correctness, safety, or concurrency issues found. Not block-production-sensitive (shadow-metrics is a read-only observability service).

@depot-code-access

depot-code-access Bot commented Aug 27, 2026

Copy link
Copy Markdown

Base Std historical fork tests

Fork Result Passed Failed Skipped base/base base-anvil base-std
Beryl pass 616 0 13 1e8bec9e cecddfa5 4658f1b7
Cobalt pass 721 0 14 1e8bec9e fb00db40 e30b3421

View run

… cursor

Dismantle the shadow-metrics background pipeline, leaving the crate as
the read-only block-explorer API over persisted shadow blocks.

Metric emission: delete the ShadowMetrics definitions and all recording
(gas/tx/inversion histograms, block counters, backlog gauges, poll-error
counters); drop the base-metrics dependency, the optional metrics
dependency, and the metrics feature.

Reader + cursor: the reader existed only to poll shadow_blocks via a
persisted cursor and emit metrics, so both are now removed.
- shadow-indexer-db: drop ShadowBlockCursor, ShadowMetricsCursorRepo,
  ShadowBlockRepo::list_reorged_since/max_cursor, ShadowBlockRow::cursor.
- shadow-metrics: delete ShadowMetricsReader; readiness now only checks
  that shadow_blocks is readable (drop the shadow_metrics_cursor probe);
  remove the now-unused tokio dependency.
- bin: drop reader spawning, liveness tracking, and poll-interval/max-rows
  args; /readyz now checks only schema readiness. The Prometheus metrics
  initializer is left in place.
- system tests: keep the DB reconciliation tests (renamed to
  shadow_blocks_reconciliation.rs), remove the reader/cursor tests, and
  drop the now-unused base-shadow-metrics dev-dependency.

The shadow_metrics_cursor table and its migrations are left in place; no
code references them.

Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
@wbj-cb
wbj-cb force-pushed the remove-shadow-metrics-emission branch from f63ed1c to a0f1cb5 Compare August 27, 2026 18:43
@github-actions

Copy link
Copy Markdown
Contributor

Review Summary

Clean removal PR. The deleted code (ShadowMetricsReader, ShadowMetrics, ShadowBlockCursor, ShadowMetricsCursorRepo) is fully unreferenced after the change — grep confirms no remaining call sites. Migration 0008_drop_metrics_cursor.sql correctly uses DROP TABLE IF EXISTS for safe idempotent application.

Minor finding (not in diff, so no inline comment possible)

Stale doc comment in crates/execution/shadow-indexer-db/src/models.rs:21: The ShadowBlockRow.payload field doc still says "Incompatible serde changes fail the whole fetch and stall the reader" — the reader no longer exists after this PR. Consider updating to something like "Incompatible serde changes fail the whole fetch; this risk is accepted."

PR description inconsistency

The PR description says "The shadow_metrics_cursor table and its migrations are intentionally left in place; no code references them." but the PR actually adds migration 0008_drop_metrics_cursor.sql to drop the table. The description should be updated to match the implementation.

No correctness, safety, concurrency, or performance issues found. This PR does not touch block-production-sensitive paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants