-
Notifications
You must be signed in to change notification settings - Fork 590
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
fix(metrics): avoid calling with_label_values in every report for label guarded metrics #13997
Conversation
… yiming/add_target_source_to_actor_input_output_rows
…to yiming/correct-with-label-values
…to yiming/correct-with-label-values
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.
LGTM
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.
LGTM. Shall we cherry pick this pr to the 1.5.1-rc?
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #13997 +/- ##
==========================================
- Coverage 68.04% 68.04% -0.01%
==========================================
Files 1540 1540
Lines 265465 265529 +64
==========================================
+ Hits 180637 180679 +42
- Misses 84828 84850 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…el guarded metrics (#13997) Co-authored-by: Eric Fu <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Part of #13981.
For label guarded metrics, if we call
with_label_values
and drop the returned value label guarded metric immediately after we use it, the label will be seen as unused anymore. After the metrics are collected, the label will be removed, and later report on the same label will reinitialize the metrics, which may cause incorrect metrics (#13948).In this PR, we remove all incorrect usages of
with_label_values
on the label guarded metrics, which uses the metrics in the form ofmetrics.with_label_values(...).inc()
. Instead, we will store the value returned fromwith_label_values
as field of some struct with longer lifetime so that the label will be regarded as being used and will not be repeatedly dropped and reinitialized.Besides, the
with_label_values
of label guarded metrics is renamed towith_guarded_label_values
so that all previous usages ofwith_label_values
of label guarded metrics can be reviewed in this PR. And since we change to a new method name, when we are working on #12805, after we replace some usage of the raw metrics vec to the label guarded metrics vec, the code cannot be compiled directly. Any previous usage onwith_label_values
of raw metrics vec can be reviewed and change to be used in the correct way.Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.