Skip to content

Commit 6a8b8a7

Browse files
committed
Keep using _MetricsHistogramAdvisory for storing advisory internally
1 parent 82536bc commit 6a8b8a7

File tree

6 files changed

+20
-18
lines changed

6 files changed

+20
-18
lines changed

opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
from logging import getLogger
4747
from os import environ
4848
from threading import Lock
49-
from typing import Any, Dict, List, Optional, Sequence, Union, cast
49+
from typing import Dict, List, Optional, Sequence, Union, cast
5050

5151
from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER
5252
from opentelemetry.metrics._internal.instrument import (
@@ -65,6 +65,7 @@
6565
ObservableGauge,
6666
ObservableUpDownCounter,
6767
UpDownCounter,
68+
_MetricsHistogramAdvisory,
6869
_ProxyCounter,
6970
_ProxyGauge,
7071
_ProxyHistogram,
@@ -185,7 +186,7 @@ class _InstrumentRegistrationStatus:
185186
instrument_id: str
186187
already_registered: bool
187188
conflict: bool
188-
current_advisory: Optional[Any]
189+
current_advisory: Optional[_MetricsHistogramAdvisory]
189190

190191

191192
class Meter(ABC):
@@ -205,7 +206,9 @@ def __init__(
205206
self._name = name
206207
self._version = version
207208
self._schema_url = schema_url
208-
self._instrument_ids: Dict[str, Optional[Any]] = {}
209+
self._instrument_ids: Dict[
210+
str, Optional[_MetricsHistogramAdvisory]
211+
] = {}
209212
self._instrument_ids_lock = Lock()
210213

211214
@property
@@ -235,7 +238,7 @@ def _register_instrument(
235238
type_: type,
236239
unit: str,
237240
description: str,
238-
advisory: Optional[Any] = None,
241+
advisory: Optional[_MetricsHistogramAdvisory] = None,
239242
) -> _InstrumentRegistrationStatus:
240243
"""
241244
Register an instrument with the name, type, unit and description as
@@ -743,15 +746,14 @@ def create_histogram(
743746
explicit_bucket_boundaries_advisory: Optional[Sequence[float]] = None,
744747
) -> Histogram:
745748
"""Returns a no-op Histogram."""
746-
# FIXME: should be use a dataclass here instead for the advisory param?
747749
status = self._register_instrument(
748750
name,
749751
NoOpHistogram,
750752
unit,
751753
description,
752-
{
753-
"explicit_bucket_boundaries_advisory": explicit_bucket_boundaries_advisory
754-
},
754+
_MetricsHistogramAdvisory(
755+
explicit_bucket_boundaries=explicit_bucket_boundaries_advisory
756+
),
755757
)
756758
if status.conflict:
757759
self._log_instrument_registration_conflict(

opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,8 @@
4545
_unit_regex = re_compile(r"[\x00-\x7F]{0,63}")
4646

4747

48-
# FIXME: remove this if we are not going to use it to store advisory in registered instruments
4948
@dataclass(frozen=True)
50-
class MetricsHistogramAdvisory:
49+
class _MetricsHistogramAdvisory:
5150
explicit_bucket_boundaries: Optional[Sequence[float]] = None
5251

5352

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,6 @@ def create_histogram(
201201
description: str = "",
202202
explicit_bucket_boundaries_advisory: Optional[Sequence[float]] = None,
203203
) -> APIHistogram:
204-
# FIXME: should pack everything into the class instead?
205204
if explicit_bucket_boundaries_advisory is not None:
206205
invalid_advisory = False
207206
try:

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,8 +1284,8 @@ def _create_aggregation(
12841284

12851285
if isinstance(instrument, Histogram):
12861286
boundaries: Optional[Sequence[float]] = (
1287-
instrument._explicit_bucket_boundaries_advisory
1288-
if instrument._explicit_bucket_boundaries_advisory is not None
1287+
instrument._advisory.explicit_bucket_boundaries
1288+
if instrument._advisory.explicit_bucket_boundaries is not None
12891289
else None
12901290
)
12911291
return _ExplicitBucketHistogramAggregation(

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@
3131
)
3232
from opentelemetry.metrics import UpDownCounter as APIUpDownCounter
3333
from opentelemetry.metrics import _Gauge as APIGauge
34-
from opentelemetry.metrics._internal.instrument import CallbackOptions
34+
from opentelemetry.metrics._internal.instrument import (
35+
CallbackOptions,
36+
_MetricsHistogramAdvisory,
37+
)
3538
from opentelemetry.sdk.metrics._internal.measurement import Measurement
3639
from opentelemetry.sdk.util.instrumentation import InstrumentationScope
3740

@@ -235,9 +238,8 @@ def __init__(
235238
instrumentation_scope=instrumentation_scope,
236239
measurement_consumer=measurement_consumer,
237240
)
238-
# FIXME: should the dataclass instead?
239-
self._explicit_bucket_boundaries_advisory = (
240-
explicit_bucket_boundaries_advisory
241+
self._advisory = _MetricsHistogramAdvisory(
242+
explicit_bucket_boundaries=explicit_bucket_boundaries_advisory
241243
)
242244

243245
def __new__(cls, *args, **kwargs):

opentelemetry-sdk/tests/metrics/test_metrics.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ def test_create_histogram_with_advisory(self):
525525
self.assertIsInstance(histogram, Histogram)
526526
self.assertEqual(histogram.name, "name")
527527
self.assertEqual(
528-
histogram._explicit_bucket_boundaries_advisory,
528+
histogram._advisory.explicit_bucket_boundaries,
529529
[0.0, 1.0, 2],
530530
)
531531

0 commit comments

Comments
 (0)