[Metricbeat] Prometheus based module should support metrics filtering

[mtojek]

Lightweight modules based on Prometheus store all collected metrics. Unfortunately if there are metrics not related to the observed product (e.g. exporter's metrics), they will be collected as well, even if they're less important and just overloading the storage).

The goal of this task is to expose a module configuration feature that defines allowed metric name prefixes.

This feature could be used by the IBM MQ module: #15301

[ChrsMark]

Proposal

Still need to investigate how it could be implemented on the collector's level. Most probably we will have to scape the whole response from the endpoint and filter out the metrics accordingly before sending the events.

Collector config options

1. Include metrics:

include_metrics:
  - ibmmq_*

This would only keep metrics that start with ibmmq_. In the example of ibmmq module this is currently implemented with the script processor.

2. Ignore Metrics

ignore_metrics:
  - process_*
  - go_*
  - istio_mcp_request_acks_total

This would filter out all unnecessary metrics (see the source at istio sample metrics)

3. Metrics

metrics:
  - mixer_config_attributes_total: config_attributes
  - mixer_handler_daemons_total: handler_daemons
 ...

This would implement the mapping which defines which metrics should only be collected and how they should be mapped to fields.

In order to cover cases where we only want to map some specific metrics but we want to keep all the rest as is a * option could be used maybe:

metrics:
  - *
  - mixer_config_attributes_total: config_attributes
  - mixer_handler_daemons_total: handler_daemons

Combining these options

The process of filtering out metrics would be first to apply the include_metrics/ignore_metrics (they are complementary) filters and then on the remaining metrics run the metrics mappings.
None of the above metrics are strictly required. At a first glance the filters could be applied at

beats/metricbeat/module/prometheus/collector/collector.go

Line 83 in 1c32be3

for _, family := range families {

, where we could check if every family should be fetched or not, according to family.Name and the filter lists.

[ChrsMark]

This could be useful for OpenMetrics( #14982) module too.

[jsoriano]

Comments moved from #14982:

Still need to investigate how it could be implemented on the collector's level. Most probably we will have to scape the whole response from the endpoint and filter out the metrics accordingly before sending the events.

Yes, probably, I don't think exporters in general support filtering. Maybe we can do the filtering in GetFamilies() and at least we will save some parsing and the creation of some objects. If it is tricky to do it there, doing it where you mention in the collector sounds good to me too, and we can optimize later if needed.

In order to cover cases where we only want to map some specific metrics but we want to keep all the rest as is a * option could be used maybe.

I would prefer an explicit option for something like this, or we could also not support using both filtering and mapping to avoid tricky cases.

[exekias]

Have we considered using existing processors to filter out unwanted metrics? The clear benefit is that it provides the same method for all modules, instead of something Prometheus specific. That said, I see value in what you propose, as it should save some memory & cpu we use when processing unwanted metrics, just to drop them afterwards.

Regarding the config options, include_metrics sounds great, but I think the other option would be better called exclude_metrics for consistency. WDYT?

I have mixed feelings about allow for custom mappings in Prometheus metrics. We have done them in the past for some modules, using the Prometheus helper, but as we move forward with lightweight modules/inputs it sounds to me that keeping the original names is better for users. Specially once we provide better ways to query them as they would expect. Perhaps we should leave mappings out of the scope of this issue and discuss the need in a different one?

[ChrsMark]

Thanks @exekias for commenting here! Answering inline:

Have we considered using existing processors to filter out unwanted metrics? The clear benefit is that it provides the same method for all modules, instead of something Prometheus specific. That said, I see value in what you propose, as it should save some memory & cpu we use when processing unwanted metrics, just to drop them afterwards.

Processors are already used in ibmmq light-module . My perspective here is that using processors in general to handle keep/drop/edit fields is ok, however in Prometheus case I would prefer something more native and user-friendly. This would not only be useful for all the light modules that are based on collector metricset but also in OpenMetrics light module. OpenMetrics module would need to be manually tuned according to each case. So if a user want to use OpenMetrics module for a custom case and want to filter out metrics (I see this to be quite common) then they would have to write the script processors. On the other hand if they have the option to do it natively it would be more user friendly. Actually this is what other monitoring agents do out there.

The fact that we gain in performance by skipping events is also a plus.

Regarding the config options, include_metrics sounds great, but I think the other option would be better called exclude_metrics for consistency. WDYT?

I agree!

I have mixed feelings about allow for custom mappings in Prometheus metrics. We have done them in the past for some modules, using the Prometheus helper, but as we move forward with lightweight modules/inputs it sounds to me that keeping the original names is better for users. Specially once we provide better ways to query them as they would expect. Perhaps we should leave mappings out of the scope of this issue and discuss the need in a different one?

I agree with that too! As it was mentioned in the implementation PR #16420 we can leave it out for now. To be honest having in mind OpenMetrics case I think that include/exclude_metrics options are enough!

[mtojek]

Here comes another PR that would appreciate this feature: #16482

[ChrsMark]

PR was merged. Closing this one.