-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ui: Plot circuit breaker tripped events as a rate values #81438
ui: Plot circuit breaker tripped events as a rate values #81438
Conversation
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.
Looks good mod the comment Help in metrics.go, thanks!
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @koorosh)
pkg/kv/kvserver/metrics.go
line 1407 at r1 (raw file):
metaReplicaCircuitBreakerCumTripped = metric.Metadata{ Name: "kv.replica_circuit_breaker.num_tripped_events", Help: `The number of circuit breaker events occurred per second across all nodes since the process started.`,
This isn't "across all nodes", each metric counter is local to a store and thus will be reported at the store level. It just so happens that the chart we later expose in the DB console aggregates over all stores. So the comment in graphTooltips.tsx
is correct. Also, this here is still a counter, not a rate. We only compute a rate in the console.
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.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @tbg)
pkg/kv/kvserver/metrics.go
line 1407 at r1 (raw file):
Previously, tbg (Tobias Grieger) wrote…
This isn't "across all nodes", each metric counter is local to a store and thus will be reported at the store level. It just so happens that the chart we later expose in the DB console aggregates over all stores. So the comment in
graphTooltips.tsx
is correct. Also, this here is still a counter, not a rate. We only compute a rate in the console.
gotcha, then I tend to revert back this Help message because it also doesn't count "per second".
361bfd0
to
7948daf
Compare
7948daf
to
54358de
Compare
Before, Circuit Breaker Tripped events chart displayed accumulated number of events since process started. Now, it displays number of events per aggregated interval of time. Release note (ui change): Circuit Breaker Tripped events chart displays rate of events per interval instead of accumulated number of events.
54358de
to
ea6289b
Compare
bors r+ |
Build failed (retrying...): |
bors r- |
Canceled. |
bors r=tbg Sorry, needed to cancel this run to move |
Build succeeded: |
Resolves #78939
Before, Circuit Breaker Tripped events chart displayed the accumulated number
of events since the process started.
Now, it displays the number of events per aggregated interval of time.
Release note (ui change): Circuit Breaker Tripped events chart displays
rate of events per interval instead of the accumulated number of events.