Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove monotonic and absolute metrics intrument options #410

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,7 @@ def set(self, value: ValueT, label_set: LabelSet) -> None:
class Measure(Metric):
"""A measure type metric that represent raw stats that are recorded.

Measure metrics represent raw statistics that are recorded. By
default, measure metrics can accept both positive and negatives.
Negative inputs will be discarded when monotonic is True.
Measure metrics represent raw statistics that are recorded.
"""

def get_handle(self, label_set: LabelSet) -> "MeasureHandle":
Expand Down Expand Up @@ -268,8 +266,6 @@ def create_metric(
metric_type: Type[MetricT],
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
absolute: bool = True,
) -> "Metric":
"""Creates a ``metric_kind`` metric with type ``value_type``.

Expand All @@ -281,10 +277,6 @@ def create_metric(
metric_type: The type of metric being created.
label_keys: The keys for the labels with dynamic values.
enabled: Whether to report the metric by default.
monotonic: Configure a counter or gauge that accepts only
monotonic/non-monotonic updates.
absolute: Configure a measure that does or does not accept negative
updates.
Returns: A new ``metric_type`` metric with values of ``value_type``.
"""

Expand Down Expand Up @@ -318,8 +310,6 @@ def create_metric(
metric_type: Type[MetricT],
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
absolute: bool = True,
) -> "Metric":
# pylint: disable=no-self-use
return DefaultMetric()
Expand Down
41 changes: 0 additions & 41 deletions opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ class BaseHandle:
Args:
value_type: The type of values this handle holds (int, float).
enabled: True if the originating instrument is enabled.
monotonic: Indicates acceptance of only monotonic/non-monotonic values
for updating counter and gauge handles.
absolute: Indicates acceptance of negative updates to measure handles.
aggregator: The aggregator for this handle. Will handle aggregation
upon updates and checkpointing of values for exporting.
"""
Expand All @@ -66,14 +63,10 @@ def __init__(
self,
value_type: Type[metrics_api.ValueT],
enabled: bool,
monotonic: bool,
absolute: bool,
aggregator: Aggregator,
):
self.value_type = value_type
self.enabled = enabled
self.monotonic = monotonic
self.absolute = absolute
self.aggregator = aggregator
self.last_update_timestamp = time_ns()

Expand Down Expand Up @@ -103,29 +96,20 @@ class CounterHandle(metrics_api.CounterHandle, BaseHandle):
def add(self, value: metrics_api.ValueT) -> None:
"""See `opentelemetry.metrics.CounterHandle.add`."""
if self._validate_update(value):
if self.monotonic and value < 0:
logger.warning("Monotonic counter cannot descend.")
return
self.update(value)


class GaugeHandle(metrics_api.GaugeHandle, BaseHandle):
def set(self, value: metrics_api.ValueT) -> None:
"""See `opentelemetry.metrics.GaugeHandle.set`."""
if self._validate_update(value):
if self.monotonic and value < self.aggregator.current:
logger.warning("Monotonic gauge cannot descend.")
return
self.update(value)


class MeasureHandle(metrics_api.MeasureHandle, BaseHandle):
def record(self, value: metrics_api.ValueT) -> None:
"""See `opentelemetry.metrics.MeasureHandle.record`."""
if self._validate_update(value):
if self.absolute and value < 0:
logger.warning("Absolute measure cannot accept negatives.")
return
self.update(value)


Expand All @@ -149,8 +133,6 @@ def __init__(
meter: "Meter",
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
absolute: bool = True,
):
self.name = name
self.description = description
Expand All @@ -159,8 +141,6 @@ def __init__(
self.meter = meter
self.label_keys = label_keys
self.enabled = enabled
self.monotonic = monotonic
self.absolute = absolute
self.handles = {}

def get_handle(self, label_set: LabelSet) -> BaseHandle:
Expand All @@ -170,8 +150,6 @@ def get_handle(self, label_set: LabelSet) -> BaseHandle:
handle = self.HANDLE_TYPE(
self.value_type,
self.enabled,
self.monotonic,
self.absolute,
# Aggregator will be created based off type of metric
self.meter.batcher.aggregator_for(self.__class__),
)
Expand All @@ -188,10 +166,6 @@ def __repr__(self):

class Counter(Metric, metrics_api.Counter):
"""See `opentelemetry.metrics.Counter`.

By default, counter values can only go up (monotonic). Negative inputs
will be rejected for monotonic counter metrics. Counter metrics that have a
monotonic option set to False allows negative inputs.
"""

HANDLE_TYPE = CounterHandle
Expand All @@ -205,8 +179,6 @@ def __init__(
meter: "Meter",
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = True,
absolute: bool = False,
):
super().__init__(
name,
Expand All @@ -216,8 +188,6 @@ def __init__(
meter,
label_keys=label_keys,
enabled=enabled,
monotonic=monotonic,
absolute=absolute,
)

def add(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:
Expand All @@ -229,9 +199,6 @@ def add(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:

class Gauge(Metric, metrics_api.Gauge):
"""See `opentelemetry.metrics.Gauge`.

