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.

Sign up for GitHub

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

Jump to bottom

Ability to create metrics with different aggregators #989

Closed
legendecas opened this issue Apr 28, 2020 · 2 comments · Fixed by #1100
Closed

Ability to create metrics with different aggregators #989

legendecas opened this issue Apr 28, 2020 · 2 comments · Fixed by #1100
Assignees
@vmarchaud
Labels
feature-request
Projects
Metrics API and SDK

Comments

@legendecas
Copy link
Member

Is your feature request related to a problem? Please describe.
In the current API shape, it is not able to create a metric and set an expected aggregator. Like say, we have two observer metrics, one we'd like to aggregate with HistogramAggregator, and another with DistributionAggregator.

Describe the solution you'd like
Document the batcher API, or allow meters to accept an option of aggregation type on creating metrics.
@dyladan dyladan added the up-for-grabs Good for taking. Extra help will be provided by maintainers label Apr 28, 2020
@dyladan dyladan added this to To do in Metrics API and SDK via automation Apr 29, 2020
@dyladan dyladan moved this from Triage to In progress in Metrics API and SDK Apr 29, 2020
@dyladan dyladan removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Apr 29, 2020
@vmarchaud
Copy link
Member

Do we want to put those kind of documentation inside examples or doc ?

@legendecas
Copy link
Member Author

legendecas commented May 2, 2020 •
edited

@vmarchaud example is cool but doc can still be useful.

Another point is that I checked otel impl in go, Batcher in go can determine aggregator by MetricDescriptor but not MetricKind. I'm wondering if it's ok to port this API to js before we document the batcher API, since batcher (or maybe integrator) is not been in spec yet.

vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue May 3, 2020
@vmarchaud
feat(metrics): use MetricDescriptor to determine aggregator open-tele…
758d55f 
…metry#989
@vmarchaud vmarchaud mentioned this issue May 3, 2020
Merged
mayurkale22 pushed a commit that referenced this issue May 4, 2020
@vmarchaud
feat(metrics): use MetricDescriptor to determine aggregator #989 (#1014)
16ae2a7
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue May 23, 2020
@vmarchaud
docs(batcher): document how to configure custom aggregators open-tele…
75f5fbb 
…metry#989
@vmarchaud vmarchaud mentioned this issue May 23, 2020
Merged
vmarchaud added a commit to vmarchaud/opentelemetry-js that referenced this issue May 28, 2020
@vmarchaud
docs(batcher): document how to configure custom aggregators open-tele…
f03b9f8 
…metry#989
Metrics API and SDK automation moved this from In progress to Done May 28, 2020
mayurkale22 added a commit that referenced this issue May 28, 2020
@vmarchaud @mayurkale22
docs(batcher): document how to configure custom aggregators #989 (#1100)
2987534 
* docs(batcher): document how to configure custom aggregators #989

* chore: address PR comments

Co-authored-by: Mayur Kale <mayurkale@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vmarchaud vmarchaud

Labels
feature-request
Projects
No open projects
Metrics API and SDK
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs(batcher): document how to configure custom aggregators #989 vmarchaud/opentelemetry-js
3 participants
@dyladan @vmarchaud @legendecas