Skip to content

Conversation

@fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Oct 23, 2025

This PR removes the use of cosmossdk.io/telemetry (which depends on go-metrics) and replaces it with direct usage of the Prometheus client library (prometheus/client_golang). This aligns beacon-kit with CometBFT’s metrics approach and brings several key improvements:

  • Eliminates race conditions introduced by go-metrics.
  • Avoids reliance on a questionably maintained upstream repo.
  • Fixes problematic metric retention behavior.
  • Simplifies and unifies metrics code across all services.

All components now expose metrics via a common metrics.Factory interface, which standardizes registration, labeling, and naming conventions.

Additionally:

  • Replaces cosmossdk.io/store/metrics imports with a lightweight storage.NoOpStoreMetrics implementation to remove telemetry dependencies.
  • While go-metrics remains a transitive dependency through cosmossdk.io/store/types, it is no longer used at runtime.

NOTE: Since this is a large PR I tried to make it backwards compatible as much as possible, even if it went against prometheus conventions. Future PRs could address these items separately:

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 82.05374% with 187 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.15%. Comparing base (cc4ec8a) to head (5fe4aab).

Files with missing lines Patch % Lines
observability/metrics/prometheus/prometheus.go 36.06% 78 Missing ⚠️
execution/client/metrics.go 82.65% 30 Missing ⚠️
node-core/components/metrics_factory.go 17.14% 29 Missing ⚠️
execution/engine/metrics.go 84.13% 23 Missing ⚠️
config/config/config.go 0.00% 5 Missing ⚠️
state-transition/core/metrics.go 94.52% 4 Missing ⚠️
beacon/blockchain/blob_fetcher_metrics.go 95.83% 2 Missing ⚠️
beacon/blockchain/deposit.go 0.00% 2 Missing ⚠️
beacon/blockchain/metrics.go 97.22% 2 Missing ⚠️
cmd/beacond/defaults.go 0.00% 2 Missing ⚠️
... and 6 more
Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##           blobreactor    #2966      +/-   ##
===============================================
+ Coverage        63.24%   64.15%   +0.91%     
===============================================
  Files              367      371       +4     
  Lines            18014    18597     +583     
===============================================
+ Hits             11393    11931     +538     
- Misses            5713     5759      +46     
+ Partials           908      907       -1     
Files with missing lines Coverage Δ
beacon/blockchain/blob_fetcher.go 73.39% <ø> (-0.48%) ⬇️
beacon/blockchain/blob_queue.go 58.87% <100.00%> (ø)
beacon/blockchain/service.go 86.84% <100.00%> (ø)
beacon/validator/metrics.go 100.00% <100.00%> (ø)
beacon/validator/service.go 100.00% <100.00%> (ø)
consensus/cometbft/service/prepare_proposal.go 83.33% <100.00%> (-0.35%) ⬇️
consensus/cometbft/service/process_proposal.go 49.29% <100.00%> (-0.71%) ⬇️
consensus/cometbft/service/service.go 39.89% <100.00%> (ø)
consensus/cometbft/service/state/state.go 51.51% <100.00%> (ø)
da/blob/factory.go 64.28% <100.00%> (ø)
... and 44 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fridrik01 fridrik01 force-pushed the blobreactor-prometheus2 branch from 0a936e8 to 784b633 Compare October 24, 2025 16:00
@fridrik01 fridrik01 force-pushed the blobreactor-prometheus2 branch from 784b633 to 78c4ed4 Compare October 24, 2025 16:09
@fridrik01 fridrik01 changed the title Use prometheus client directly for metrics + remove go-metrics depend… chore(metrics): Replace cosmossdk.io/telemetry with prometheus/client_golang Oct 27, 2025
@fridrik01 fridrik01 marked this pull request as ready for review October 27, 2025 15:46
@fridrik01 fridrik01 requested a review from a team as a code owner October 27, 2025 15:46
@fridrik01 fridrik01 force-pushed the blobreactor-prometheus2 branch from 4c886ad to 9a5c6eb Compare October 28, 2025 11:42
@fridrik01 fridrik01 force-pushed the blobreactor-prometheus2 branch from 9a5c6eb to 8666900 Compare October 28, 2025 12:00
@fridrik01 fridrik01 force-pushed the blobreactor branch 2 times, most recently from b62bf9f to 0ef6906 Compare October 31, 2025 15:14
…ency

Remove use of cosmossdk.io/telemetry package which relies on go-metrics and
replace it with direct use of prometheus/client_golang. This aligns with
how CometBFT handles metrics and brings several benefits: eliminates race
conditions from go-metrics, avoids questionable maintenance of upstream
repo, removes problematic metric retention, and significantly simplifies
the metrics codebase. All beacon-kit services now use prometheus client
directly through a unified metrics.Factory interface.

Additionally, replace cosmossdk.io/store/metrics imports with a custom
storage.NoOpStoreMetrics implementation to avoid pulling in the telemetry
wrapper. While go-metrics remains as a transitive dependency
through cosmossdk.io/store/types, it is no longer used at runtime.
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