voxpupuli / puppet-prometheus

Puppet module for prometheus
https://forge.puppet.com/puppet/prometheus
Apache License 2.0
60 stars 239 forks source link

Puppet strings and interface cleanup #385

Open alexjfisher opened 4 years ago

alexjfisher commented 4 years ago

At the moment, I think it's very confusing that there are two interfaces to prometheus::server. You can declare that class directly, or declare the base class with manage_prometheus_server => true.

prometheus::server and all the node exporters inherit prometheus for common defaults, (they use to inherit from params.pp), but it's very difficult to see what prometheus parameters are common defaults and which ones are just for wrapping prometheus::server.

I was planning to add puppet strings docs, but don't think we should be doing that in two places. I'm happy to spend time refactoring, but am looking for ideas for the best way forward.

alexjfisher commented 4 years ago

It's worse than I thought. prometheus::server is actually missing plenty of parameters and prometheus::config mixes getting server specific values from prometheus and prometheus::server :(

alexjfisher commented 4 years ago

How about something like...

Option 1:

ekohl commented 4 years ago

My impression is that you can use exporters on a machine without the server itself so I don't think you can get rid of prometheus::server. Since it does make sense to have some common defaults, I think getting rid of prometheus and introducing prometheus::global makes the most sense.