Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prometheus "Counter" metrics (sent from Python client) are skipped with OpenTelemetry collector #678

Closed
nayasam opened this issue Jun 29, 2021 · 3 comments

Comments

@nayasam
Copy link

nayasam commented Jun 29, 2021

We observed Prometheus "Counter" metrics (sent from the Python prometheus_client API) are skipped (in OpenTelemetry collector) when configured to grab them (in PrometheusReceiver collector configuration) from a HTTP endpoint. However, this issue is not seen when Prometheus metrics are sent using the C# prometheus client.

With The Python prometheus_client API, Counter metrics sent have a _total suffix appended (this is not the case with C# prometheus client API). It appears that this _total suffix causes the Counter metrics to skip in the OpenTelemetry collector.

Upon checking the Python prometheus_client code - https://github.com/prometheus/client_python/blob/master/prometheus_client/registry.py#L58-L70 , we observe _total suffix is appended in Counter metric name; _sum and _counter suffixes are appended in Summary metric names; _bucket suffix is appended in Histogram metric names.

Then in OpenTelemetry collector code (version 0.28.0), the suffixes appended to Summary and Histogram metric names are trimmed, but not the _total suffix appended to Counter metric name.
https://github.com/open-telemetry/opentelemetry-collector/blob/main/receiver/prometheusreceiver/internal/metricsbuilder.go#L33-L46
https://github.com/open-telemetry/opentelemetry-collector/blob/main/receiver/prometheusreceiver/internal/metricsbuilder.go#L215-L222

However, when I update the code in metricbuilder.go (of the OpenTelemetry collector code) as follows (i.e., to trim the _total suffix of Counters) --- see bold text, I can see prometheus Counter metrics properly exported by the PrometheusExporter (of OpenTelemetry collector), and sent to the prometheus backend.
const (
metricsSuffixCount = "_count"
metricsSuffixBucket = "_bucket"
metricsSuffixSum = "_sum"
startTimeMetricName = "process_start_time_seconds"
scrapeUpMetricName = "up"
metricsSuffixTotalCount = "_total"
)
var (
trimmableSuffixes = []string{metricsSuffixBucket, metricsSuffixCount, metricsSuffixSum, metricsSuffixTotalCount }
errNoDataToBuild = errors.New("there's no data to build")
errNoBoundaryLabel = errors.New("given metricType has no BucketLabel or QuantileLabel")
errEmptyBoundaryLabel = errors.New("BucketLabel or QuantileLabel is empty")
)

Why do we see the skipping of prometheus Counter metrics when they are sent from the Python prometheus client API, and not with other prometheus client APIs (e.g., C#)? Is this a design bug of the prometheus client API (Python) as the particular issue is not observed from the C# prometheus client API?

@csmarchbanks
Copy link
Member

Hello, thank you for opening this issue.

I think this is an issue in open telemetry. All counters should end with a _total suffix, and that is actually required for OpenMetrics. I could be wrong but I believe the opentelemetry collector negotiates OpenMetrics preferentially, so it definitely needs to handle _total suffixes on counters. It looks like C# does not support OpenMetrics yet, so that may account for the difference you are seeing.

It looks like this bug in open telemetry might be what you are looking for? https://github.com/open-telemetry/opentelemetry-collector/issues/3118.

@bogdandrutu
Copy link

@csmarchbanks what about C# why do they not send _total? Can you ensure that in Prometheus you are doing consistent things across different implementations?

@csmarchbanks
Copy link
Member

csmarchbanks commented Jul 2, 2021

The C# client is maintained by the community, not anyone on the Prometheus team. In the C# client (and any other client that I have used) a user should add _total to all counter metric names. If the user does not know of the best practice/does not choose to abide by it, then some counter metrics will not end with _total. The example in C#'s documentation includes the _total suffix. This client, as the reference implementation of OpenMetrics, adds _total more aggressively, but as other libraries support OpenMetrics all counters will eventually end up with the suffix (unless they are mistyped as gauge or unknown).

I am going to close this issue as I don't think there is a bug/issue in this library, if it is shown otherwise I am happy to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants