Dual placement of Labels makes certain translations expensive about opentelemetry-proto HOT 5 CLOSED

@bogdandrutu what do you think?

from opentelemetry-proto.

This approach is probably less desirable since it makes impossible to translate series of data points from OTLP to OpenCensus TimeSeries because each DataPoint can have a different set of keys (not possible in OpenCensus).

This is already the case unfortunately (I tried my best to fight for having this as a requirement that a metric always produces the same set of label keys, @jmacd pushed to not have this requirement). So in open-telemetry we don't know the set of label-keys that we produce and the default aggregation for all the instruments will produce unbounded number of label-keys. I know this is not easily compatible with OpenCensus but I don't think we can do anything at this moment, unless we add the constraint to predefined the keys. There was a tentative way to solve this by using the "recommended label keys", but that was not enforced as requirement to produce aggregation only using these keys, so the problem was not resolved.

I suggest that we eliminate one or the other and have LabelKeys in one place only.

We can remove only the labels from the MetriDescriptor if we want to remove any of them.

So overall I think we will have to pay a large cost of translating from OTLP to OpenCensus anyway because of the fact that the set of available label keys is not known in OTLP. So you need to iterate over all the points and find the set of label keys anyway. So I am not sure how does the Approach 2 help with this problem.

I do agree that if we keep both we need to define a mechanism to remove duplicates, and maybe the easiest way for the moment may be to remove the MetricDescriptor labels (we will need them soon) but we can discuss that when we need them. But keep in mind that if you remove these labels you still don't achieve what you want - faster conversion between OTLP and OpenCensus.

from opentelemetry-proto.

@tigrannajaryan I think the main cause why the translation is expensive is not the dual labels is actually the fact that OpenTelemetry do not enforce the set of label-keys for an instrument, so you may want to clarify the issue title.

from opentelemetry-proto.

FYI: I can see a lot of the protocols not requiring the label keys to be predefined so we have to do a trade-off, if we relax the protocol we can have a more optimal conversion for other protocols, and with the idea that OpenCensus will be deprecated is probably better long term to not require that.

from opentelemetry-proto.

@bogdandrutu I do not quite understand what does "predefined" label mean. Perhaps I was not clear in my description, let me try to clarify.

The problem that I see with current protocol definition is shown by the following example. Let's assume I have the following OTLP data (pardon the data syntax):

"metric": {
  "descriptor": {"labelkeys": {"a": "1", "b": "2"}},
  "int64_data_points": [{"value":100,"labelkeys": {"a": "5", "c": "10"}}]
}

If I need to translate this to OpenCensus I have to merge labelkeys in a way that "a" is only listed once and the data point overrides:

"metric": {
  "descriptor": {"labelkeys": {"a": "5", "b": "2", "c": "10"}},
  "timeseries": [{"value":100}]
}

To do this merging we need expensive per-key merging over lists.

This is not only a problem with OpenCensus format but with any other format that does not have OTLP-like dual placement of labels with implied overridinig of values (I am not aware of any metric formats that allow this, which means many other format translations will be slow too).

If it is necessary to have the labelkeys in 2 places, because it makes client libraries simpler (is this true?) then at least I think we should require that the 2 labelkeys maps do not have overlapping keys (so that "a" cannot exist in 2 places in the above example). In that case labelkeys can be merged by concatenation, which is faster.

@jmacd can you clarify what is your requirement and why we need this?

from opentelemetry-proto.