Add MinMaxSumCount data type #122
Conversation
The specification includes this as a default aggregation type, however, the proto has no way to transfer this data natively. This adds a new data type and datapoints to be used in this transfer.
MrAlias
requested review from
arminru,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
jmacd,
reyang,
SergeyKanzhelev,
tedsuo,
tigrannajaryan and
yurishkuro
as code owners
|
We can use Summary with 0 percentile min and 1 percentile max |
|
@jmacd & @bogdandrutu: further down in the conversation that @jmacd linked both @c24t and @jkwatson continued to discuss the fact that the OpenTelemetry proto definition did not match the OpenTelemetry specified aggregations, and there was desire there to have them match. It seems like this proto definition is a loud echo (or a direct renaming) of the OpenCensus proto. As such, it seems like this kind of hold over is going to provide future confusion and friction. Parts of the OpenTelemetry project will be bound by a previous specification's design choices. The solution @bogdandrutu proposed indeed can be used to solve this transport mismatch, but should it? I think for the same reasons we decided to include more than just Are there other reasons besides an opposition to changing what was inherited that are leading you to oppose this inclusion? I'm interested to understand both of your reasons on this issue, and guessing I've overlooked something. |
|
Your comment makes me think that we actually misdefined what is the difference between The Another reason is that the cardinality of the combinations I would be happy to add a new Aggregation in the proto, if that gives us a clear benefit, but for this specific request I do not see what is the advantage to know that this is
We are trying to also stay compatible with OpenMetrics protocol which does not define a MinMaxCountSum representation. |
|
What if we add an optional |
|
@jkwatson that is an interesting reason and I would I would like to document a bit about that. I personally didn't know that is the case, will investigate more. But hope that my explanation makes sense why we don't need an aggregations for every possible aggregators. |
|
To quote wikipedia: so, there is, by definition, nothing in the "0th" percentile. :) |
|
I've realized I probably should have opened an issue prior to this to have the discussion there. Going to close in favor of #125 where I hope we can come to a solution to better support OpenTelemetry aggregations. |
Update metric descriptor to contain information on the data qualities. This includes: - The meaning of the time interval measurements were made over is now described by the `interval` value. - The monotonic nature of the data is captured if known. - If the data values are restricted to a subset of numbers. The `SummaryDataPoint` and `HistogramDataPoint` were merged into a unified `Distribution` message that contains statistics about the observed population of values. Adds support for multiple histogram bucket definitions and adds a linear bucket definition message. Includes an explicit minimum and maximum for a `Distribution` (open-telemetry#122). Removes the exemplar code (open-telemetry#81). This can be added back in as a more generalized case now given the new `Data` structure of the Metrics. But, it has been left for a separate PR. Unifies the common parts of the previous `*DataPoints` into a unified `Data` message. This is not complete. It likely needs better names and it certainly needs comments. Related to open-telemetry#125 Fixes to open-telemetry#81
The specification includes this as a default aggregation type, however, the proto has no way to transfer this data natively. This adds a new data type and datapoints to be used in this transfer.