-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[bitnami/keycloak] Issue with metrics into ServiceMonitor #30370
Conversation
…and master realm metrics. Signed-off-by: Renaud Demarneffe <[email protected]>
Signed-off-by: Renaud Demarneffe <[email protected]>
@@ -1011,7 +1011,8 @@ metrics: | |||
## @param metrics.service.ports.http Metrics service HTTP port | |||
## | |||
ports: | |||
http: 9000 | |||
metrics: 9000 |
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 for your contribution. Please note you need to add the metadata for this new parameter, see the above metadata corresponding to the already existing parameters.
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 updated the metadata.
Signed-off-by: Renaud Demarneffe <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
Signed-off-by: Bitnami Containers <[email protected]>
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 a lot @RDemarneffe, in #30095 didn't see that instance and realm metrics were using different interfaces. Great work!
I was revisiting monitoring features and WDYT about this:
charts/bitnami/keycloak/values.yaml
Lines 1015 to 1019 in 089c308
## @param metrics.service.annotations [object] Annotations for enabling prometheus to access the metrics endpoints | |
## | |
annotations: | |
prometheus.io/scrape: "true" | |
prometheus.io/port: "{{ .Values.metrics.service.ports.http }}" |
I think is worth to change it with something like this:
## @param metrics.service.annotations [object] Annotations for enabling prometheus to access the metrics endpoints
##
annotations:
prometheus.io/scrape: "true"
prometheus.io/port: "{{ .Values.metrics.service.ports.metrics }}"
prometheus.io/path: "{{ (.Values.metrics.serviceMonitor.endpoints | first).path }}"
@RDemarneffe This has been fixed by #30303, merged yesterday |
Thanks @RDemarneffe. I'll close this PR. |
Description of the change
Allow using different ports into ServiceMonitor for instance metrics and master realm metrics.
Benefits
Correctly configure prometheus for grabbing metrics.
Possible drawbacks
None.
Applicable issues
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm