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

Add details to Asynchronous Gauge API #1703

Merged
merged 6 commits into from
May 20, 2021

Conversation

reyang
Copy link
Member

@reyang reyang commented May 14, 2021

Follow up #1617.

Changes

Added details to the Asynchronous Gauge instrument. In #1617 these were left as TODO.

@reyang reyang requested review from a team May 14, 2021 17:59
reyang and others added 2 commits May 14, 2021 11:06
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
@reyang reyang added area:api Cross language API specification issue spec:metrics Related to the specification/metrics directory labels May 15, 2021
observed. Individual language client SHOULD define whether this callback
function needs to be reentrant safe / thread safe or not.

The callback function SHOULD NOT take indefinite amount of time. If multiple
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any rules about how many times per-exporter this is called?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be called once per export cycle. That is my understanding.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't put restriction in the API spec. The SDK spec might put additional requirements/explanation.
I would say by default it follows what @bogdandrutu mentioned.
In some situation, the user might decide to collect the data at a higher frequency but report it on lower frequency (e.g. configure the SDK to gather the data from callbacks every 5 seconds, but only export the data every one minute - refer to #1432).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even if the user wants to do that, they need to schedule their own timer then. The valid SDK implementation should call the callback once every export cycle I think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call it "collection" cycle for the moment. Until we define if collection cycle == exporter cycle.

specification/metrics/api.md Outdated Show resolved Hide resolved
reyang and others added 2 commits May 19, 2021 10:16
Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
@SergeyKanzhelev
Copy link
Member

@jsuereth please comment if you still has concerns. Otherwise I will merge in a few hours

@SergeyKanzhelev
Copy link
Member

@jsuereth if there are outstanding comments, let's resolve them as issues

@SergeyKanzhelev SergeyKanzhelev merged commit ea96c08 into open-telemetry:main May 20, 2021
@reyang reyang deleted the reyang/async-gauge branch May 20, 2021 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants