Metrics Proto Does Not Support the OpenTelemetry Specification about opentelemetry-proto HOT 15 CLOSED

If the goal of the OTLP is to transport the output of Instrument -> Aggregator, it should be modeled after the output of the Aggregators, aggregations. And while, yes, the Histogram is one of these aggregations, the included histogram kinds are conflated with instrument qualifiers (Gauge, Cumulative).

I think the goal of OTLP should be:

1. Support Aggregator output - in a generic way and not having 100s of combinations like SumCount, SumCountMin, etc.
2. There is a request to also support the output for synchronous Instruments -> to a possible Aggregator. This is still under discussion but will be addressed.

from opentelemetry-proto.

@MrAlias our approach so far has been that we file individual issues and PRs that fix them, gradually moving towards the desirable final proto. We had more time to do this for traces and it looks like it went well. With metric we had less time and there are issues that need to be addressed.

If you know what the right fixes to the identified issues are it would be great if you could submit corresponding PRs.

major overhaul to provide first class support for OpenTelemetry. Possibly being done in a v2 of the proto.

Is there such a disconnect that requires a major overhaul? I am not a metric spec/SDK contributor, so I cannot assess myself how big is the disconnect.

@open-telemetry/specs-approvers whatever changes we want to do to the proto it is very desirable to do them quickly. I know multiple people who are currently ramping up to write code that assumes the current proto. If we do not finalize the proto quickly we will cause pain by introducing breaking changes.

If we are unable to have the final metric proto version within the next few days it is best that we announce the metric part of the protocol as not ready for implementation.

from opentelemetry-proto.

If we are unable to have the final metric proto version within the next few days it is best that we announce the metric part of the protocol as not ready for implementation.

@tigrannajaryan what did you decide on this? As it's written now, the types in the metrics proto are very different than the instruments and aggregations in the spec or OTEPS (e.g. open-telemetry/oteps#88 and open-telemetry/oteps#93).

Unless we expect to change the spec to match the proto this looks to me like it's not ready for implementation.

from opentelemetry-proto.

@c24t I do not know enough about the metrics to make a call on this. @bogdandrutu @jmacd can you help?

from opentelemetry-proto.

@tigrannajaryan: I started in on a proposal refactor of the proto, but have not found the time to get it into a presentable state. The rough structure of the proposal is to switch to something closer to the Stackdriver proto model for metrics and update the data points to be more along the lines of instantaneous, delta, and cumulative values. I'm hoping to get this more formalized and will share when it is.

I agree with @c24t in that it might be wasted work to implement at this point as a I think there needs to be a refactor to support the OpenTelemetry metric data.

from opentelemetry-proto.

I look forward to seeing this proposal @MrAlias.
I've updated OTEPs 93 and 96 with the terms "delta", "cumulative", and "instantaneous", fwiw.

from opentelemetry-proto.

Do note that we have an implementation of metrics proto in Collector already and increasingly large amount of code depends on it because it is the foundation of internal representation.

The longer we wait for the proposal the more code needs to be rewritten. If the proposal doesn't happen quickly them we have a problem, we do not have time rewrite thousands of lines of cod (depending on how big the changes are).

from opentelemetry-proto.

If this gets delayed more we may end up staying with OpenCensus representation in the Collector, since I don't think we will have time or people to rewrite all that code. The consequence will be that metrics will work significantly slower in the Collector.

from opentelemetry-proto.

from opentelemetry-proto.

@jmacd do you think the current proto definition is robust enough to encode all of the aggregations you envision for OTEP-88 and following?

from opentelemetry-proto.

from opentelemetry-proto.

After some progress in both the set of instruments and the terminology used to explain them, I've come to think that the ideal set of properties for describing an aggregation are:

• Collective vs Additive representation
• Made synchronously vs asynchronously
• Monotonic output.

These can be stored using 3 bits. These bits are independent of whether a unit of metric data is considered as "cumulative" or "delta", the so-called temporal quality. In my thinking, both kinds of data are characterized by two timestamps: the start of the interval, and the end of the interval over which the data applies. A cumulative expression of metric data will re-use the same start time, as in: [T0, T1], [T0, T2], [T0, T3], ...

A delta expression of metric data will not re-use the start time:
[T0, T1], [T1, T2], [T2, T3], ...

from opentelemetry-proto.

@MrAlias this is very hard to track, do you mind splitting in smaller PRs so we make sure every issue is track accordingly.

from opentelemetry-proto.

@MrAlias is there anything left in this issue, I feel like more than 80-90% of the concerns were addressed. Would like to close this and recommend open a new issue if there is anything else left.

from opentelemetry-proto.

@MrAlias is there anything left in this issue, I feel like more than 80-90% of the concerns were addressed. Would like to close this and recommend open a new issue if there is anything else left.

SGTM. Was just going to recommend the same 👍

from opentelemetry-proto.