-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat: enable built in Prometheus servlet #695
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
This reverts commit 3f0d9a3.
Release NotesThe built-in Prometheus servlet is now enabled by default and metrics are available under the |
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 found a small typo, otherwise LGTM once the test passes (seems to be hanging because of external problems?).
Nit pick, feel free to ignore since it doesn't seem to cause problems:
The items in the expected_metrics
list are used as regular expressions, it works, but it would be "more correct" if the curly braces were escaped.
Alternative: Maybe assert metric in response.text
would also work, then re
would not be needed at all. It's less strict though since it doesn't require the string to be at the beginning of the line.
Co-authored-by: Lukas Krug <lukas@luvosys.de>
Yeah I saw that and forgot about it but now that you mentioned it ... I geeked out a little. |
I ran the tests locally because it was faster:
|
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.
Thanks, LGTM!
Description
Fixes #694
CI https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/hdfs-operator-it-custom/24/
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecation
label & add to the deprecation scheduletype/experimental
label & add to the experimental features tracker