-
Notifications
You must be signed in to change notification settings - Fork 771
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 #905: use SUM_LOCK_TIME and SUM_CPU_TIME with mysql >= 8.0.28 #916
Fix #905: use SUM_LOCK_TIME and SUM_CPU_TIME with mysql >= 8.0.28 #916
Conversation
a6f8246
to
12ad6a1
Compare
…8.0.28 Fix issue introduced in prometheus#862 Only query SUM_LOCK_TIME and SUM_CPU_TIME when using MySQL >= 8.0.28. Fixes prometheus#905 Signed-off-by: Cristian Greco <cristian@regolo.cc>
12ad6a1
to
22c7a26
Compare
I think we typically version the whole query rather than try and do string manipulation. |
eb1e4d3
to
f2955ea
Compare
Signed-off-by: Cristian Greco <cristian@regolo.cc>
f2955ea
to
28bb162
Compare
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
if pb.Summary != nil { | ||
return MetricResult{labels: labels, value: pb.GetSummary().GetSampleSum(), metricType: dto.MetricType_SUMMARY} | ||
} |
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.
Ideally would add quantiles as well, but that seems too much refactoring for this PR.
Followup of #916, where I wrongly deleted query params formatting.
Followup of #916, where I wrongly deleted query params formatting. Signed-off-by: Cristian Greco <cristian@regolo.cc>
Fix issue introduced in #862
Only query SUM_LOCK_TIME and SUM_CPU_TIME when using MySQL >= 8.0.28.
Fixes #905