-
Notifications
You must be signed in to change notification settings - Fork 60
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
chore: more efficient metrics usage #3298
base: master
Are you sure you want to change the base?
Conversation
* Bound the metrics-label-values in arbitrary queries. * The metrics-label-values for prepared statements are kept as they already represent a fixed set.
You can find the image built from this PR at
Built from 3d125ea |
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.
I would appreciate some reviews from @waku-org/waku team as this is generating alerting noise for our metrics cluster.
Any news on this? The metrics are still being dropped. |
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.
Just one comment.
return AnalyzeMessagesQuery | ||
elif query.contains("SELECT EXISTS"): | ||
return CheckVersionTableExistsQuery | ||
|
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.
add else exception here... for stmnt not matching.
const QueriesToMetricMap* = { | ||
"contentTopic IN": ContentTopicQuery, | ||
"SELECT version()": SelectVersionQuery, | ||
"WITH min_timestamp": MessagesLookupQuery, |
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.
That looks much better, but what's the point of these const
s if they are not used anywhere else? Seem pointless.
Description
they already represent a fixed set.
Issue
closes #3282