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

Batch Observer callback support #717

Merged
merged 13 commits into from May 13, 2020
Merged

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented May 12, 2020

Implements the design discussed in #634 (comment) for batch observer callbacks.

Resolves #633.

See the spec-level issue:
open-telemetry/opentelemetry-specification#549

@jmacd
Copy link
Contributor Author

jmacd commented May 13, 2020

@marwan-at-work Will you take a look?

@jmacd
Copy link
Contributor Author

jmacd commented May 13, 2020

Here are the other possible APIs discussed in #634:

#634 (comment)
#634 (comment)
#634 (comment)
#634 (comment)
#634 (comment)

and lastly this, which was used for this PR:
#634 (comment)

@jmacd jmacd requested a review from evantorrie as a code owner May 13, 2020 19:23
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

Looks good on the overall approach. 👍

Minor requests and questions.

sdk/metric/sdk.go Outdated Show resolved Hide resolved
internal/metric/async.go Show resolved Hide resolved
api/metric/observer.go Outdated Show resolved Hide resolved
api/metric/observer.go Show resolved Hide resolved
api/metric/observer.go Outdated Show resolved Hide resolved
api/metric/observer.go Outdated Show resolved Hide resolved
api/metric/observer.go Outdated Show resolved Hide resolved
internal/metric/async.go Outdated Show resolved Hide resolved
internal/metric/async.go Outdated Show resolved Hide resolved
sdk/metric/sdk.go Outdated Show resolved Hide resolved
@jmacd jmacd mentioned this pull request May 13, 2020
go.sum Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor Author

jmacd commented May 13, 2020

Thank you!

@jmacd jmacd merged commit fefdf59 into open-telemetry:master May 13, 2020
@jmacd jmacd deleted the jmacd/batchobs3 branch May 13, 2020 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metric Observer API changes for batch reporting
3 participants