-
Notifications
You must be signed in to change notification settings - Fork 332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Configurable range for histogram buckets for latency metrics #3382
Conversation
crates/telemetry/src/state.rs
Outdated
} | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
struct CustomAggregatorSelector; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the previous aggregator selector ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
crates/telemetry/src/state.rs
Outdated
} | ||
|
||
pub fn get_submitted_range(&self) -> Vec<f64> { | ||
let step = (self.tx_latency_submitted_max - self.tx_latency_submitted_min) / 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want the number of buckets to be configurable also ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes good idea!
…and confirmed latencies
Closes: #3366
Description
This PR adds 2 new configurations:
submitted_range
used to specify the range of the histogrambuckets for the
tx_latency_submitted
metric.confirmed_range
used to specify the range of the histogrambuckets for the
tx_latency_confirmed
metric.Allowing the buckets to be changed without the need to recompile Hermes
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.