Add GetProcessedMetrics method to Prometheus helper
#6916
Conversation
New `GetProcessedMetrics` method to fetch all families and map them to Metricbeat fields based on a given mapping.
exekias
force-pushed
the
prometheus-helper-reloaded
branch
2 times, most recently
from
0f3dd50 to
30cabe8
Compare
exekias
force-pushed
the
prometheus-helper-reloaded
branch
from
30cabe8 to
a74d300
Compare
| } | ||
| } | ||
|
|
||
| // KeyLabel maps a Prometheus label to a Metricbeat field. The lable is flagged as key. |
| } | ||
|
|
||
| // Metric directly maps a Prometheus metric to a Metricbeat field | ||
| func Metric(field string) MetricMap { |
There was a problem hiding this comment.
Should we make a difference between float and int?
There was a problem hiding this comment.
Uhm that's a good question, so far I think that mapping should take care of that?
There was a problem hiding this comment.
It indeed does for the define fields. Coming from the schema package I kind of expected to have both.
| @@ -26,14 +26,14 @@ | |||
| - name: cpu | |||
| type: group | |||
| fields: | |||
| - name: limit.nanocores | |||
| - name: limit.cores | |||
There was a problem hiding this comment.
As state_container is already GA we should not do breaking changes. Could we perhaps keep both fields but mark one as deprecated? Same below.
There was a problem hiding this comment.
I've pushed a change keeping them (deprecated)
| ExtraFields map[string]string | ||
| } | ||
|
|
||
| func (p *prometheus) GetProcessedMetrics(mapping *MetricsMapping) ([]common.MapStr, error) { |
There was a problem hiding this comment.
It reminds me a lot of our schema package.
There was a problem hiding this comment.
Definitely inspired on it :)
|
|
||
| // Ignore unknown metrics | ||
| if !ok { | ||
| continue |
There was a problem hiding this comment.
We should sync up the behaviour with the schema package on what we do on error / missing fields. I'm not happy with the current error handling in schema. Let's have a follow up chat on how we should push that forward.
CHANGELOG.asciidoc
Outdated
| @@ -39,6 +39,8 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di | |||
| - Add config option for windows/perfmon metricset to ignore non existent counters. {pull}6432[6432] | |||
| - Refactor docker CPU calculations to be more consistent with `docker stats`. {pull}6608[6608] | |||
| - Update logstash.node_stats metricset to write data under `logstash.node.stats.*`. {pull}6714[6714] | |||
| - Changed `state_container` metric from `cpu.*.nanocores` to `cpu.*.cores`. | |||
| - Fixed typo in values for `state_container` `status.phase`, from `terminate` to `terminated`. | |||
There was a problem hiding this comment.
Here we could perhaps use copy_to to copy the field for now?
There was a problem hiding this comment.
I used code as there is a change on the scale, so needed to perform the math
exekias
added
review
Metricbeat
| - name: limit.nanocores | ||
| type: long | ||
| deprecated: true |
There was a problem hiding this comment.
Now we need a way so that this shows up in our docs ...
This PR adds a new
GetProcessedMetricsmethod to ease the creation of metricsets based on Prometheus. It moves from code (current approach) to a mapping template for the conversion of Prometheus metrics to our format, better suited for Elasticsearch mappings.With this new method you only need to fill the
MetricsMappingstructure to map all metrics to events. There are a couple of different use cases to support different patterns in Prometheus metrics, like a way to extract labels intokeyworudvalues, or doing boolean conversion.This PR is divided into 2 commits:
state_*metricsets to make use of itI plan to use this for incoming modules/metricsets.
state_containermetricset (currently beta):status.phasevalue:terminate->terminatedAlso this deprecates calculated metric
cpu.*.nanocoresin favor ofcpu.*.cores.