[receiver/prometheus] Add config option to allow dropping of untyped metrics

[sky333999]

Component(s)

receiver/prometheus

Is your feature request related to a problem? Please describe.

As a result of open-telemetry/opentelemetry-collector#1103, currently, all untyped metrics are converted to gauges and there is no way to determine later on in the pipeline if a metric was originally untyped.

While converting to gauge might be ok in most cases, there could be scenarios when customers instead choose to drop all untyped metrics (for example to filter out any go internal scrape_ metrics).

Describe the solution you'd like

Add a config option that would allow customers to specify if they would like untyped metrics to be dropped.

Describe alternatives you've considered

An alternative could be to introduce a label on the metric to capture the "original" metric type thus allowing customers to then have some sort of a filter processor to drop metrics if needed.

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

Pinging code owners for receiver/prometheus: @Aneurysm9 @dashpole. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[dashpole]

This sounds reasonable to me. Also related is open-telemetry/prometheus-interoperability-spec#69, which would try to preserve the notion that the metric type was unknown.

[fatsheep9146]

Do you have enough bandwidth to add this config? @dashpole

[kovrus]

I am trying to get more familiar with the code base of the Prometheus components. I'd be happy to work on this issue.

Do we want to address this issue with the config option or preserve the notion of unknown type in metric attributes?

[dashpole]

Do we want to address this issue with the config option or preserve the notion of unknown type in metric attributes?

I think preserving the notion of unknown would be better, but is definitely less straightforward.

If we choose to try and preserve the notion of unknown, we should probably make changes to the conversion spec here before moving forward with the implementation.

Do you have enough bandwidth to add this config?

Not at the moment.

[sky333999]

Would memory overhead be also be a consideration for adding an additional attribute for every metric for all consumers of the receiver?

Perhaps that should also be opt-in via a config option? Something along the lines of:

config:
    includeMetaAttributes:
        - originMetricType

Also, one more option is to have ability to both drop untyped metrics and add this additional attribute - not sure if thats an overkill.

[dashpole]

I would expect only unknown-typed metrics to get an additional attribute. Metrics with a type should be left unmodified.

[khanhntd]

Would it be better to not converting to Gauge but instead converting to MetricTypeEmpty? There are several reasons for that:

• Firstly, there are no processors/exporter that support for unknown-typed. If we convert to Gauge, would it corrupt the data? Its better to let the processors/exporter to set an appropriate actions for unknown-type.
• Secondly, we will always have an resource attribute of the receiver (e.g awsemfexporter knows the metrics coming from prometheus)
• Thirdly, its a waste to have addition datapoint attributes and its scale based on metrics (not a good thing for resources usage).

[dashpole]

We discussed using MetricTypeEmpty around when this issue was first opened. From what I recall, MetricTypeEmpty isn't a "real" OTel metric type. It is the default value of the pdata metric type to indicate that something isn't right, and processors/exporters that are given such a metric are expected to drop it with an error.

Firstly, there are no processors/exporter that support for unknown-typed. If we convert to Gauge, would it corrupt the data? Its better to let the processors/exporter to set an appropriate actions for unknown-type.

Thats because there isn't such a type to reference today :). The prometheus, prometheusremotewrite, and googlemanagedprometheus exporters would make use of such a type if it existed. I'm not sure what you mean about corrupting data. At the time, Gauge seemed like the least bad option, but I can understand that being confusing in some cases.

Secondly, we will always have an resource attribute of the receiver (e.g awsemfexporter knows the metrics coming from prometheus)

Where does the prometheus receiver set that resource attribute, though?

Thirdly, its a waste to have addition datapoint attributes and its scale based on metrics (not a good thing for resources usage).

A single, non-identifying attribute on a (hopefully) small number of metrics shouldn't make much of a difference in terms of increased resource usage. My bigger hesitation is that looking for a particular attribute it seems like poor contract for exporters. Something like a data point flag feels like a better fit...

[Aneurysm9]

A single, non-identifying attribute on a (hopefully) small number of metrics shouldn't make much of a difference in terms of increased resource usage. My bigger hesitation is that looking for a particular attribute it seems like poor contract for exporters. Something like a data point flag feels like a better fit...

+1

I think a data point flag would be the ideal representation of that information. I'm not sure whether it would have any applicability outside of this use case, though, which may lead to resistance to including a new flag in the spec.

[khanhntd]

Hi @kovrus,
Thanks for contributing and can I ask are there any progress towards the issues? If not, we can pick it up and deliver the sol for the issue since its time sensitive for us.

[khanhntd]

I am working on it. Please assign to me this issue though ! Will have the PR by the end of the day

[fatsheep9146]

If we choose to try and preserve the notion of unknown, we should probably make changes to the conversion spec here before moving forward with the implementation.

@khanhntd thanks for your contribution!
but I think before implementation, you should make changes to the conversion spec as @dashpole suggested.

[khanhntd]

Thanks @fatsheep9146 for the recommandation. Yes that is my next TODO (as mentioned in PR). I'm fine with updating the spec first too .

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• receiver/prometheus: @Aneurysm9 @dashpole

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been closed as inactive because it has been stale for 120 days with no activity.