-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add bundle and operation counters #4991
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
Conversation
linera-core/src/worker.rs
Outdated
| }); | ||
|
|
||
| pub static OPERATION_COUNT: LazyLock<IntCounterVec> = | ||
| LazyLock::new(|| register_int_counter_vec("operation_count", "Operation count", &[])); |
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.
It doesn't need to be a _vec if we don't use labels.
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.
True, but I guess my thinking was: if we one day decide to add labels to this, will Prometheus keep the non-vec historical data logged to the same metric? That's why I've been using _vec everywhere 🤔
It seems actually that adding/removing labels creates an entirely new time series https://prometheus.io/docs/concepts/data_model/?metric-names-and-labels so we might as well drop the _vec
afck
left a comment
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.
No blockers from my side, but @deuszx's comment should be addressed.
4bbb2ed to
59364ff
Compare

Motivation
We should track operations and messages as well, not just transactions generically
Proposal
Add metrics for operations and messages
Test Plan
Deployed a network with this, saw the metrics
Release Plan