Page MenuHomePhabricator

Review prometheus_nodes params
Open, MediumPublic

Description

In https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/463143/6/modules/profile/manifests/mail/smarthost.pp@29 @faidon raised the question "Should it be another profile to indicate that the host is monitored by Prometheus. That port looks like it's mtail-specific, so perhaps an mtail-specific profile?"

Since this approach is used by many other classes the scope is larger than the one patch above. Creating this task to review and discuss the current state as a whole, and consider/plan possible alternatives to abstract the prometheus config.

Event Timeline

herron triaged this task as Medium priority.Oct 17 2018, 3:23 PM
herron created this task.

I don't think this is the pattern widely adopted in our codebase.

What we typically do is:

  • Create a specific profile for the exporter, so in this case probably profile::prometheus::mtail, separated from any profile where we might use it into
  • Include both profiles into your role

This would mean that the same code/configuration can be shared between all servers that need to export mtail stats to prometheus.

If this is impractical because you need to change the configuration for the exporter in every case, then you're in one of those fringe cases where a mixed approach could be valuable: you should require profile::prometheu::mtail at the start of your profile, then add an explicit configuration in your class, that notifies the exporter's service.

Let me add that in line of principle there is nothing wrong with calling prometheus_nodes from a random profile where it could be useful; but in this specific case that signals you're factoring your code in a suboptimal way because it won't allow wider re-use.