(PIE-51) Ability to define a metrics server to ship to #19
Conversation
Prior to this commit, there was no built-in way to ship metrics to an outside metrics server. After this commit, we pull in a script from puppetlabs/puppet-metrics-viewer that allows conversion of the metrics data we gather to influxdb or graphite format. This commit adds the initial support to enabling shipping of metrics directly to influxdb when given a hostname to connect to.
Previously, we only allowed passing an influxdb host but you may want to pass a port or a different database type for influx. Additionally, you may prefer to ship to graphite. You can now do any of these by using the correct metrics_server_info hash.
HelenCampbell
force-pushed
the
PIE-51
branch
2 times, most recently
from
c50e028 to
9bf8b19
Compare
|
@HelenCampbell Thanks for picking this up and making those changes! In #6 I was wrong about the |
HelenCampbell
changed the title
manifests/pe_metric.pp
Outdated
| $metrics_command = $metrics_server_type ? { | ||
| 'influxdb' => "${port_metrics_command} --influx-db ${metrics_server_db} > /dev/null", | ||
| 'graphite' => "${port_metrics_command} > /dev/null", | ||
| 'splunk_hec' => "${port_metrics_command} | puppet splunk_hec --sourcetype puppet:summary --saved_report > /dev/null", |
There was a problem hiding this comment.
should be the following for splunk_hec (no assumption on $path setting, etc) /opt/puppetlabs/bin/puppet splunk_hec --sourcetype puppet:metrics --pe_metrics
README.md
Outdated
| port => Optional[Integer], | ||
| db_name => Optional[String], | ||
|
|
||
| Specifies a metrics server to write data to. Currently it supports `influxdb` and `graphite` type servers. The parameters `metrics_server_type` and `hostname` are both required, and `dbname` is required for a `metrics_server_type` of `influxdb`. |
There was a problem hiding this comment.
add splunk_hec as data source, link to splunk_hec module since users need that setup first and then just selection splunk_hec, etc.
Since we define all attributes of a splunk server in the splunk_hec module we will remove it from the type here and just let a user define using a splunk_hec as a top level toggle.
Enables use_splunk_hec bool
|
@jarretlavallee this is ready for your review / merge - we had to update puppetlabs/splunk_hec to 0.7.1, but now we have a way to enable splunk support OR the metrics / graphite servers, etc. |
manifests/pe_metric.pp
Outdated
| @@ -10,6 +10,9 @@ | |||
| String $metric_script_file = 'tk_metrics', | |||
| Array[Hash] $additional_metrics = [], | |||
| Boolean $ssl = true, | |||
| Boolean $use_splunk_hec = false, | |||
There was a problem hiding this comment.
I'm kind of thinking we shouldn't special case splunk here. What about having an enum['graphite','influxdb','splunk'] export_to parameter and get rid of metrics server type inside of metrics_server_info.
|
@npwalker I've done an initial attempt at pulling out the type and putting it into parameters. Still need to fully test but is this implementation along the lines of what you were thinking of? Thanks |
|
This seems good. I wasn't necessarily thinking of moving all of the type out to individual parameters but that's mostly because I'm lazy and didn't want to have to add the parameters across all the classes. Since you already did the work to move them all out I think this is good and probably easier for most end-consumers vs using the hash of info. |
There was a problem hiding this comment.
This looks good to me. I tested it out with the following classification and it was able to ship the metrics correctly.
class{'puppet_metrics_collector':
metrics_server_type => 'influxdb',
metrics_server_hostname => 'pe-201910-agent.puppetdebug.vlan',
metrics_server_port => 8086,
metrics_server_db_name => 'puppet_metrics',
}
This is a rebase of the work done in #6 with the addition of a splunk_hec option.
All comments in the original PR should have hopefully been addressed.
Still currently a WIP.