-
-
Notifications
You must be signed in to change notification settings - Fork 241
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
have a $service_name parameter for all prometheus-exporters #430
have a $service_name parameter for all prometheus-exporters #430
Conversation
40e47f2
to
d0b562c
Compare
manifests/consul_exporter.pp
Outdated
@@ -63,6 +65,7 @@ | |||
String $log_level, | |||
String $package_ensure, | |||
String $package_name, | |||
String $service_name, |
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.
for all $service_name
parameters please use String[1]
to prohibit empty empty strings
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.
Sure, will adapt the PR. Shall I change it for $package_name
too?
As far as I remember I always have just copied the package-name-line as base and adapted it for the service. So there might be a mixture of String
and String[1]
there too.
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.
yes please :)
c3becd8
to
303a043
Compare
@unki please have a look at the failing travis jobs |
303a043
to
14849dd
Compare
ready to go now |
thanks for the work! |
…-to-all-exporters have a $service_name parameter for all prometheus-exporters
looks like, not all exporters have this $service_name as daemon name |
Currently there is a mixture of exporters having a
$service_name
parameter or not.I've tried to make them all to look the same know.