jsonnet: Support scraping the config-reloader for AlertManager and Pr… #1344
Conversation
philipgough
force-pushed
the
MON-1085
branch
2 times, most recently
from
e03bba3
to
039d2bf
Compare
|
@ArthurSens thanks for the review and testing it. I was happy to leave it off as default actually but just wanted to ensure CI passed with it enabled. As you said, we can wait to see what others think and am happy to leave it or change it either way. |
There was a problem hiding this comment.
Just a few comments from my side but it looks great overall, thanks @philipgough.
| @@ -19,6 +19,8 @@ local utils = import './lib/utils.libsonnet'; | |||
| common: { | |||
| namespace: 'default', | |||
| platform: null, | |||
| scrapeConfigReloader: true, | |||
There was a problem hiding this comment.
I don't think this should be conditional since we thrive to monitor all the applications that we run.
There was a problem hiding this comment.
@dgrisonnet - understood, my thinking was that there might be situations where the downstream might be injecting a proxy for example or setting the reloader to listen on localhost where it wouldn't be scrape-able via the service monitor. Any concerns in that respect?
| @@ -19,6 +19,8 @@ local utils = import './lib/utils.libsonnet'; | |||
| common: { | |||
| namespace: 'default', | |||
| platform: null, | |||
| scrapeConfigReloader: true, | |||
| reloaderPort: 8080, | |||
There was a problem hiding this comment.
This value is specific to each component so I wouldn't define its value globally but rather per component.
philipgough
force-pushed
the
MON-1085
branch
from
039d2bf
to
6c04490
Compare
|
@dgrisonnet removed the option for conditional config and added component specific config for the port as requested. |
| @@ -136,6 +137,7 @@ function(params) { | |||
| spec: { | |||
| ports: [ | |||
| { name: 'web', targetPort: 'web', port: 9093 }, | |||
| { name: 'reloader-web', port: am._config.reloaderPort, targetPort: am._config.reloaderPort }, | |||
There was a problem hiding this comment.
targetPort should be reloader-web
| @@ -98,6 +99,7 @@ function(params) { | |||
| spec: { | |||
| ports: [ | |||
| { name: 'web', targetPort: 'web', port: 9090 }, | |||
| { name: 'reloader-web', port: p._config.reloaderPort, targetPort: p._config.reloaderPort }, | |||
There was a problem hiding this comment.
targetPort should be reloader-web
philipgough
force-pushed
the
MON-1085
branch
from
6c04490
to
7b32afb
Compare
…ometheus
Description
This change updates the ServiceMonitor to scrape the reloader sidecar for Promtheus and AlertManager via the service when the components are not set to listen on localhost.
Type of change
What type of changes does your code introduce to the kube-prometheus? Put an
xin the box that apply.CHANGE(fix or feature that would cause existing functionality to not work as expected)FEATURE(non-breaking change which adds functionality)BUGFIX(non-breaking change which fixes an issue)ENHANCEMENT(non-breaking change which improves existing functionality)NONE(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Changelog entry