Skip to content

Commit

Permalink
Revert "Select histogram aggregation with environment variables"
Browse files Browse the repository at this point in the history
This reverts commit fa1a167.
  • Loading branch information
ocelotl committed Apr 20, 2023
1 parent fa1a167 commit 17de6c9
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 95 deletions.
2 changes: 0 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- Enable selection of histogram aggregation via environment variable
([#3265](https://github.com/open-telemetry/opentelemetry-python/pull/3265))
- Move Protobuf encoding to its own package
([#3169](https://github.com/open-telemetry/opentelemetry-python/pull/3169))
- Add experimental feature to detect resource detectors in auto instrumentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@
The :envvar:`OTEL_METRICS_EXEMPLAR_FILTER` is the filter for which measurements can become Exemplars.
"""

OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION = (
_OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION = (
"OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION"
)
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from enum import IntEnum
from logging import getLogger
from math import inf
from os import environ
from threading import Lock
from typing import Generic, List, Optional, Sequence, TypeVar

Expand All @@ -34,9 +33,6 @@
Synchronous,
UpDownCounter,
)
from opentelemetry.sdk.environment_variables import (
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
)
from opentelemetry.sdk.metrics._internal.exponential_histogram.buckets import (
Buckets,
)
Expand Down Expand Up @@ -931,29 +927,6 @@ def _create_aggregation(
)

if isinstance(instrument, Histogram):

histogram_type = environ.get(
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
"explicit_bucket_histogram",
)

if histogram_type == "base2_exponential_bucket_histogram":

return _ExponentialBucketHistogramAggregation(
attributes, start_time_unix_nano
)

if histogram_type != "explicit_bucket_histogram":

_logger.warning(
(
"Invalid value for %s: %s, using explicit bucket "
"histogram aggregation"
),
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
histogram_type,
)

return _ExplicitBucketHistogramAggregation(
attributes, start_time_unix_nano
)
Expand Down
66 changes: 1 addition & 65 deletions opentelemetry-sdk/tests/metrics/test_aggregation.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from logging import WARNING
from math import inf
from os import environ
from time import sleep
from typing import Union
from unittest import TestCase
from unittest.mock import Mock, patch
from unittest.mock import Mock

from opentelemetry.metrics import Histogram
from opentelemetry.sdk.environment_variables import (
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
)
from opentelemetry.sdk.metrics._internal.aggregation import (
_ExplicitBucketHistogramAggregation,
_ExponentialBucketHistogramAggregation,
_LastValueAggregation,
_SumAggregation,
)
Expand Down Expand Up @@ -548,60 +541,3 @@ def test_observable_gauge(self):
0,
)
self.assertIsInstance(aggregation, _LastValueAggregation)

def test_exponential_explicit_bucket_histogram(self):

histogram = Mock(spec=Histogram)
default_aggregation = DefaultAggregation()

self.assertIsInstance(
default_aggregation._create_aggregation(histogram, Mock(), Mock()),
_ExplicitBucketHistogramAggregation,
)

with patch.dict(
environ,
{
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "base2_exponential_bucket_histogram"
},
):
self.assertIsInstance(
default_aggregation._create_aggregation(
histogram, Mock(), Mock()
),
_ExponentialBucketHistogramAggregation,
)

with patch.dict(
environ,
{OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "abc"},
):
with self.assertLogs(level=WARNING) as log:
self.assertIsInstance(
default_aggregation._create_aggregation(
histogram, Mock(), Mock()
),
_ExplicitBucketHistogramAggregation,
)
self.assertEqual(
log.output[0],
(
"WARNING:opentelemetry.sdk.metrics._internal.aggregation:"
"Invalid value for OTEL_EXPORTER_OTLP_METRICS_DEFAULT_"
"HISTOGRAM_AGGREGATION: abc, using explicit bucket "
"histogram aggregation"
),
)

with patch.dict(
environ,
{
OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "explicit_bucket_histogram"
},
):
self.assertIsInstance(
default_aggregation._create_aggregation(
histogram, Mock(), Mock()
),
_ExplicitBucketHistogramAggregation,
)

0 comments on commit 17de6c9

Please sign in to comment.