WIP: add ability to export/collect scrape_jobs #141
Conversation
costela
changed the title
|
Fixes #126 |
| @@ -3,10 +3,11 @@ | |||
| class prometheus::config ( | |||
| $global_config, | |||
| $rule_files, | |||
| $scrape_configs, | |||
| Array[Hash] $scrape_configs, | |||
Datatypes \o/
Do you know the structure of the Hash? Could that be enforced as well within the datatype?
how about we just don't try to fix that in this specific pull request? there wasn't a type before either...
| Array[Hash] $scrape_configs, | |
| $scrape_configs, |
:P
manifests/daemon.pp
Outdated
| $service_ensure = 'running', | ||
| $service_enable = true, | ||
| $manage_service = true, | ||
| Boolean $export_scrape_job = false, |
Datatypes \o/
| Array $alert_relabel_config = $::prometheus::params::alert_relabel_config, | ||
| Array $alertmanagers_config = $::prometheus::params::alertmanagers_config, | ||
| String $storage_retention = $::prometheus::params::storage_retention, | ||
| Array[Hash] $collect_scrape_jobs = $::prometheus::params::collect_scrape_jobs, |
Datatypes \o/
manifests/scrape_job.pp
Outdated
| String $job_name, | ||
| Array[String] $targets, | ||
| Hash[String, String] $labels = {}, | ||
|
|
please remove the newline here
manifests/scrape_job.pp
Outdated
| mode => $mode, | ||
| content => template("${module_name}/file_sd_config.yaml.erb"), | ||
| } | ||
| } |
please add anewline
|
This looks good on a first view, but we need tests for it. |
|
What's the state of this? It would be really handy for me ;) |
|
@viq sorry, this kinda fell between the cracks. |
|
I am super interested in this. Can you give me an idea of what still needs to happen? Not sure I can help with much, other than real world testing... |
|
@ajcollett sorry, I just haven't had the time to add the needed tests. And now I also have to rebase and solve conflicts... |
|
No problem. I hope it's easier than it seems! I am slightly surprised this particular issue isn't more popular. Is it just that people are all doing this themselves anyway? |
|
@ajcollett personally, I use consul for this.It already knows all services that I have, they expose a prometheus endpoint. Also I've got certain exporters that are also registered to consul. That scales way better than exported resources. |
| @@ -78,6 +78,9 @@ | |||
| $manage_service = true, | |||
| Hash[String, Scalar] $env_vars = {}, | |||
| Optional[String] $env_file_path = $::prometheus::params::env_file_path, | |||
| Boolean $export_scrape_job = false, | |||
| String $scrape_host = $::fqdn, | |||
Please use $facts['fqdn']
| @@ -0,0 +1,3 @@ | |||
| # this file is managed by puppet; changes will be overwritten | |||
Could you provide this as a simple file resource without a template? We could pass the content to it with https://github.com/puppetlabs/puppetlabs-stdlib#to_yaml
| # Array of scrape_configs. Format, e.g.: | ||
| # - job_name: some_exporter | ||
| # scheme: https | ||
| # The jobs defined here will be used to collect resources exported via prometheus::daemon, |
i find this datastructure a little over-complicated and inconvenient - what if i want to collect all jobs, regardless of the type? the way it's setup now, i have to specify each one by hand....
i'm thinking of simplifying this and simply make that a mixed type of boolean / array of job names. if it's a list, it behaves basically the same way (minus the scheme override, which belongs in the scrape_job def). if it's a boolean and true, all jobs are collected. if it's false, nothing is collected of course.
i'm working on an update of this PR to make it work with latest master which includes those changes.
Thanks for picking up the work!
I'm not sure I understand your suggestion though. I'll follow up on your new PR #304.
|
okay, i have done my homework and this is actually working in production in #304 so I think this PR should be closed and discussion be moved there. |
|
agreed |
No description provided.
The text was updated successfully, but these errors were encountered: