Skip to content

rpc-v2/metrics: Align RPC metrics with tx pool metrics#10105

Open
lexnv wants to merge 5 commits into
masterfrom
lexnv/update-buckets
Open

rpc-v2/metrics: Align RPC metrics with tx pool metrics#10105
lexnv wants to merge 5 commits into
masterfrom
lexnv/update-buckets

Conversation

@lexnv

@lexnv lexnv commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

This PR syncs the tx pool metrics with the RPC layer metrics.

The TX Pool metrics are re-exported under a histogram module, such that the RPC layer can reuse the same settings. This ensures that the RPC will be in sync with the Tx Pool metrics for the reliability dashboards.

Closes: #10067

lexnv added 4 commits October 23, 2025 18:07
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv requested review from a team and michalkucharczyk October 23, 2025 16:00
@lexnv lexnv self-assigned this Oct 23, 2025
@lexnv lexnv added the T0-node This PR/Issue is related to the topic “node”. label Oct 23, 2025
@lexnv

lexnv commented Oct 23, 2025

Copy link
Copy Markdown
Contributor Author

/cmd prdoc --audience node_dev --bump patch


/// Histogram of timings for reporting `Broadcast` event.
pub fn broadcast(name: &'static str, label: &'static str) -> Result<Histogram, PrometheusError> {
Histogram::with_opts(histogram_opts!(name, label, linear_buckets(0.01, 0.25, 16).unwrap()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean the first bucket is (-Inf .. 0.01)? Is it intentional, or it should be linear_buckets(0, 0.25, 16)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would may be even write it as linear_buckets(0.25, 0.25, 16) to not waste a bucket on (-Inf .. 0), because there is no negative timings and bucket (-Inf .. 0.25) is essentially (0 .. 0.25). But this applies to other histograms as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, honestly, I don't know how the histograms are rendered in the UI, and if it makes sense visualization-wise.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not entirely familiar with the reliability buckets 🤔 Maybe @michalkucharczyk or @olliecorbisiero could provide better answers

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmitry-markin @lexnv do the buckets I outlined in issue #10067 help clear things up?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave it as is.

@@ -0,0 +1,97 @@
// This file is part of Substrate.

@michalkucharczyk michalkucharczyk Oct 27, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe metric_buckets.rs (or shared_metric_buckets.rs) would be a better name for this file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or timing_metric_buckets.rs ?

histogram_opts!(name, label).buckets(
[
linear_buckets(0.0, 3.0, 20).unwrap(),
// requested in PR 9158

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// requested in PR 9158
// requested in PR #9158

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

Labels

T0-node This PR/Issue is related to the topic “node”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reliability Dashboard - Align rpc_transaction histogram buckets with substrate_sub_txpool_timing_event buckets for gossip metrics analysis

5 participants