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

Issue when mapping OpenTelemetry MinMaxSumCount aggregator to Prometheus Gauge metric #228

Closed
CunjunWang opened this issue Jul 29, 2020 · 6 comments
Labels

Comments

@CunjunWang
Copy link
Contributor

CunjunWang commented Jul 29, 2020

Is your feature request related to a problem?

Yes. According to the specification here, we need to support MinMaxSumCount in OpenTelemetry. However, when I was translating OpenTelemetry metric data to Prometheus data type and mapping OpenTelemetry Aggregator to Prometheus, I referred to the specification here. The spec states we need to map MinMaxLastSumCount to Prometheus Gauge, and use the Last Value as the gauge metric. The problem is we don’t have a MinMaxLastSumCount aggregator. What we have is MinMaxSumCount aggregator but there is nothing can be mapped to gauge in this aggregator.

Describe the solution you'd like

  1. Ignore this aggregator when mapping from OpenTelemetry to Prometheus for now, if possible.

  2. Choose another value in the aggregator (min, max, sum, or count) to expose as the Prometheus gauge metric.

Describe alternatives you've considered

  1. Support the MinMaxLastSumCount Aggregator. This is not a preferred solution because the team that implemented the aggregators will end their internship next week. It’s not a good choice to ask them add new implementations at this point.
@jmacd
Copy link

jmacd commented Jul 31, 2020

I would be happy for you to deviate from the specification, use the LastValue aggregator (to give the correct Prometheus Gauge) and let us sort this out later. The MMLSC proposal is stuck behind some OTLP negotiations, and it's possible we'll reject it and declare that LastValue is the correct default for ValueRecorder anyway.

For the sake of getting something to work, reporting (Sum/Count) is not terrible. You can see this was debated already: open-telemetry/opentelemetry-specification#636

To be explicit, that's open-telemetry/oteps#117 and open-telemetry/oteps#118

@jmacd
Copy link

jmacd commented Jul 31, 2020

(sorry we didn't get to discuss this in the SIG meeting today!)

@CunjunWang
Copy link
Contributor Author

I would be happy for you to deviate from the specification, use the LastValue aggregator (to give the correct Prometheus Gauge) and let us sort this out later. The MMLSC proposal is stuck behind some OTLP negotiations, and it's possible we'll reject it and declare that LastValue is the correct default for ValueRecorder anyway.

For the sake of getting something to work, reporting (Sum/Count) is not terrible. You can see this was debated already: open-telemetry/opentelemetry-specification#636

To be explicit, that's open-telemetry/oteps#117 and open-telemetry/oteps#118

Thanks for your reply! I will use sum as a gauge at the current stage. If there is any update, I will modify my implementation then.

@erichsueh3
Copy link
Contributor

Hi @jmacd, just wanted to clarify on whether you mean Sum/Count as "Sum or Count", or as the average, "Sum divided by Count". I notice in the debate in #636 you mentioned Sum/Count as an average, but we just wanted to make sure here as well.

@lalitb lalitb added the metrics label Apr 19, 2021
@github-actions
Copy link

This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.

@github-actions github-actions bot added the Stale label Nov 27, 2021
@github-actions
Copy link

github-actions bot commented Dec 4, 2021

Closed as inactive. Feel free to reopen if this is still an issue.

@github-actions github-actions bot closed this as completed Dec 4, 2021
@lalitb lalitb removed the Stale label Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants