search

census-instrumentation / opencensus-csharp

Distributed tracing and stats collecting framework
https://opencensus.io
Apache License 2.0
139 stars 32 forks source link

Counter type conversion inconsistency in Prometheus exporter #59

Closed shvez closed 5 years ago

shvez commented 5 years ago

hi, there. I'm trying to use your library. Thank you for it.

So, I found some inconsistency in the implementation of exporters. Namely, 'Sum' aggregation in case of Prometheus converted to 'gauge', in case of StackDriver to CUMULATIVE, which is 'counter'.

The fun is that neither of both is totally right. 'Sum' can represent either 'counter' or 'gauge'. For instance, when we count peers or users currently online it is very convenient to use 'Sum' and, in this case, it should be converted to 'gauge' by all exporters because value can go up and down The second case, when we count the amount of data sent. When we have just buffer length it is very convenient to use 'Sum', but in this case, a value will only grow and measure should be converted to 'counter'

So, I would propose to add a second type for 'Sum' aggregation. first will be converted to 'gauge' second to 'counter'

shvez commented 5 years ago

@SergeyKanzhelev Should I expect any reaction to this report?

SergeyKanzhelev commented 5 years ago

Current Prometheus implementation only provides a skeleton. There are many inconsistencies of metrics conversion. Marking it as up-for-grabs. If nobody will pick it up I think I'll get to it next week-ish

shvez commented 5 years ago

I understand that it is easy to fix this inconsistency. but I rather interested in understanding of the approach. why these obvious cases are not covered? or how to cover them using OpenCensus approach

SergeyKanzhelev commented 5 years ago

@shvez time/resources issue. It simply wasn't implemented and tested yet. This is exactly why we didn't put it on NuGet yet. Only MyGet.

shvez commented 5 years ago

@SergeyKanzhelev you have got me wrong. I was not clear. Sorry. When I asked about the approach, I thought about 'Sum' aggregation and a proposal to introduce two types of 'Sum' aggregation.

SergeyKanzhelev commented 5 years ago

Moved to OpenTelemetry