Remove Temporality and add Structure #158
Conversation
Consensus was reached that the description of the time interval over which data was measured can be determined as `DELTA` or `CUMULATIVE` by comparison of the timestamps sent with the data. The classification of `INSTANTANEOUS` has also been found irrelevant in meaningfully description of the data. For these reasons, `Temporality` is removed. Conversely, an understanding of what operations can be performed on data is essential information to contain in this protocol so it can support systems of statistics or roll-up. To provide this support, the `Structure` enum is added and an appropriate field in the Metric descriptor included to capture if the data is adding or grouping.
MrAlias
requested review from
arminru,
bogdandrutu,
c24t,
carlosalberto,
iredelmeier,
jmacd,
reyang,
SergeyKanzhelev,
tedsuo,
tigrannajaryan and
yurishkuro
as code owners
| // request counts from a server. This data has ADDING structure, but if | ||
| // the two measurements were for different services, or even different | ||
| // servers, this result is likely meaningless. | ||
| enum Structure { |
There was a problem hiding this comment.
Is there a corresponding change to the API?
There was a problem hiding this comment.
(This brings the protocol closer to the existing API specification. ADDING structure is generated by Counter, UpDownCounter, SumObserver, and UpDownSumObserver instruments. GROUPING structure is generated by ValueObserver and ValueRecorder instruments.)
There was a problem hiding this comment.
Echoing what @jmacd said, this doesn't change the API. The added dimension allows the data to not lose meaning during transport.
| // request counts from a server. This data has ADDING structure, but if | ||
| // the two measurements were for different services, or even different | ||
| // servers, this result is likely meaningless. | ||
| enum Structure { |
There was a problem hiding this comment.
(This brings the protocol closer to the existing API specification. ADDING structure is generated by Counter, UpDownCounter, SumObserver, and UpDownSumObserver instruments. GROUPING structure is generated by ValueObserver and ValueRecorder instruments.)
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
|
@MrAlias can you please show how this would map to OpenCensus Proto metric types? We will need to do this translation in the Collector but I am not sure how it will need to be mapped. |
@tigrannajaryan that is a good question. My understanding is that the @jmacd does this sound correct to you? |
|
I am starting to have doubts about this change. Initially my goal was to provide some information about the original instrument--I'd like to know whether the inputs were ADDING or GROUPING, but I'm not sure it matters for the consumer of an aggregation whether the inputs were ADDING or GROUPING. I still think we should be able to know the kind of the source instrument, but it's not clear how the information will be used. I have a different kind of doubt about the translation to and from the OpenCensus format. I do not believe we have a correct conversion from OpenCensus into the protocol before this PR, much less after this PR. The OpenCensus GAUGE_INT64, GAUGE_DOUBLE, GAUGE_DISTRIBUTION would become Temporality=INSTANTANEOUS before this PR, Structure=GROUPING after this PR, I think. The OpenCensus CUMULATIVE_INT64 CUMULATIVE_DOUBLE CUMULATIVE_DISTRIBUTION could become either DELTA or CUMULATIVE. OpenCensus is not telling whether the caller should expect the next interval to reset the start time or not. In that sense, we're better off AFTER this PR because there is no more temporality field: OpenCensus is not telling us temporality. One of my colleagues made an argument for keeping Temporality (at least the notion of Cumulative vs Delta) in OTLP. The argument goes: Sure, you can infer whether a point is DELTA or CUMULATIVE after you've seen two data points, but you're likely to want to validate the timestamps, and the validation logic will be different. Having explicit information will make processing CUMULATIVE and DELTA data easier, the argument goes, because an individual point will carry that information. (To repeat, OpenCensus doesn't tell us this.) I am struggling to find a conclusion to this. I don't know that we need either Structure or Temporality. I still find myself wanting to know the kind of the original instrument, but I'm not convinced that's needed either. |
|
After the Metric SIG meeting today it sounds like we are circling on the correct direction here, but not strongly enough to move this forward without exploring other issues and approaches. Going to close with the anticipation it will be retried in a week or so after we have further explored these topics. |
Consensus was reached that the description of the time interval over which data was measured can be determined as
DELTAorCUMULATIVEby comparison of the timestamps sent with the data. The classification ofINSTANTANEOUShas also been found irrelevant in meaningfully description of the data. For these reasons,Temporalityis removed.Conversely, an understanding of what operations can be performed on data is essential information to contain in this protocol so it can support systems of statistics or roll-up. To provide this support, the
Structureenum is added and an appropriate field in the Metric descriptor included to captureif the data is adding or grouping.
Relates to #125
Related to open-telemetry/opentelemetry-specification#625