By default, gauge values can go both up and down (non-monotonic).
Negative inputs will be rejected for monotonic gauge metrics.
"""

HANDLE_TYPE = GaugeHandle
Expand All @@ -245,8 +212,6 @@ def __init__(
meter: "Meter",
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
absolute: bool = False,
):
super().__init__(
name,
Expand All @@ -256,8 +221,6 @@ def __init__(
meter,
label_keys=label_keys,
enabled=enabled,
monotonic=monotonic,
absolute=absolute,
)

def set(self, value: metrics_api.ValueT, label_set: LabelSet) -> None:
Expand Down Expand Up @@ -339,8 +302,6 @@ def create_metric(
metric_type: Type[metrics_api.MetricT],
label_keys: Sequence[str] = (),
enabled: bool = True,
monotonic: bool = False,
absolute: bool = True,
) -> metrics_api.MetricT:
"""See `opentelemetry.metrics.Meter.create_metric`."""
# Ignore type b/c of mypy bug in addition to missing annotations
Expand All @@ -352,8 +313,6 @@ def create_metric(
self,
label_keys=label_keys,
enabled=enabled,
monotonic=monotonic,
absolute=absolute,
)
self.metrics.add(metric)
return metric
Expand Down
48 changes: 12 additions & 36 deletions opentelemetry-sdk/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,36 +203,28 @@ def test_record(self):
class TestCounterHandle(unittest.TestCase):
def test_add(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.CounterHandle(int, True, False, False, aggregator)
handle = metrics.CounterHandle(int, True, aggregator)
handle.add(3)
self.assertEqual(handle.aggregator.current, 3)

def test_add_disabled(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.CounterHandle(int, False, False, False, aggregator)
handle = metrics.CounterHandle(int, False, aggregator)
handle.add(3)
self.assertEqual(handle.aggregator.current, 0)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_add_monotonic(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.CounterHandle(int, True, True, False, aggregator)
handle.add(-3)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_add_incorrect_type(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.CounterHandle(int, True, False, False, aggregator)
handle = metrics.CounterHandle(int, True, aggregator)
handle.add(3.0)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.time_ns")
def test_update(self, time_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.CounterHandle(int, True, False, False, aggregator)
handle = metrics.CounterHandle(int, True, aggregator)
time_mock.return_value = 123
handle.update(4.0)
self.assertEqual(handle.last_update_timestamp, 123)
Expand All @@ -243,36 +235,28 @@ def test_update(self, time_mock):
class TestGaugeHandle(unittest.TestCase):
def test_set(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.GaugeHandle(int, True, False, False, aggregator)
handle = metrics.GaugeHandle(int, True, aggregator)
handle.set(3)
self.assertEqual(handle.aggregator.current, 3)

def test_set_disabled(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.GaugeHandle(int, False, False, False, aggregator)
handle = metrics.GaugeHandle(int, False, aggregator)
handle.set(3)
self.assertEqual(handle.aggregator.current, 0)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_set_monotonic(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.GaugeHandle(int, True, True, False, aggregator)
handle.set(-3)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_set_incorrect_type(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.GaugeHandle(int, True, False, False, aggregator)
handle = metrics.GaugeHandle(int, True, aggregator)
handle.set(3.0)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.time_ns")
def test_update(self, time_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.GaugeHandle(int, True, False, False, aggregator)
handle = metrics.GaugeHandle(int, True, aggregator)
time_mock.return_value = 123
handle.update(4.0)
self.assertEqual(handle.last_update_timestamp, 123)
Expand All @@ -283,36 +267,28 @@ def test_update(self, time_mock):
class TestMeasureHandle(unittest.TestCase):
def test_record(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.MeasureHandle(int, False, False, False, aggregator)
handle = metrics.MeasureHandle(int, False, aggregator)
handle.record(3)
self.assertEqual(handle.aggregator.current, 0)

def test_record_disabled(self):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.MeasureHandle(int, False, False, False, aggregator)
handle = metrics.MeasureHandle(int, False, aggregator)
handle.record(3)
self.assertEqual(handle.aggregator.current, 0)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_record_monotonic(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.MeasureHandle(int, True, False, True, aggregator)
handle.record(-3)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.logger")
def test_record_incorrect_type(self, logger_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.MeasureHandle(int, True, False, False, aggregator)
handle = metrics.MeasureHandle(int, True, aggregator)
handle.record(3.0)
self.assertEqual(handle.aggregator.current, 0)
self.assertTrue(logger_mock.warning.called)

@mock.patch("opentelemetry.sdk.metrics.time_ns")
def test_update(self, time_mock):
aggregator = export.aggregate.CounterAggregator()
handle = metrics.MeasureHandle(int, True, False, False, aggregator)
handle = metrics.MeasureHandle(int, True, aggregator)
time_mock.return_value = 123
handle.update(4.0)
self.assertEqual(handle.last_update_timestamp, 123)
Expand Down