[enh] Make max_accumulations configurable and add remove method for meters.

[tjiuming]

Make max_accumulations configurable and add remove method for meters.

Related PR: open-telemetry/opentelemetry-java#4647

[cijothomas]

what is "accumulation"? This seems like an implementation detail from Java?

[tjiuming]

like Counter.Child in Prometheus. Currently, OTEL SDK limits the maximum number of Child to 2000, it should be configurable or Unlimited

[cijothomas]

Dont see anything like "Counter.Child" in OTel specs. Most likely this is something very java specific.

[tjiuming]

So, in Otel specs, the number of child in unlimited? accumulation's limits is just a java spec?

how about remove method? should it be added to Otel spec?

[tjiuming]

@cijothomas sorry, I've read the open-telemetry/opentelemetry-java#4647 's comments carefully, max_accumulations is truly a java spec, please ignore about it.

besides, I wonder that if remove method should be added to Otel spec?

[jack-berg]

Accumulation is a term internal to java's implementation. The max accumulations refers to the number of distinct attribute sets / timeseries that are allowed in memory for a particular instrument at any time. If exceeded, we log a warning and drop the measurement.

The topic of metrics SDK memory limits was punted on for the initial stable release of the metrics spec, and in java (and I believe dotnet) we opted for defensive approach with a fixed limit until the spec clears things up.

[cijothomas]

Thanks for clarifying! Yes .NET also uses its own mechanism, called SetMaxMetricPointsPerMetricStream.
https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/docs/metrics/customizing-the-sdk#changing-maximum-metricpoints-per-metricstream

Would be good to have it in spec. (allowing sufficient flexibility so that we wont break existing API in .NET)

[tjiuming]

@jack-berg @cijothomas If I understand you correctly, it's good to add max_accumulations and remove to spec right?

[cijothomas]

I dont know if there is a need to have remove.
I definitely like spec to have the equivalent of max_accumulations. (need a diff. name, as accumulation is java specific.) .NET named it SetMaxMetricPointsperMetricStream

[jmacd]

I believe this is the same as #1891

[jmacd]

OpenTelemetry has resisted adding a remove() or delete() method; I personally do not feel it is necessary. We are able to reset start timestamps and eject streams from memory, we just need to clearly write the specification.

[tjiuming]

OpenTelemetry has resisted adding a remove() or delete() method; I personally do not feel it is necessary. We are able to reset start timestamps and eject streams from memory, we just need to clearly write the specification.

if there are a meter named broker_published_message_count to record each topic's incoming messages, we often code like this:

Counter counter = ...
counter.add({namespace = 'xxx', topic = 'xxx'}, 1)

after the topic deleted, I believe we need to discard the accumulation directly, not through other indirect ways:

counter.remove({namespace = 'xxx', topic = 'xxx'})

[jmacd]

@tjiuming thanks for explaining. We should discuss these two problems separately. For the limit question, please use #1891.

Although I understand the precedent set in Prometheus and implied meaning in OpenMetrics when data is removed from a metrics pipeline, I would like OpenTelemetry to ensure correctness. In the example you gave, it is difficult to know when it is safe to remove the metric; we might for example specify that data must be reported before it can be removed, which leads to fairly difficult questions. After you remove the data, how should the data be safely removed from memory? For this question, please see #2711.

Lastly, I wonder if you've considered using delta temporality to work around this problem? The intention behind delta temporality is that any synchronous-instrument-timeseries that you stop using can be erased from memory after it is exported. If you configured your SDK to export to an OTel collector using delta temporality, the collector could be configured to address this problem. The problem still has to be solved (i.e., there's a limit on memory) but at least it's outside your SDK.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.