-
Notifications
You must be signed in to change notification settings - Fork 14
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
config: Add Prometheus config #191
Conversation
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.
Need to check the conditional inclusion in the configmap
prometheus: | ||
enabled: {{ .Values.forge.telemetry.prometheus.enabled }} |
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.
This won't get added unless forge.telemtry.sentry
is also set (controlled by the if
block above and the end
under).
Also it looks like the promethus
tag should be one level deeper (to match the depth of sentry
under backend
I can't suggest a change as it would need to include changes to lines not added by this PR
Something like this should work:
backend:
{{ if and (hasKey .Values.forge.telemetry "sentry") (hasKey .Values.forge.telemetry.sentry "backend_dsn") -}}
sentry:
dsn: {{ .Values.forge.telemetry.sentry.backend_dsn}}
{{- end }}
prometheus:
enabled: {{ .Values.forge.telemetry.prometheus.enabled }}
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 also think the denting of prometheus
is one level too deep, it's not a subkey of backend
but top level. Thanks for catching this. Pushed a change, let me know if this is correct @hardillb
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.
But the code in the forge app expects it to be under backend
3ac5e86
to
f78d305
Compare
The application now features a [/metrics endpoint][1] for Prometheus to scrape. Since merging, the configuration file structure has been updated since through [another PR][2]. This change requires both changes to be in the application and afterwards allows enabling the endpoint and an engineer to configure scraping of the data. [1]: FlowFuse/flowfuse#2889 [2]: FlowFuse/flowfuse#2893
f78d305
to
52185d1
Compare
The application now features a /metrics endpoint for Prometheus to scrape. Since merging, the configuration file structure has been updated since through another PR.
This change requires both changes to be in the application and afterwards allows enabling the endpoint and an engineer to configure scraping of the data.