Skip to content

Commit

Permalink
Fix OTLPMetricExporter ignores preferred_aggregation property (#3603)
Browse files Browse the repository at this point in the history
* fix #3522 OTLPMetricExporter ignores preferred_aggregation

* make OTLPMetricExporter pass the preferred_aggregation argument to the _common_configuration function

* Remove unnecessary import from metrics encoder

* docs: added changelog entry for pr #3603

* Added unit test to make sure preferred_aggregation override works

* chore: break aggregation and temporality config in two functions

* chore: removed instrument_class_aggregation variable external declaration, to avoid possible problems with linters

* chore: removed unnecessary variable declaration

* docs: moved changelog entry to unreleased

* chore: added preferred_aggregation argument to _common_configuration call

* chore: added unit test for grpc otlp metric exporter

* test: moved preferred_aggregation test to class

* chore: fix linter findings on metrics_encoder/__init__.py

* chore: fix linter findings on grpc/metrics_exporter/__init__.py

* chore: fix linter findings on http/metrics_exporter/__init__.py

* fix: removed code duplicate

* chore: fixed linter errors

* Ignoring pylint for protected access

* Fix Aggregation import

* Rename getter methods to private

---------

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
  • Loading branch information
Dudssource and ocelotl committed Feb 14, 2024
1 parent 6e6590c commit 12f4490
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Fix `OTLPMetricExporter` ignores `preferred_aggregation` property
([#3603](https://github.com/open-telemetry/opentelemetry-python/pull/3603))
- Logs: set `observed_timestamp` field
([#3565](https://github.com/open-telemetry/opentelemetry-python/pull/3565))
- Add missing Resource SchemaURL in OTLP exporters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from opentelemetry.sdk.metrics.export import (
MetricExporter,
)
from opentelemetry.sdk.metrics.view import Aggregation
from os import environ
from opentelemetry.sdk.metrics import (
Counter,
Expand Down Expand Up @@ -65,9 +66,18 @@ class OTLPMetricExporterMixin:
def _common_configuration(
self,
preferred_temporality: Dict[type, AggregationTemporality] = None,
preferred_aggregation: Dict[type, Aggregation] = None,
) -> None:

instrument_class_temporality = {}
MetricExporter.__init__(
self,
preferred_temporality=self._get_temporality(preferred_temporality),
preferred_aggregation=self._get_aggregation(preferred_aggregation),
)

def _get_temporality(
self, preferred_temporality: Dict[type, AggregationTemporality]
) -> Dict[type, AggregationTemporality]:

otel_exporter_otlp_metrics_temporality_preference = (
environ.get(
Expand Down Expand Up @@ -119,6 +129,13 @@ def _common_configuration(

instrument_class_temporality.update(preferred_temporality or {})

return instrument_class_temporality

def _get_aggregation(
self,
preferred_aggregation: Dict[type, Aggregation],
) -> Dict[type, Aggregation]:

otel_exporter_otlp_metrics_default_histogram_aggregation = environ.get(
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
"explicit_bucket_histogram",
Expand All @@ -128,7 +145,9 @@ def _common_configuration(
"base2_exponential_bucket_histogram"
):

histogram_aggregation_type = ExponentialBucketHistogramAggregation
instrument_class_aggregation = {
Histogram: ExponentialBucketHistogramAggregation(),
}

else:

Expand All @@ -145,13 +164,13 @@ def _common_configuration(
otel_exporter_otlp_metrics_default_histogram_aggregation,
)

histogram_aggregation_type = ExplicitBucketHistogramAggregation
instrument_class_aggregation = {
Histogram: ExplicitBucketHistogramAggregation(),
}

MetricExporter.__init__(
self,
preferred_temporality=instrument_class_temporality,
preferred_aggregation={Histogram: histogram_aggregation_type()},
)
instrument_class_aggregation.update(preferred_aggregation or {})

return instrument_class_aggregation


def encode_metrics(data: MetricsData) -> ExportMetricsServiceRequest:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ def __init__(
else compression
)

self._common_configuration(preferred_temporality)
self._common_configuration(
preferred_temporality, preferred_aggregation
)

OTLPExporterMixin.__init__(
self,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,24 @@ def test_exponential_explicit_bucket_histogram(self):
ExplicitBucketHistogramAggregation,
)

def test_preferred_aggregation_override(self):

histogram_aggregation = ExplicitBucketHistogramAggregation(
boundaries=[0.05, 0.1, 0.5, 1, 5, 10],
)

exporter = OTLPMetricExporter(
preferred_aggregation={
Histogram: histogram_aggregation,
},
)

self.assertEqual(
# pylint: disable=protected-access
exporter._preferred_aggregation[Histogram],
histogram_aggregation,
)


def _resource_metrics(
index: int, scope_metrics: List[ScopeMetrics]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ def __init__(
{"Content-Encoding": self._compression.value}
)

self._common_configuration(preferred_temporality)
self._common_configuration(
preferred_temporality, preferred_aggregation
)

def _export(self, serialized_data: str):
data = serialized_data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,19 @@ def test_2xx_status_code(self, mock_otlp_metric_exporter):
OTLPMetricExporter().export(MagicMock()),
MetricExportResult.SUCCESS,
)

def test_preferred_aggregation_override(self):

histogram_aggregation = ExplicitBucketHistogramAggregation(
boundaries=[0.05, 0.1, 0.5, 1, 5, 10],
)

exporter = OTLPMetricExporter(
preferred_aggregation={
Histogram: histogram_aggregation,
},
)

self.assertEqual(
exporter._preferred_aggregation[Histogram], histogram_aggregation
)

0 comments on commit 12f4490

Please sign in to comment.