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

Implement LabelSet for metrics #258

Merged
merged 17 commits into from
Dec 3, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,17 @@
("environment",),
)

label_values = ("staging",)
label_set = meter.get_label_set({"environment": "staging"})

# Direct metric usage
counter.add(label_values, 25)
counter.add(label_set, 25)

# Handle usage
counter_handle = counter.get_handle(label_values)
counter_handle = counter.get_handle(label_set)
counter_handle.add(100)

# Record batch usage
meter.record_batch(label_values, [(counter, 50)])
meter.record_batch(label_set, [(counter, 50)])
print(counter_handle.data)

# TODO: exporters
64 changes: 46 additions & 18 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@


"""
from typing import Callable, Optional, Sequence, Tuple, Type, TypeVar
from typing import Callable, Dict, Optional, Sequence, Tuple, Type, TypeVar

from opentelemetry.util import loader

Expand Down Expand Up @@ -67,14 +67,33 @@ def record(self, value: ValueT) -> None:
"""


class LabelSet:
"""A canonicalized set of labels useful for preaggregation

Re-usable LabelSet objects provide a potential optimization for scenarios
where handles might not be effective. For example, if the LabelSet will be
re-used but only used once per metrics, handles do not offer any
optimization. It may best to pre-compute a canonicalized LabelSet once and
re-use it with the direct calling convention. LabelSets are immutable and
should be opaque in implementation.
"""


class DefaultLabelSet(LabelSet):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required? Should LabelSet be made an ABC?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like how metric and handle have default implementations, we don't want to have the meter functions to return None objects (it would be get_label_set in this case). Also a LabelSet has no methods so I didn't think it was necessary to use an ABC (unless my understanding of the benefits is mistaken).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point here is that since LabelSet is not an ABC we technically don't need DefaultLabelSet since we could return a plain LabelSet instance from Meter.get_label_set.

"""The default LabelSet.

Used when no LabelSet implementation is available.
"""


class Metric:
"""Base class for various types of metrics.

Metric class that inherit from this class are specialized with the type of
handle that the metric holds.
"""

def get_handle(self, label_values: Sequence[str]) -> "object":
def get_handle(self, label_set: LabelSet) -> "object":
"""Gets a handle, used for repeated-use of metrics instruments.

Handles are useful to reduce the cost of repeatedly recording a metric
Expand All @@ -85,34 +104,34 @@ def get_handle(self, label_values: Sequence[str]) -> "object":
a value was not provided are permitted.

Args:
label_values: Values to associate with the returned handle.
label_set: `LabelSet` to associate with the returned handle.
"""


class DefaultMetric(Metric):
"""The default Metric used when no Metric implementation is available."""

def get_handle(self, label_values: Sequence[str]) -> "DefaultMetricHandle":
def get_handle(self, label_set: LabelSet) -> "DefaultMetricHandle":
"""Gets a `DefaultMetricHandle`.

Args:
label_values: The label values associated with the handle.
label_set: `LabelSet` to associate with the returned handle.
"""
return DefaultMetricHandle()


class Counter(Metric):
"""A counter type metric that expresses the computation of a sum."""

def get_handle(self, label_values: Sequence[str]) -> "CounterHandle":
def get_handle(self, label_set: LabelSet) -> "CounterHandle":
"""Gets a `CounterHandle`."""
return CounterHandle()

def add(self, label_values: Sequence[str], value: ValueT) -> None:
def add(self, label_set: LabelSet, value: ValueT) -> None:
"""Increases the value of the counter by ``value``.

Args:
label_values: The label values associated with the metric.
label_set: `LabelSet` to associate with the returned handle.
value: The value to add to the counter metric.
"""

Expand All @@ -126,15 +145,15 @@ class Gauge(Metric):
the measurement interval is arbitrary.
"""

def get_handle(self, label_values: Sequence[str]) -> "GaugeHandle":
def get_handle(self, label_set: LabelSet) -> "GaugeHandle":
"""Gets a `GaugeHandle`."""
return GaugeHandle()

def set(self, label_values: Sequence[str], value: ValueT) -> None:
def set(self, label_set: LabelSet, value: ValueT) -> None:
"""Sets the value of the gauge to ``value``.

Args:
label_values: The label values associated with the metric.
label_set: `LabelSet` to associate with the returned handle.
value: The value to set the gauge metric to.
"""

Expand All @@ -147,15 +166,15 @@ class Measure(Metric):
Negative inputs will be discarded when monotonic is True.
"""

