open-telemetry / opentelemetry-python

OpenTelemetry Python API and SDK
https://opentelemetry.io
Apache License 2.0
1.8k stars 626 forks source link

Add ExponentialHistogram data point to metrics SDK #2421

Closed codeboten closed 1 year ago

codeboten commented 2 years ago

The spec says there are 4 data points:

  1. Sum
  2. Gauge
  3. Histogram
  4. Exponential Histogram

Currently the SDK only implements 3 of them: Sum, Gauge, Histogram. See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/datamodel.md#opentelemetry-protocol-data-model

aabmass commented 2 years ago

You probably already know this Alex, but ExponentialHistogram aggregation is not in the Metrics SDK spec yet (https://github.com/open-telemetry/opentelemetry-specification/pull/2252). I do think it's worth adding it to our export types now to get the OTLP exporter ready in anticipation 🙂

clesleycode commented 1 year ago

These are the major changes I'm thinking we'd need to make?:

  1. Implement an ExponentialHistogramAggregation class in aggregation.py.
  2. Add an ExponentialHistogram] data class to point.py
  3. Add support for that class in export/__init__.py.
  4. Modify any modules to make sure they support the new metric type.

I'm sure I'm missing small things here and there, but wanted to see if I'm on the right track before I go any further down a rabbit hole :)

srikanthccv commented 1 year ago

@clesleycode The majority of work on this is done by @ocelotl. The last remaining part is implemented here https://github.com/open-telemetry/opentelemetry-python/pull/2964.

clesleycode commented 1 year ago

@srikanthccv should we close this then?

srikanthccv commented 1 year ago

No, there is one more task pending. We should close this when that gets merged.

ocelotl commented 1 year ago

2964 is merged, closing.