have node file definitions use underscore instead of column

[ndelic0]

Pull Request (PR) description

Currently if collect_scrape_jobs is set to true all file definitions created in /etc/prometheus/file_sd_config.d/ will contain \:. This change would make file definitions look cleaner.
On the other note, if all the exporters running on a node are running on a loop back interface - behind reverse proxy, exported resource will complain on double declarations. This change should give us more ways to set file definition names.

This Pull Request (PR) fixes the following issues

[bastelfreak]

Thanks for the fix!

[antondollmaier]

Sorry for commenting on an already merged PR.

This change broke our custom prometheus::scrape_job usages: the Scrape job name must now be specified explicitly in the declaration, instead of relying on scrape_job to use the proper file name.

While appreciating the change itself, it's kind of breaking existing behaviour - and a note in the Changelog would have been appreciated.

Should a note be added to the changelog?

[ndelic0]

@antondollmaier Maybe you can share how custom prometheus::scrabe_job on your side? Wondering if this could be refactored so that it does not break existing behaviour.

[antondollmaier]

@antondollmaier Maybe you can share how custom prometheus::scrabe_job on your side?

Sure!

Previously working example below. It's used to add the native RabbitMQ Prometheus plugin of 3.8 without the exporter.

class mod::rabbitmq (
  Boolean $export_scrape_job = true,
  String $scrape_host = $facts['fqdn'],
  String $scrape_job_name = 'rabbitmq',
  Integer $stats_port = 15692,
) {
  if $export_scrape_job {
    @@prometheus::scrape_job { "${facts['fqdn']}:${stats_port}":
      job_name => $scrape_job_name,
      targets  => ["${scrape_host}:${stats_port}"],
    }
  }

Obviously, the change to my code is simple and does the job perfectly. Targets are scraped again.

-    @@prometheus::scrape_job { "${facts['fqdn']}:${stats_port}":
+    @@prometheus::scrape_job { "${scrape_job_name}_${facts['fqdn']}_${stats_port}":

When comparing with the previous version in prometheus::daemon, it was identical to my adaption (well, I copied from there):

    @@prometheus::scrape_job { "${scrape_host}:${scrape_port}":

BTW, the $job_name parameter seems to be unused as well. Dropping it will break more, I suppose, but I don't see any reference to that parameter any more.

As I said, I don't object in any way with the change (especially, as it's already released), I only wasn't able to read that change from the changelog and was left wondering why my targets weren't found any more.