Skip to content

refactor: move metrics into their owning crates - #1795

Draft
bmuddha wants to merge 2 commits into
stack/chainsync/cutoverfrom
stack/chainsync/metrics
Draft

bmuddha wants to merge 2 commits into
stack/chainsync/cutoverfrom
stack/chainsync/metrics

Conversation

@bmuddha

@bmuddha bmuddha commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Move surviving collectors into Aperture, Committor, RPC client, table management, and the validator adapter. Put the scrape endpoint in Aperture for leaders and verifiers, reuse nucleus primitives and the default registry, and remove the legacy metrics crate.

Closes #1787

Impact

Remove obsolete Chainlink collectors, collection-frequency configuration, and transaction-history polling used only for compute-usage metrics. Surviving application collectors have explicit owners and share the scrape registry with Engine.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: magicblock-labs/magicblock-validator/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2b4dae3a-0f74-477a-a45c-b7acbc6cd472
📥 Commits

Reviewing files that changed from the base of the PR and between 91bcf54 and 861fc9f.

⛔ Files ignored due to path filters: 2
  • Cargo.lock is excluded by !**/*.lock
  • test-integration/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 51
  • .github/actions/setup-yellowstone/action.yml
  • Cargo.toml
  • aperture/Cargo.toml
  • aperture/src/account.rs
  • aperture/src/lib.rs
  • aperture/src/metrics.rs
  • aperture/src/requests/http/dispatch.rs
  • aperture/src/requests/http/send_transaction.rs
  • aperture/src/server/metrics.rs
  • aperture/src/server/mod.rs
  • aperture/src/server/websocket/dispatch.rs
  • bins/validator/Cargo.toml
  • bins/validator/src/leader.rs
  • bins/validator/src/magic_sys_adapter.rs
  • bins/validator/src/main.rs
  • bins/validator/src/metrics.rs
  • bins/verifier/Cargo.toml
  • bins/verifier/src/main.rs
  • committor/Cargo.toml
  • committor/src/intent_engine/db.rs
  • committor/src/intent_engine/intent_execution_engine.rs
  • committor/src/intent_engine/intent_scheduler.rs
  • committor/src/intent_executor/accepted_intent_executor.rs
  • committor/src/intent_executor/error.rs
  • committor/src/intent_executor/intent_execution_client.rs
  • committor/src/intent_executor/mod.rs
  • committor/src/intent_executor/single_stage_intent_executor.rs
  • committor/src/intent_executor/strategy_executor/error.rs
  • committor/src/lib.rs
  • committor/src/metrics.rs
  • committor/src/service.rs
  • committor/src/tasks/mod.rs
  • committor/src/tasks/task_info_fetcher.rs
  • committor/src/transaction_preparator/delivery_preparator.rs
  • committor/src/transaction_preparator/mod.rs
  • config/src/config/metrics.rs
  • config/src/consts.rs
  • metrics/Cargo.toml
  • metrics/README.md
  • metrics/src/lib.rs
  • metrics/src/metrics/mod.rs
  • metrics/src/metrics/types.rs
  • rpc-client/Cargo.toml
  • rpc-client/src/lib.rs
  • rpc-client/src/metrics.rs
  • rpc-client/src/signature_confirmer.rs
  • table-mania/Cargo.toml
  • table-mania/src/lib.rs
  • table-mania/src/lookup_table_rc.rs
  • table-mania/src/manager.rs
  • table-mania/src/metrics.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: magicblock-labs/magicblock-validator/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 310c5318-48cf-496d-a096-5fd42b688116
📥 Commits

Reviewing files that changed from the base of the PR and between 9258666 and ea05c3c.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • test-integration/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (50)
  • Cargo.toml
  • aperture/Cargo.toml
  • aperture/src/account.rs
  • aperture/src/lib.rs
  • aperture/src/metrics.rs
  • aperture/src/requests/http/dispatch.rs
  • aperture/src/requests/http/send_transaction.rs
  • aperture/src/server/metrics.rs
  • aperture/src/server/mod.rs
  • aperture/src/server/websocket/dispatch.rs
  • bins/validator/Cargo.toml
  • bins/validator/src/leader.rs
  • bins/validator/src/magic_sys_adapter.rs
  • bins/validator/src/main.rs
  • bins/validator/src/metrics.rs
  • bins/verifier/Cargo.toml
  • bins/verifier/src/main.rs
  • committor-service/Cargo.toml
  • committor-service/src/intent_engine/db.rs
  • committor-service/src/intent_engine/intent_execution_engine.rs
  • committor-service/src/intent_engine/intent_scheduler.rs
  • committor-service/src/intent_executor/accepted_intent_executor.rs
  • committor-service/src/intent_executor/error.rs
  • committor-service/src/intent_executor/intent_execution_client.rs
  • committor-service/src/intent_executor/mod.rs
  • committor-service/src/intent_executor/single_stage_intent_executor.rs
  • committor-service/src/intent_executor/strategy_executor/error.rs
  • committor-service/src/lib.rs
  • committor-service/src/metrics.rs
  • committor-service/src/service.rs
  • committor-service/src/tasks/mod.rs
  • committor-service/src/tasks/task_info_fetcher.rs
  • committor-service/src/transaction_preparator/delivery_preparator.rs
  • committor-service/src/transaction_preparator/mod.rs
  • config/src/config/metrics.rs
  • config/src/consts.rs
  • metrics/Cargo.toml
  • metrics/README.md
  • metrics/src/lib.rs
  • metrics/src/metrics/mod.rs
  • metrics/src/metrics/types.rs
  • rpc-client/Cargo.toml
  • rpc-client/src/lib.rs
  • rpc-client/src/metrics.rs
  • rpc-client/src/signature_confirmer.rs
  • table-mania/Cargo.toml
  • table-mania/src/lib.rs
  • table-mania/src/lookup_table_rc.rs
  • table-mania/src/manager.rs
  • table-mania/src/metrics.rs
💤 Files with no reviewable changes (11)
  • metrics/Cargo.toml
  • committor-service/src/intent_executor/single_stage_intent_executor.rs
  • metrics/README.md
  • config/src/consts.rs
  • Cargo.toml
  • committor-service/src/intent_executor/accepted_intent_executor.rs
  • metrics/src/lib.rs
  • aperture/src/account.rs
  • config/src/config/metrics.rs
  • metrics/src/metrics/types.rs
  • metrics/src/metrics/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Metrics move from the shared metrics crate into the crates that own the measured behavior. Aperture adds the scrape service and RPC metrics; Committor, the validator, the RPC client, and table-mania add local metrics. The shared crate and collection-frequency configuration are removed.

Changes: Metrics ownership migration

Layer / File(s) Summary
Aperture metrics and scrape service
aperture/Cargo.toml, aperture/src/account.rs, aperture/src/lib.rs, aperture/src/metrics.rs, aperture/src/requests/http/*, aperture/src/server/*, bins/validator/src/leader.rs, bins/verifier/Cargo.toml, bins/verifier/src/main.rs
Aperture adds RPC request and skip-preflight metrics and a Prometheus scrape service. HTTP and WebSocket request handlers record metrics through local helpers. The validator leader and verifier use Aperture's MetricsService. Account synchronization timers and the transaction-processing timer are removed.
Validator nonce-wait metrics
bins/validator/Cargo.toml, bins/validator/src/main.rs, bins/validator/src/magic_sys_adapter.rs, bins/validator/src/metrics.rs
The validator adds a histogram and timer helper for synchronous commit-nonce waits. The nonce-fetch path uses the helper, and the validator enables the nucleus metrics feature.
Committor metrics and event recording
committor-service/Cargo.toml, committor-service/src/metrics.rs, committor-service/src/intent_engine/*, committor-service/src/intent_executor/*, committor-service/src/service.rs, committor-service/src/tasks/*, committor-service/src/transaction_preparator/*
Committor adds local metrics for intent scheduling, backlog, inflight work, poisoned keys, execution results, preparation, lookups, account fetches, retiring entries, and history queries. Call sites record these events. Metric-label mappings become inherent methods, and transaction-history polling for compute usage is removed.
RPC signature-confirmation metrics
rpc-client/Cargo.toml, rpc-client/src/lib.rs, rpc-client/src/metrics.rs, rpc-client/src/signature_confirmer.rs
The RPC client adds counters for WebSocket subscriptions and notifications, HTTP fallbacks, and status batches. Signature confirmation records those events through its local metrics module.
Table-mania metrics
table-mania/Cargo.toml, table-mania/src/lib.rs, table-mania/src/lookup_table_rc.rs, table-mania/src/manager.rs, table-mania/src/metrics.rs
Table-mania adds counters for lookup-table readiness fetch attempts and closure checks. The readiness loop and closure check record events through its local metrics module.
Shared metrics crate and frequency removal
Cargo.toml, config/src/config/metrics.rs, config/src/consts.rs, metrics/*
The workspace removes the shared metrics crate and its path dependency. The crate's documentation, collectors, metric types, and tests are deleted. Metrics configuration no longer includes a collection-frequency field or its default constant.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to ea05c

No concrete issue was found that should block this metrics migration. Normal build and scrape checks remain appropriate before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 20.99% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 33 files. (6 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed The changes implement the coding requirements in #1787. The PR removes the legacy metrics crate and its workspace dependency. Aperture owns RPC metrics and MetricsService, and the leader and verif…
Out of Scope Changes check Passed The changed files remain within #1787. Dependency updates, owner-local metric modules, call-site migrations, endpoint wiring, configuration removal, obsolete collector removal, and transaction-history…
Title check Passed The title clearly and concisely summarizes the primary change: moving metrics into the crates that own them.
Description check Passed The description directly matches the changeset. It covers moving surviving collectors, relocating the scrape endpoint, removing the legacy metrics crate, and deleting obsolete metrics configuration an…
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.99% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 33 files. (6 skipped: 6 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@bmuddha
bmuddha added this pull request to stack #1797 October 12, 2026 04:24
@bmuddha bmuddha added techdebt Issues outlining shortcuts we took that need to be fixed when we have time rust Pull requests that update rust code labels Oct 12, 2026
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch 2 times, most recently from 64dabfa to 3c94a72 Compare October 13, 2026 06:46
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch 2 times, most recently from 3b467ed to ea05c3c Compare October 13, 2026 13:33
@bmuddha

bmuddha commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch from ea05c3c to 22170f0 Compare October 13, 2026 14:04
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch from 22170f0 to 5097d56 Compare October 13, 2026 14:36
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch from 5097d56 to b60064b Compare October 13, 2026 15:55
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch 4 times, most recently from 4061e60 to 91bcf54 Compare October 16, 2026 06:26
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch from 91bcf54 to 5c3ad8b Compare October 16, 2026 06:59
@bmuddha
bmuddha force-pushed the stack/chainsync/metrics branch from 5c3ad8b to 861fc9f Compare October 16, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update rust code techdebt Issues outlining shortcuts we took that need to be fixed when we have time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move metrics into their owning crates

2 participants