Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify how should SDK invokes async instrument callbacks #2408

Closed
Tracked by #2822
legendecas opened this issue Mar 8, 2022 · 1 comment · Fixed by #2363 or #2538
Closed
Tracked by #2822

Clarify how should SDK invokes async instrument callbacks #2408

legendecas opened this issue Mar 8, 2022 · 1 comment · Fixed by #2363 or #2538
Assignees
Labels
area:sdk Related to the SDK release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory

Comments

@legendecas
Copy link
Member

legendecas commented Mar 8, 2022

What are you trying to achieve?

In the change of #2361, it defines that:

Callback functions MUST be documented as follows for the end user:

  • Callback functions SHOULD be reentrant safe. The SDK expects to evaluate callbacks for each MetricReader independently.

It sounds like the SDK can invoke the callback functions each time a MetricReader initiates collection. While implementing the behavior, I found that when the async instrument is configured with multiple views with different names, a created async instrument may be mapped into multiple underlying, exported async instruments. The problems are:

  1. Should the SDK call the callback each time a MetricReader initiates collection, or one concurrent invocation for each MetricReader independently?

Like, we have two readers, should the SDK call the callback when reader A initiates a second-time collection when the first collection is not timed out yet?

  1. Should the SDK call the callback once for an end-user registered async instrument, or for a view-ed async instrument?
@legendecas legendecas added the spec:metrics Related to the specification/metrics directory label Mar 8, 2022
@reyang reyang added area:sdk Related to the SDK release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs labels Mar 11, 2022
@jmacd
Copy link
Contributor

jmacd commented Mar 11, 2022

The specification is missing something from the recent revisions. #2317 should have added one more statement, more-or-less implied I think, in sdk.md#observations-inside-asynchronous-callbacks about executing callbacks exactly once per collection.

The pending PR #2363 includes the statement I had intended in an explicit way, since the option to support multiple callbacks requires more precision. See https://github.com/open-telemetry/opentelemetry-specification/pull/2363/files#diff-055661289c337a191d4767d311dbdf1fe63e0aa07f9d58944da92c54f4382df1R359. The specification there is intended to give an SDK of the future freedom to collect instruments with different intervals (i.e., "exactly once" applies to callbacks, but only to callbacks associated with desired instruments).

Please use the interpretation in that PR. Every callback is evaluated once during collection. If a value from an asynchronous instrument is used in multiple views, they should use identical values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:metrics Related to the specification/metrics directory
Projects
None yet
3 participants