open-telemetry / opentelemetry-cpp

The OpenTelemetry C++ Client
https://opentelemetry.io/
Apache License 2.0
888 stars 421 forks source link

[Metrics SDK] Add support for Exponential Histogram Aggregation #1391

Open lalitb opened 2 years ago

lalitb commented 2 years ago

Currently, metrics SDK implementation support following aggregation: https://github.com/open-telemetry/opentelemetry-cpp/blob/88e23ad469c6439226d929c6f949f4af48e1135c/sdk/include/opentelemetry/sdk/metrics/instruments.h#L30-L37

The default Histogram aggregation is implemented as described in the data model: https://github.com/open-telemetry/opentelemetry-specification/blob/90e5022a846a65600b7ad7e8d262678a83f76191/specification/metrics/datamodel.md#histogram

The data model also adds (experimental) support for Exponential histogram aggregation - https://github.com/open-telemetry/opentelemetry-specification/blob/90e5022a846a65600b7ad7e8d262678a83f76191/specification/metrics/datamodel.md#exponentialhistogram

This support needs to be implemented in the SDK.

lalitb commented 2 years ago

Moving it out of RC, as the current specs is still not stable, and the complexities involved in implementing this.

github-actions[bot] commented 1 year ago

This issue was marked as stale due to lack of activity.

euroelessar commented 1 year ago

@lalitb Are you open to accept changes to implement exponential histogram, given that it's now a stable feature? https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/data-model.md#exponentialhistogram

lalitb commented 1 year ago

@euroelessar Yes, the contributions are welcome. Thanks for volunteering. Let me know if you want me to assign it to you ?

euroelessar commented 1 year ago

@lalitb Yeah. I don't have a lot of throughput in the next month or so, but should be able to slowly produce some changes. I have started with an implementation of an underlying adapting circular buffer (as suggested by the spec).

lalitb commented 1 year ago

@euroelessar - do you plan to incorporate the remaining changes for this feature? It's not urgent, but but just in case someone want to pick up the further changes :)

euroelessar commented 1 year ago

@lalitb yep! I need to update tests, implementation itself is overall ready, should be able to send it within a week or so

lalitb commented 1 year ago

@euroelessar Let us know if you need any help on getting this ready? Alternatively, if someone else could take it from here, since you've already set up the foundational data structures. :)

ThomsonTan commented 1 year ago

@euroelessar is there any update on this?

luan commented 1 year ago

This would be super cool if finished, @euroelessar are you still working on it?