def get_handle(self, label_values: Sequence[str]) -> "MeasureHandle":
def get_handle(self, label_set: LabelSet) -> "MeasureHandle":
"""Gets a `MeasureHandle` with a float value."""
return MeasureHandle()

def record(self, label_values: Sequence[str], value: ValueT) -> None:
def record(self, label_set: LabelSet, value: ValueT) -> None:
"""Records the ``value`` to the measure.

Args:
label_values: The label values associated with the metric.
label_set: `LabelSet` to associate with the returned handle.
value: The value to record to this measure metric.
"""

Expand All @@ -174,7 +193,7 @@ class Meter:

def record_batch(
self,
label_values: Sequence[str],
label_set: LabelSet,
record_tuples: Sequence[Tuple["Metric", ValueT]],
) -> None:
"""Atomically records a batch of `Metric` and value pairs.
Expand All @@ -184,7 +203,7 @@ def record_batch(
match the key-value pairs in the label tuples.

Args:
label_values: The label values associated with all measurements in
label_set: The `LabelSet` associated with all measurements in
the batch. A measurement is a tuple, representing the `Metric`
being recorded and the corresponding value to record.
record_tuples: A sequence of pairs of `Metric` s and the
Expand All @@ -211,8 +230,6 @@ def create_metric(
value_type: The type of values being recorded by the metric.
metric_type: The type of metric being created.
label_keys: The keys for the labels with dynamic values.
Order of the sequence is important as the same order must be
used on recording when suppling values for these labels.
enabled: Whether to report the metric by default.
monotonic: Whether to only allow non-negative values.

Expand All @@ -221,6 +238,17 @@ def create_metric(
# pylint: disable=no-self-use
return DefaultMetric()

def get_label_set(self, labels: Dict[str, str]) -> "LabelSet":
"""Gets a `LabelSet` with the given labels.

Args:
labels: A dictionary representing label key to label value pairs.

Returns: A `LabelSet` object canonicalized using the given input.
"""
# pylint: disable=no-self-use
return DefaultLabelSet()


# Once https://github.com/python/mypy/issues/7092 is resolved,
# the following type definition should be replaced with
Expand Down
28 changes: 20 additions & 8 deletions opentelemetry-api/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,45 +24,57 @@ def setUp(self):

def test_record_batch(self):
counter = metrics.Counter()
self.meter.record_batch(("values"), ((counter, 1),))
label_set = metrics.LabelSet()
self.meter.record_batch(label_set, ((counter, 1),))

def test_create_metric(self):
metric = self.meter.create_metric("", "", "", float, metrics.Counter)
self.assertIsInstance(metric, metrics.DefaultMetric)

def test_get_label_set(self):
metric = self.meter.get_label_set({})
self.assertIsInstance(metric, metrics.DefaultLabelSet)


class TestMetrics(unittest.TestCase):
def test_default(self):
default = metrics.DefaultMetric()
handle = default.get_handle(("test", "test1"))
label_set = metrics.LabelSet()
lzchen marked this conversation as resolved.
Show resolved Hide resolved
handle = default.get_handle(label_set)
self.assertIsInstance(handle, metrics.DefaultMetricHandle)

def test_counter(self):
counter = metrics.Counter()
handle = counter.get_handle(("test", "test1"))
label_set = metrics.LabelSet()
handle = counter.get_handle(label_set)
self.assertIsInstance(handle, metrics.CounterHandle)

def test_counter_add(self):
counter = metrics.Counter()
counter.add(("value",), 1)
label_set = metrics.LabelSet()
counter.add(label_set, 1)

def test_gauge(self):
gauge = metrics.Gauge()
handle = gauge.get_handle(("test", "test1"))
label_set = metrics.LabelSet()
handle = gauge.get_handle(label_set)
self.assertIsInstance(handle, metrics.GaugeHandle)

def test_gauge_set(self):
gauge = metrics.Gauge()
gauge.set(("value",), 1)
label_set = metrics.LabelSet()
gauge.set(label_set, 1)

def test_measure(self):
measure = metrics.Measure()
handle = measure.get_handle(("test", "test1"))
label_set = metrics.LabelSet()
handle = measure.get_handle(label_set)
self.assertIsInstance(handle, metrics.MeasureHandle)

def test_measure_record(self):
measure = metrics.Measure()
measure.record(("value",), 1)
label_set = metrics.LabelSet()
measure.record(label_set, 1)

def test_default_handle(self):
metrics.DefaultMetricHandle()
Expand Down
Loading