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

Implement aggregators for metrics #413

Closed
lzchen opened this issue Feb 13, 2020 · 6 comments
Closed

Implement aggregators for metrics #413

lzchen opened this issue Feb 13, 2020 · 6 comments
Labels

Comments

@lzchen
Copy link
Contributor

lzchen commented Feb 13, 2020

Currently, only the aggregator for counter metrics has been implemented.
Implement the corresponding aggregators for the rest of the metrics types.

https://github.com/open-telemetry/opentelemetry-specification/blob/2b75442b05fd968f197422dc18124338a955f3a2/specification/sdk-metric.md#aggregator-implementations

@mauriciovasquezbernal
Copy link
Member

I think we need to better define the scope of this issue. The link on the description is a document that is not part of the specification yet, so I don't know if we should base the decision of what aggregators to implement to that. Currently we have Counter, MinMaxSumCount and MinMaxSumCount (observer) aggregators. Which ones are we missing for the beta release?

@MikeGoldsmith
Copy link
Member

Observer should have a "last value" aggregator.
OTEP-80
OTEL-Spec-412.

@mauriciovasquezbernal
Copy link
Member

@MikeGoldsmith I made a typo, currently the Observer has a MinMaxSumCountLast aggregator. I remember there were some discussion about simplifying it, do you have any info of the final decision on that discussion?

@MikeGoldsmith
Copy link
Member

@mauriciovasquezbernal - Metrics Spec just says LastValue aggregation 👍

@krnowak
Copy link
Member

krnowak commented Mar 4, 2020

In golang, the simple selector for observer basically chooses an "measure"-like aggregator. See open-telemetry/opentelemetry-go#474 (comment)

So yeah, that aggregations part in metrics spec seems a bit conflicting with standard impls of instruments in metrics specs.

@lzchen
Copy link
Contributor Author

lzchen commented Mar 6, 2020

@toumorokoshi @c24t
I think this issue can be closed. We have the default aggregators for each metric type and I believe that is sufficient enough for the beta release.

@c24t c24t closed this as completed Mar 9, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* fix(http-plugin): move node-sdk to dev deps

* refactor: rename opentelemetry-sdk-base

* refactor: rename opentelemetry-tracer-basic

* refactor: rename opentelemetry-node-sdk

* docs: update readme

* docs: update readme

* refactor: rename opentelemetry-tracer-web

* fix: styling

* fix: doc consistency

* fix: sliiped in typo

* fix: postgres and doc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants