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

Adding a View seems very strange, not clear ownership #3210

Closed
bogdandrutu opened this issue Sep 21, 2022 · 2 comments · Fixed by #3387
Closed

Adding a View seems very strange, not clear ownership #3210

bogdandrutu opened this issue Sep 21, 2022 · 2 comments · Fixed by #3387
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request pkg:SDK Related to an SDK package

Comments

@bogdandrutu
Copy link
Member

Right now you can configure Aggregation into few places:

  • Default aggregation in the Reader.
  • Views when configured the Reader into the MeterProvider

An idea would be to make Reader owner of all the "aggregation selection" and register Views into the Reader. That way the we have a clear owner for this.

This is food for thought, but when trying to use the API I got confused about the two owners of aggregation selection.

@bogdandrutu bogdandrutu added the enhancement New feature or request label Sep 21, 2022
@MrAlias MrAlias added this to the Metric SDK: Beta milestone Sep 21, 2022
@MrAlias MrAlias added pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Sep 21, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Sep 21, 2022

I think this is a good idea, have the reader construction accept the views instead of the meter provider option. It would indeed be less confusing to the users.

We will need to check this confirms with the specification. I seem to remember that choice coming from the specification. But I'd need to verify.

@jmacd
Copy link
Contributor

jmacd commented Sep 22, 2022

This design may have been inadvertently influenced by my prototype. See also open-telemetry/opentelemetry-specification#2288

It makes sense to me to let each reader have their own view of the metrics, but actually the current specification says the view configuration is SDK-wide (which 🤷 to me defeats a lot of the value of the mechanism--I would recommend different views for pull and push-based configurations, for example).

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 enhancement New feature or request pkg:SDK Related to an SDK package
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants