Clarification needed: handling repeated async counter observations

[dashpole]

What are you trying to achieve?

I'd like to clarify the behavior when multiple observations are made during a single collection round using an asynchronous counter or asynchronous updown counter (not a gauge).

Question: If I observe 10 and then 30 with the same label set and within a single callback (no filtering), are the observations summed, or is the last value used?

obserer.Observe(10, {"foo", "bar"})
obserer.Observe(30, {"foo", "bar"})
// is the value now 30, or 40?

We've established in #2905 (comment), and described in the supplementary guidelines that async counter observations with the same label set are summed when a filter is applied using a view. This means that the example above, when the foo label is filtered out, will produce the value 40.

Fundamentally I want a filtered-out attribute to be the same as an attribute that never existed. Thus, I think multiple observations with the same label set within a single collection round should be summed, even if no filter was applied, in order to be consistent with the current supplementary guidelines. I would propose a supplementary guideline to clarify this.

Additional context.

Motivating PR + Discussion: open-telemetry/opentelemetry-go#3549 (comment)

Prior issues touching the topic:

• Verify compliant metric API specification implementation: Instrument Asynchronous UpDownCounter opentelemetry-go#3379 (comment)
• Clarification needed on how values for attributes filtered by a view are used #2905
• AsyncCounter and AsyncUpDownCounter expected callback value changed in v0.32.0 opentelemetry-go#3278

Supplementary guidelines related to the topic: https://github.com/open-telemetry/opentelemetry-specification/blob/25f75bfde209630c60728039f27c041f70e5e738/specification/metrics/supplementary-guidelines.md#asynchronous-example-attribute-removal-in-a-view

cc @reyang @jmacd @MrAlias

[MrAlias]

To add to the description, this is specifically for asynchronous counters and up-down counters (not gauges).

[tsloughter]

I'd expect only 1 to be used, and not summed, since returning the value instead of doing a method like observer.Observe is described as a viable option for implementations.

[reyang]

I'd expect only 1 to be used, and not summed, since returning the value instead of doing a method like observer.Observe is described as a viable option for implementations.

+1

The spec doesn't explicitly talk about it because the observer.Observe API might be designed in many ways. I personally feel it's okay to leave this unspecified.

[jack-berg]

If I observe 10 and then 30 with the same label set and within a single callback (no filtering), are the observations summed, or is the last value used?

This usage violates the API spec, which states:

Callback functions SHOULD NOT make duplicate observations (more than one Measurement with the same attributes) across all registered callbacks.

The SDK shouldn't jump through hoops to restore consistency when the API is used improperly. I agree with @tsloughter and @reyang that only 1 of the measurements should be used, and that its ok to leave it unspecified which is used (i.e. no need to state that the last observed wins).

[MrAlias]

This usage violates the API spec, which states:

Callback functions SHOULD NOT make duplicate observations (more than one Measurement with the same attributes) across all registered callbacks.

This relates to #3071. It is problematic to make a recommendation for the user of the API instead of making a recommendation for the API itself. The specification is not meant to be read by API users and it ultimately does not govern their behavior. We need to update the specification to state that an API is documented that callback functions should not make duplicate observations. That way we inform users where they will actually meet the API.

[MrAlias]

The SDK shouldn't jump through hoops to restore consistency when the API is used improperly. I agree with @tsloughter and @reyang that only 1 of the measurements should be used, and that its ok to leave it unspecified which is used (i.e. no need to state that the last observed wins).

I agree the SDK shouldn't do anything complex to try to handle this, but I do think the language implementations should be consistent. A bug quickly becomes a feature in this case and users may become frustrated if in one language they get a sum of the values, another the first one wins, another the last wins, and another it drops the whole measurement.

[dashpole]

I personally feel it's okay to leave this unspecified.

I think that is also a reasonable outcome. I also found https://github.com/open-telemetry/opentelemetry-specification/blob/25f75bfde209630c60728039f27c041f70e5e738/specification/metrics/api.md#asynchronous-counter-creation in the API spec, which very clearly leaves it up to SDK implementations:

if two [identical] measurements are reported, OpenTelemetry SDK authors MAY decide to simply let them pass through (so the downstream consumer can handle duplication), drop the entire data, pick the last one, or something else.