From 3629f5aa5d1961d8efd70413cda7941b1105662c Mon Sep 17 00:00:00 2001 From: Leighton Date: Thu, 31 Oct 2019 16:13:37 -0700 Subject: [PATCH 01/15] Implement labelset --- .../metrics_example.py | 8 +-- .../src/opentelemetry/metrics/__init__.py | 65 ++++++++++++++----- .../src/opentelemetry/sdk/metrics/__init__.py | 63 ++++++++++++++---- 3 files changed, 102 insertions(+), 34 deletions(-) diff --git a/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py b/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py index 41bdba85972..7e9a204891a 100644 --- a/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py +++ b/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py @@ -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 diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index e866aa97cfa..2987e5695cb 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -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 @@ -66,6 +66,24 @@ def record(self, value: ValueT) -> None: value: The value to record to the handle. """ +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): + """The default LabelSet. + + Used when no LabelSet implementation is available. + """ + class Metric: """Base class for various types of metrics. @@ -74,7 +92,7 @@ class Metric: 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 @@ -85,18 +103,18 @@ 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() @@ -104,15 +122,15 @@ def get_handle(self, label_values: Sequence[str]) -> "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. """ @@ -126,15 +144,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. """ @@ -147,15 +165,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. """ @@ -174,7 +192,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. @@ -184,7 +202,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 @@ -211,8 +229,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. @@ -221,6 +237,19 @@ def create_metric( # pylint: disable=no-self-use return DefaultMetric() + def get_label_set(self, + labels: Dict[str, str], + encoded: str) -> "LabelSet": + """Gets a `LabelSet` with the given `labels`. + + Args: + labels: A dictionary representing label key to label value pairs. + encoded: A unique encoding to represent this `LabelSet`. + + 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 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 0a941cd0d18..0022c60c52c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -13,13 +13,26 @@ # limitations under the License. import logging -from typing import Sequence, Tuple, Type +from collections import OrderedDict +from typing import Dict, Sequence, Tuple, Type from opentelemetry import metrics as metrics_api logger = logging.getLogger(__name__) +class LabelSet(metrics_api.LabelSet): + """See `opentelemetry.metrics.LabelSet.""" + + def __init__( + self, + labels: Dict[str, str] = None, + encoded: str = "" + ): + self.labels = labels + self.encoded = encoded + + class BaseHandle: def __init__( self, @@ -97,14 +110,14 @@ def __init__( self.monotonic = monotonic self.handles = {} - def get_handle(self, label_values: Sequence[str]) -> BaseHandle: + def get_handle(self, label_set: LabelSet) -> BaseHandle: """See `opentelemetry.metrics.Metric.get_handle`.""" - handle = self.handles.get(label_values) + handle = self.handles.get(label_set.encoded) if not handle: handle = self.HANDLE_TYPE( self.value_type, self.enabled, self.monotonic ) - self.handles[label_values] = handle + self.handles[label_set.encoded] = handle return handle UPDATE_FUNCTION = lambda x, y: None # noqa: E731 @@ -141,10 +154,10 @@ def __init__( ) def add( - self, label_values: Sequence[str], value: metrics_api.ValueT + self, label_set: LabelSet, value: metrics_api.ValueT ) -> None: """See `opentelemetry.metrics.Counter.add`.""" - self.get_handle(label_values).add(value) + self.get_handle(label_set).add(value) UPDATE_FUNCTION = add @@ -179,10 +192,10 @@ def __init__( ) def set( - self, label_values: Sequence[str], value: metrics_api.ValueT + self, label_set: LabelSet, value: metrics_api.ValueT ) -> None: """See `opentelemetry.metrics.Gauge.set`.""" - self.get_handle(label_values).set(value) + self.get_handle(label_set).set(value) UPDATE_FUNCTION = set @@ -217,25 +230,30 @@ def __init__( ) def record( - self, label_values: Sequence[str], value: metrics_api.ValueT + self, label_set: LabelSet, value: metrics_api.ValueT ) -> None: """See `opentelemetry.metrics.Measure.record`.""" - self.get_handle(label_values).record(value) + self.get_handle(label_set).record(value) UPDATE_FUNCTION = record +# Singleton of meter.get_label_set() with zero arguments +EMPTY_LABEL_SET = LabelSet() class Meter(metrics_api.Meter): """See `opentelemetry.metrics.Meter`.""" + def __init__(self): + self.labels = {} + def record_batch( self, - label_values: Sequence[str], + label_set: LabelSet, record_tuples: Sequence[Tuple[metrics_api.Metric, metrics_api.ValueT]], ) -> None: """See `opentelemetry.metrics.Meter.record_batch`.""" for metric, value in record_tuples: - metric.UPDATE_FUNCTION(label_values, value) + metric.UPDATE_FUNCTION(label_set, value) def create_metric( self, @@ -260,5 +278,26 @@ def create_metric( monotonic=monotonic, ) + def get_label_set(self, labels: Dict[str, str]): + """See `opentelemetry.metrics.Meter.create_metric`. + + This implementation encodes the labels to use as a map key. + + Args: + labels: The dictionary of label keys to label values. + """ + if not len(labels): + return EMPTY_LABEL_SET + sorted_labels = OrderedDict(sorted(labels.items())) + # Uses statsd encoding for labels + encoded = '|#' + ','.join('%s:%s' % (key,value) \ + for (key, value) in sorted_labels.items()) + # If LabelSet exists for this meter in memory, use existing one + if not self.labels.get(encoded): + self.labels[encoded] = \ + LabelSet(labels=sorted_labels, encoded=encoded) + return self.labels[encoded] + meter = Meter() + From 6a433fed472855cbd0807059a635f956b106e369 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 1 Nov 2019 12:03:49 -0700 Subject: [PATCH 02/15] add tests --- .../src/opentelemetry/metrics/__init__.py | 5 +- .../tests/metrics/test_metrics.py | 4 + .../tests/metrics/test_metrics.py | 82 +++++++++++++------ 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 2987e5695cb..d6e1be88f38 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -238,13 +238,12 @@ def create_metric( return DefaultMetric() def get_label_set(self, - labels: Dict[str, str], - encoded: str) -> "LabelSet": + labels: Dict[str, str] + ) -> "LabelSet": """Gets a `LabelSet` with the given `labels`. Args: labels: A dictionary representing label key to label value pairs. - encoded: A unique encoding to represent this `LabelSet`. Returns: A `LabelSet` object canonicalized using the given input. """ diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 758534f2356..177f444fbc1 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -29,6 +29,10 @@ def test_record_batch(self): 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): diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index cc37bc1a8aa..c9284794283 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -27,35 +27,38 @@ def test_extends_api(self): def test_record_batch(self): meter = metrics.Meter() label_keys = ("key1",) - label_values = ("value1",) counter = metrics.Counter("name", "desc", "unit", float, label_keys) + kvp = {"key1":"value1"} + label_set = meter.get_label_set(kvp) record_tuples = [(counter, 1.0)] - meter.record_batch(label_values, record_tuples) - self.assertEqual(counter.get_handle(label_values).data, 1.0) + meter.record_batch(label_set, record_tuples) + self.assertEqual(counter.get_handle(label_set).data, 1.0) def test_record_batch_multiple(self): meter = metrics.Meter() label_keys = ("key1", "key2", "key3") - label_values = ("value1", "value2", "value3") + kvp = {"key1":"value1", "key2":"value2", "key3":"value3"} + label_set = meter.get_label_set(kvp) counter = metrics.Counter("name", "desc", "unit", float, label_keys) gauge = metrics.Gauge("name", "desc", "unit", int, label_keys) measure = metrics.Measure("name", "desc", "unit", float, label_keys) record_tuples = [(counter, 1.0), (gauge, 5), (measure, 3.0)] - meter.record_batch(label_values, record_tuples) - self.assertEqual(counter.get_handle(label_values).data, 1.0) - self.assertEqual(gauge.get_handle(label_values).data, 5) - self.assertEqual(measure.get_handle(label_values).data, 0) + meter.record_batch(label_set, record_tuples) + self.assertEqual(counter.get_handle(label_set).data, 1.0) + self.assertEqual(gauge.get_handle(label_set).data, 5) + self.assertEqual(measure.get_handle(label_set).data, 0) def test_record_batch_exists(self): meter = metrics.Meter() label_keys = ("key1",) - label_values = ("value1",) + kvp = {"key1":"value1"} + label_set = meter.get_label_set(kvp) counter = metrics.Counter("name", "desc", "unit", float, label_keys) - counter.add(label_values, 1.0) - handle = counter.get_handle(label_values) + counter.add(label_set, 1.0) + handle = counter.get_handle(label_set) record_tuples = [(counter, 1.0)] - meter.record_batch(label_values, record_tuples) - self.assertEqual(counter.get_handle(label_values), handle) + meter.record_batch(label_set, record_tuples) + self.assertEqual(counter.get_handle(label_set), handle) self.assertEqual(handle.data, 2.0) def test_create_metric(self): @@ -85,41 +88,72 @@ def test_create_measure(self): self.assertEqual(measure.value_type, float) self.assertEqual(measure.name, "name") + def test_get_label_set(self): + meter = metrics.Meter() + kvp = {"environment":"staging", "a":"z"} + label_set = meter.get_label_set(kvp) + encoding = '|#a:z,environment:staging' + self.assertEqual(label_set.encoded, encoding) + + def test_get_label_set_empty(self): + meter = metrics.Meter() + kvp = {} + label_set = meter.get_label_set(kvp) + self.assertEqual(label_set, metrics.EMPTY_LABEL_SET) + + def test_get_label_set_exists(self): + meter = metrics.Meter() + kvp = {"environment":"staging", "a":"z"} + label_set = meter.get_label_set(kvp) + label_set2 = meter.get_label_set(kvp) + self.assertEqual(label_set, label_set2) + class TestMetric(unittest.TestCase): def test_get_handle(self): + meter = metrics.Meter() metric_types = [metrics.Counter, metrics.Gauge, metrics.Measure] for _type in metric_types: metric = _type("name", "desc", "unit", int, ("key",)) - label_values = ("value",) - handle = metric.get_handle(label_values) - self.assertEqual(metric.handles.get(label_values), handle) + kvp = {"key":"value"} + label_set = meter.get_label_set(kvp) + handle = metric.get_handle(label_set) + self.assertEqual(metric.handles.get(label_set.encoded), handle) class TestCounter(unittest.TestCase): def test_add(self): + meter = metrics.Meter() metric = metrics.Counter("name", "desc", "unit", int, ("key",)) - handle = metric.get_handle(("value",)) - metric.add(("value",), 3) - metric.add(("value",), 2) + kvp = {"key":"value"} + label_set = meter.get_label_set(kvp) + handle = metric.get_handle(label_set) + metric.add(label_set, 3) + metric.add(label_set, 2) self.assertEqual(handle.data, 5) class TestGauge(unittest.TestCase): def test_set(self): + meter = metrics.Meter() metric = metrics.Gauge("name", "desc", "unit", int, ("key",)) - handle = metric.get_handle(("value",)) - metric.set(("value",), 3) + kvp = {"key":"value"} + label_set = meter.get_label_set(kvp) + handle = metric.get_handle(label_set) + metric.set(label_set, 3) self.assertEqual(handle.data, 3) - metric.set(("value",), 2) + metric.set(label_set, 2) self.assertEqual(handle.data, 2) class TestMeasure(unittest.TestCase): def test_record(self): + meter = metrics.Meter() metric = metrics.Measure("name", "desc", "unit", int, ("key",)) - handle = metric.get_handle(("value",)) - metric.record(("value",), 3) + kvp = {"key":"value"} + label_set = meter.get_label_set(kvp) + handle = metric.get_handle(label_set) + metric.record(label_set, 3) # Record not implemented yet self.assertEqual(handle.data, 0) From d816da89968ae8a8267ec0c851e3ba5954c1cdc3 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 1 Nov 2019 14:22:35 -0700 Subject: [PATCH 03/15] fix lint --- .../metrics_example.py | 2 +- .../src/opentelemetry/metrics/__init__.py | 6 ++-- .../tests/metrics/test_metrics.py | 24 +++++++++----- .../src/opentelemetry/sdk/metrics/__init__.py | 31 +++++++------------ .../tests/metrics/test_metrics.py | 20 ++++++------ 5 files changed, 42 insertions(+), 41 deletions(-) diff --git a/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py b/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py index 7e9a204891a..246d6c3507d 100644 --- a/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py +++ b/examples/opentelemetry-example-app/src/opentelemetry_example_app/metrics_example.py @@ -30,7 +30,7 @@ ("environment",), ) -label_set = meter.get_label_set({"environment":"staging"}) +label_set = meter.get_label_set({"environment": "staging"}) # Direct metric usage counter.add(label_set, 25) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index d6e1be88f38..b275389a538 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -66,6 +66,7 @@ def record(self, value: ValueT) -> None: value: The value to record to the handle. """ + class LabelSet: """A canonicalized set of labels useful for preaggregation @@ -237,9 +238,7 @@ def create_metric( # pylint: disable=no-self-use return DefaultMetric() - def get_label_set(self, - labels: Dict[str, str] - ) -> "LabelSet": + def get_label_set(self, labels: Dict[str, str]) -> "LabelSet": """Gets a `LabelSet` with the given `labels`. Args: @@ -250,6 +249,7 @@ def get_label_set(self, # 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 # from opentelemetry.util.loader import ImplementationFactory diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 177f444fbc1..1ae9f177927 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -24,12 +24,13 @@ 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) @@ -38,35 +39,42 @@ def test_get_label_set(self): class TestMetrics(unittest.TestCase): def test_default(self): default = metrics.DefaultMetric() - handle = default.get_handle(("test", "test1")) + label_set = metrics.LabelSet() + handle = default.get_handle(label_set) self.assertIsInstance(handle, metrics.DefaultMetricHandle) def test_counter(self): counter = metrics.Counter() + label_set = metrics.LabelSet() handle = counter.get_handle(("test", "test1")) 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() diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 0022c60c52c..d008b8c17b5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -24,11 +24,7 @@ class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" - def __init__( - self, - labels: Dict[str, str] = None, - encoded: str = "" - ): + def __init__(self, labels: Dict[str, str] = None, encoded: str = ""): self.labels = labels self.encoded = encoded @@ -153,9 +149,7 @@ def __init__( monotonic=monotonic, ) - def add( - self, label_set: LabelSet, value: metrics_api.ValueT - ) -> None: + def add(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: """See `opentelemetry.metrics.Counter.add`.""" self.get_handle(label_set).add(value) @@ -191,9 +185,7 @@ def __init__( monotonic=monotonic, ) - def set( - self, label_set: LabelSet, value: metrics_api.ValueT - ) -> None: + def set(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: """See `opentelemetry.metrics.Gauge.set`.""" self.get_handle(label_set).set(value) @@ -229,17 +221,17 @@ def __init__( monotonic=monotonic, ) - def record( - self, label_set: LabelSet, value: metrics_api.ValueT - ) -> None: + def record(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: """See `opentelemetry.metrics.Measure.record`.""" self.get_handle(label_set).record(value) UPDATE_FUNCTION = record + # Singleton of meter.get_label_set() with zero arguments EMPTY_LABEL_SET = LabelSet() + class Meter(metrics_api.Meter): """See `opentelemetry.metrics.Meter`.""" @@ -290,14 +282,15 @@ def get_label_set(self, labels: Dict[str, str]): return EMPTY_LABEL_SET sorted_labels = OrderedDict(sorted(labels.items())) # Uses statsd encoding for labels - encoded = '|#' + ','.join('%s:%s' % (key,value) \ - for (key, value) in sorted_labels.items()) + encoded = "|#" + ",".join( + "%s:%s" % (key, value) for (key, value) in sorted_labels.items() + ) # If LabelSet exists for this meter in memory, use existing one if not self.labels.get(encoded): - self.labels[encoded] = \ - LabelSet(labels=sorted_labels, encoded=encoded) + self.labels[encoded] = LabelSet( + labels=sorted_labels, encoded=encoded + ) return self.labels[encoded] meter = Meter() - diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index c9284794283..0427dd2108b 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -28,7 +28,7 @@ def test_record_batch(self): meter = metrics.Meter() label_keys = ("key1",) counter = metrics.Counter("name", "desc", "unit", float, label_keys) - kvp = {"key1":"value1"} + kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) record_tuples = [(counter, 1.0)] meter.record_batch(label_set, record_tuples) @@ -37,7 +37,7 @@ def test_record_batch(self): def test_record_batch_multiple(self): meter = metrics.Meter() label_keys = ("key1", "key2", "key3") - kvp = {"key1":"value1", "key2":"value2", "key3":"value3"} + kvp = {"key1": "value1", "key2": "value2", "key3": "value3"} label_set = meter.get_label_set(kvp) counter = metrics.Counter("name", "desc", "unit", float, label_keys) gauge = metrics.Gauge("name", "desc", "unit", int, label_keys) @@ -51,7 +51,7 @@ def test_record_batch_multiple(self): def test_record_batch_exists(self): meter = metrics.Meter() label_keys = ("key1",) - kvp = {"key1":"value1"} + kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) counter = metrics.Counter("name", "desc", "unit", float, label_keys) counter.add(label_set, 1.0) @@ -90,9 +90,9 @@ def test_create_measure(self): def test_get_label_set(self): meter = metrics.Meter() - kvp = {"environment":"staging", "a":"z"} + kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) - encoding = '|#a:z,environment:staging' + encoding = "|#a:z,environment:staging" self.assertEqual(label_set.encoded, encoding) def test_get_label_set_empty(self): @@ -103,7 +103,7 @@ def test_get_label_set_empty(self): def test_get_label_set_exists(self): meter = metrics.Meter() - kvp = {"environment":"staging", "a":"z"} + kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) label_set2 = meter.get_label_set(kvp) self.assertEqual(label_set, label_set2) @@ -115,7 +115,7 @@ def test_get_handle(self): metric_types = [metrics.Counter, metrics.Gauge, metrics.Measure] for _type in metric_types: metric = _type("name", "desc", "unit", int, ("key",)) - kvp = {"key":"value"} + kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) self.assertEqual(metric.handles.get(label_set.encoded), handle) @@ -125,7 +125,7 @@ class TestCounter(unittest.TestCase): def test_add(self): meter = metrics.Meter() metric = metrics.Counter("name", "desc", "unit", int, ("key",)) - kvp = {"key":"value"} + kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) metric.add(label_set, 3) @@ -137,7 +137,7 @@ class TestGauge(unittest.TestCase): def test_set(self): meter = metrics.Meter() metric = metrics.Gauge("name", "desc", "unit", int, ("key",)) - kvp = {"key":"value"} + kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) metric.set(label_set, 3) @@ -150,7 +150,7 @@ class TestMeasure(unittest.TestCase): def test_record(self): meter = metrics.Meter() metric = metrics.Measure("name", "desc", "unit", int, ("key",)) - kvp = {"key":"value"} + kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) metric.record(label_set, 3) From df80425d0afe2cc215cfb463ab321ea26c022cfb Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 1 Nov 2019 14:48:11 -0700 Subject: [PATCH 04/15] fix lint --- opentelemetry-api/tests/metrics/test_metrics.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 1ae9f177927..77031d614de 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -46,7 +46,7 @@ def test_default(self): def test_counter(self): counter = metrics.Counter() label_set = metrics.LabelSet() - handle = counter.get_handle(("test", "test1")) + handle = counter.get_handle(label_set) self.assertIsInstance(handle, metrics.CounterHandle) def test_counter_add(self): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index d008b8c17b5..49e02781542 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -272,13 +272,13 @@ def create_metric( def get_label_set(self, labels: Dict[str, str]): """See `opentelemetry.metrics.Meter.create_metric`. - + This implementation encodes the labels to use as a map key. Args: labels: The dictionary of label keys to label values. """ - if not len(labels): + if len(labels) == 0: return EMPTY_LABEL_SET sorted_labels = OrderedDict(sorted(labels.items())) # Uses statsd encoding for labels From c9600568440c0c536e0aceb6599cf4b3f60ee2fe Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 1 Nov 2019 15:01:31 -0700 Subject: [PATCH 05/15] fix docs --- opentelemetry-api/src/opentelemetry/metrics/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index b275389a538..aad70d5ba3d 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -239,7 +239,7 @@ def create_metric( return DefaultMetric() def get_label_set(self, labels: Dict[str, str]) -> "LabelSet": - """Gets a `LabelSet` with the given `labels`. + """Gets a `LabelSet` with the given labels. Args: labels: A dictionary representing label key to label value pairs. From 893640aa787a23131e15cd58089be2d9041b6c5d Mon Sep 17 00:00:00 2001 From: Leighton Date: Thu, 14 Nov 2019 18:20:54 -0800 Subject: [PATCH 06/15] include meter check --- .../src/opentelemetry/sdk/metrics/__init__.py | 39 +++++++++++++++++-- .../tests/metrics/test_metrics.py | 37 ++++++++++++------ 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 49e02781542..7b6c9316635 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -24,7 +24,11 @@ class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" - def __init__(self, labels: Dict[str, str] = None, encoded: str = ""): + def __init__(self, + meter: "Meter" = None, + labels: Dict[str, str] = None, + encoded: str = ""): + self.meter = meter self.labels = labels self.encoded = encoded @@ -93,6 +97,7 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], + meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = False, @@ -101,6 +106,7 @@ def __init__( self.description = description self.unit = unit self.value_type = value_type + self.meter = meter self.label_keys = label_keys self.enabled = enabled self.monotonic = monotonic @@ -108,14 +114,32 @@ def __init__( def get_handle(self, label_set: LabelSet) -> BaseHandle: """See `opentelemetry.metrics.Metric.get_handle`.""" - handle = self.handles.get(label_set.encoded) + lable_set_for = self._label_set_for(label_set) + handle = self.handles.get(lable_set_for.encoded) if not handle: handle = self.HANDLE_TYPE( self.value_type, self.enabled, self.monotonic ) - self.handles[label_set.encoded] = handle + self.handles[lable_set_for.encoded] = handle return handle + def _label_set_for(self, label_set: LabelSet) -> LabelSet: + """Returns an appropriate `LabelSet` based off this metric's `meter` + + If this metric's `meter` is the same as label_set's meter, that means + label_set was created from this metric's `meter` instance and the + proper handle can be found. If not, label_set was created using a + different `meter` implementation, in which the metric cannot interpret. + In this case, return the `EMPTY_LABEL_SET`. + + Args: + label_set: The `LabelSet` to check for the same `Meter` implentation. + """ + if label_set.meter and label_set.meter is self.meter: + return label_set + return EMPTY_LABEL_SET + + UPDATE_FUNCTION = lambda x, y: None # noqa: E731 @@ -135,6 +159,7 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], + meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = True, @@ -144,6 +169,7 @@ def __init__( description, unit, value_type, + meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -171,6 +197,7 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], + meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = False, @@ -180,6 +207,7 @@ def __init__( description, unit, value_type, + meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -207,6 +235,7 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], + meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = False, monotonic: bool = False, @@ -216,6 +245,7 @@ def __init__( description, unit, value_type, + meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -265,6 +295,7 @@ def create_metric( description, unit, value_type, + self, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -288,7 +319,7 @@ def get_label_set(self, labels: Dict[str, str]): # If LabelSet exists for this meter in memory, use existing one if not self.labels.get(encoded): self.labels[encoded] = LabelSet( - labels=sorted_labels, encoded=encoded + self, labels=sorted_labels, encoded=encoded ) return self.labels[encoded] diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 0427dd2108b..3f060c4e1b4 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -27,7 +27,7 @@ def test_extends_api(self): def test_record_batch(self): meter = metrics.Meter() label_keys = ("key1",) - counter = metrics.Counter("name", "desc", "unit", float, label_keys) + counter = metrics.Counter("name", "desc", "unit", float, meter,label_keys) kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) record_tuples = [(counter, 1.0)] @@ -39,9 +39,9 @@ def test_record_batch_multiple(self): label_keys = ("key1", "key2", "key3") kvp = {"key1": "value1", "key2": "value2", "key3": "value3"} label_set = meter.get_label_set(kvp) - counter = metrics.Counter("name", "desc", "unit", float, label_keys) + counter = metrics.Counter("name", "desc", "unit", float, meter, label_keys) gauge = metrics.Gauge("name", "desc", "unit", int, label_keys) - measure = metrics.Measure("name", "desc", "unit", float, label_keys) + measure = metrics.Measure("name", "desc", "unit", float, meter, label_keys) record_tuples = [(counter, 1.0), (gauge, 5), (measure, 3.0)] meter.record_batch(label_set, record_tuples) self.assertEqual(counter.get_handle(label_set).data, 1.0) @@ -53,7 +53,7 @@ def test_record_batch_exists(self): label_keys = ("key1",) kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) - counter = metrics.Counter("name", "desc", "unit", float, label_keys) + counter = metrics.Counter("name", "desc", "unit", float, meter, label_keys) counter.add(label_set, 1.0) handle = counter.get_handle(label_set) record_tuples = [(counter, 1.0)] @@ -64,7 +64,7 @@ def test_record_batch_exists(self): def test_create_metric(self): meter = metrics.Meter() counter = meter.create_metric( - "name", "desc", "unit", int, metrics.Counter, () + "name", "desc", "unit", int, metrics.Counter, meter, () ) self.assertTrue(isinstance(counter, metrics.Counter)) self.assertEqual(counter.value_type, int) @@ -73,7 +73,7 @@ def test_create_metric(self): def test_create_gauge(self): meter = metrics.Meter() gauge = meter.create_metric( - "name", "desc", "unit", float, metrics.Gauge, () + "name", "desc", "unit", float, metrics.Gauge, meter, () ) self.assertTrue(isinstance(gauge, metrics.Gauge)) self.assertEqual(gauge.value_type, float) @@ -82,7 +82,7 @@ def test_create_gauge(self): def test_create_measure(self): meter = metrics.Meter() measure = meter.create_metric( - "name", "desc", "unit", float, metrics.Measure, () + "name", "desc", "unit", float, metrics.Measure, meter, () ) self.assertTrue(isinstance(measure, metrics.Measure)) self.assertEqual(measure.value_type, float) @@ -114,17 +114,32 @@ def test_get_handle(self): meter = metrics.Meter() metric_types = [metrics.Counter, metrics.Gauge, metrics.Measure] for _type in metric_types: - metric = _type("name", "desc", "unit", int, ("key",)) + metric = _type("name", "desc", "unit", int, meter, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) self.assertEqual(metric.handles.get(label_set.encoded), handle) + def test_label_set_for(self): + meter = metrics.Meter() + metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) + kvp = {"environment": "staging", "a": "z"} + label_set = meter.get_label_set(kvp) + self.assertEqual(metric._label_set_for(label_set), label_set) + + def test_label_set_for_different(self): + meter = metrics.Meter() + meter2 = metrics.Meter() + metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) + kvp = {"environment": "staging", "a": "z"} + label_set = meter2.get_label_set(kvp) + self.assertEqual(metric._label_set_for(label_set), metrics.EMPTY_LABEL_SET) + class TestCounter(unittest.TestCase): def test_add(self): meter = metrics.Meter() - metric = metrics.Counter("name", "desc", "unit", int, ("key",)) + metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) @@ -136,7 +151,7 @@ def test_add(self): class TestGauge(unittest.TestCase): def test_set(self): meter = metrics.Meter() - metric = metrics.Gauge("name", "desc", "unit", int, ("key",)) + metric = metrics.Gauge("name", "desc", "unit", int, meter, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) @@ -149,7 +164,7 @@ def test_set(self): class TestMeasure(unittest.TestCase): def test_record(self): meter = metrics.Meter() - metric = metrics.Measure("name", "desc", "unit", int, ("key",)) + metric = metrics.Measure("name", "desc", "unit", int, meter, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) From 7e9e7ad400edf8bda7ddbc695624de1c52876ee4 Mon Sep 17 00:00:00 2001 From: Leighton Date: Thu, 14 Nov 2019 18:39:30 -0800 Subject: [PATCH 07/15] fix test --- opentelemetry-sdk/tests/metrics/export/test_export.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index ca8e8a36311..4d8e6df8575 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -22,19 +22,22 @@ class TestConsoleMetricsExporter(unittest.TestCase): # pylint: disable=no-self-use def test_export(self): + meter = metrics.Meter() exporter = ConsoleMetricsExporter() metric = metrics.Counter( "available memory", "available memory", "bytes", int, + meter, ("environment",), ) - label_values = ("staging",) - handle = metric.get_handle(label_values) + kvp = {"environment": "staging"} + label_set = meter.get_label_set(kvp) + handle = metric.get_handle(label_set) result = '{}(data="{}", label_values="{}", metric_data={})'.format( - ConsoleMetricsExporter.__name__, metric, label_values, handle + ConsoleMetricsExporter.__name__, metric, label_set, handle ) with mock.patch("sys.stdout") as mock_stdout: - exporter.export([(metric, label_values)]) + exporter.export([(metric, label_set)]) mock_stdout.write.assert_any_call(result) From f2842ffcb71c35b55b8df4b884feca8563875df3 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 15 Nov 2019 14:41:57 -0800 Subject: [PATCH 08/15] lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 10 ++++++---- .../tests/metrics/test_metrics.py | 20 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index f9db54b0db4..4d1e0343e19 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -25,10 +25,12 @@ class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" - def __init__(self, - meter: "Meter" = None, - labels: Dict[str, str] = None, - encoded: str = ""): + def __init__( + self, + meter: "Meter" = None, + labels: Dict[str, str] = None, + encoded: str = "", + ): self.meter = meter self.labels = labels self.encoded = encoded diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 3f060c4e1b4..2213459911f 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -27,7 +27,9 @@ def test_extends_api(self): def test_record_batch(self): meter = metrics.Meter() label_keys = ("key1",) - counter = metrics.Counter("name", "desc", "unit", float, meter,label_keys) + counter = metrics.Counter( + "name", "desc", "unit", float, meter, label_keys + ) kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) record_tuples = [(counter, 1.0)] @@ -39,9 +41,13 @@ def test_record_batch_multiple(self): label_keys = ("key1", "key2", "key3") kvp = {"key1": "value1", "key2": "value2", "key3": "value3"} label_set = meter.get_label_set(kvp) - counter = metrics.Counter("name", "desc", "unit", float, meter, label_keys) + counter = metrics.Counter( + "name", "desc", "unit", float, meter, label_keys + ) gauge = metrics.Gauge("name", "desc", "unit", int, label_keys) - measure = metrics.Measure("name", "desc", "unit", float, meter, label_keys) + measure = metrics.Measure( + "name", "desc", "unit", float, meter, label_keys + ) record_tuples = [(counter, 1.0), (gauge, 5), (measure, 3.0)] meter.record_batch(label_set, record_tuples) self.assertEqual(counter.get_handle(label_set).data, 1.0) @@ -53,7 +59,9 @@ def test_record_batch_exists(self): label_keys = ("key1",) kvp = {"key1": "value1"} label_set = meter.get_label_set(kvp) - counter = metrics.Counter("name", "desc", "unit", float, meter, label_keys) + counter = metrics.Counter( + "name", "desc", "unit", float, meter, label_keys + ) counter.add(label_set, 1.0) handle = counter.get_handle(label_set) record_tuples = [(counter, 1.0)] @@ -133,7 +141,9 @@ def test_label_set_for_different(self): metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) kvp = {"environment": "staging", "a": "z"} label_set = meter2.get_label_set(kvp) - self.assertEqual(metric._label_set_for(label_set), metrics.EMPTY_LABEL_SET) + self.assertEqual( + metric._label_set_for(label_set), metrics.EMPTY_LABEL_SET + ) class TestCounter(unittest.TestCase): From f390f879cb48e19a2a96ea6c77b8ba56e70ac4f7 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 15 Nov 2019 15:00:58 -0800 Subject: [PATCH 09/15] lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 +- opentelemetry-sdk/tests/metrics/test_metrics.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 4d1e0343e19..461f4bbb5f4 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -21,7 +21,7 @@ logger = logging.getLogger(__name__) - +# pylint: disable=redefined-outer-name class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 2213459911f..6e8a7f157d4 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -133,6 +133,7 @@ def test_label_set_for(self): metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) + # pylint: disable=protected-access self.assertEqual(metric._label_set_for(label_set), label_set) def test_label_set_for_different(self): @@ -141,6 +142,7 @@ def test_label_set_for_different(self): metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) kvp = {"environment": "staging", "a": "z"} label_set = meter2.get_label_set(kvp) + # pylint: disable=protected-access self.assertEqual( metric._label_set_for(label_set), metrics.EMPTY_LABEL_SET ) From 4c050d2d69848527455aeca234748def90bffb11 Mon Sep 17 00:00:00 2001 From: Leighton Date: Fri, 15 Nov 2019 15:15:17 -0800 Subject: [PATCH 10/15] lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 461f4bbb5f4..3bdc6719e73 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -21,6 +21,7 @@ logger = logging.getLogger(__name__) + # pylint: disable=redefined-outer-name class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" From 2fe06ca8b90a145e07e6914bb1cdcac12bb9194d Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 18 Nov 2019 12:00:40 -0800 Subject: [PATCH 11/15] Address comments --- opentelemetry-api/tests/metrics/test_metrics.py | 4 ++-- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 4 ++-- opentelemetry-sdk/tests/metrics/test_metrics.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 77031d614de..f8610a6fa4c 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -39,8 +39,8 @@ def test_get_label_set(self): class TestMetrics(unittest.TestCase): def test_default(self): default = metrics.DefaultMetric() - label_set = metrics.LabelSet() - handle = default.get_handle(label_set) + default_ls = metrics.DefaultLabelSet() + handle = default.get_handle(default_ls) self.assertIsInstance(handle, metrics.DefaultMetricHandle) def test_counter(self): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 3bdc6719e73..395669de1e0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -330,8 +330,8 @@ def get_label_set(self, labels: Dict[str, str]): return EMPTY_LABEL_SET sorted_labels = OrderedDict(sorted(labels.items())) # Uses statsd encoding for labels - encoded = "|#" + ",".join( - "%s:%s" % (key, value) for (key, value) in sorted_labels.items() + encoded = "," + ",".join( + "%s=%s" % (key, value) for (key, value) in sorted_labels.items() ) # If LabelSet exists for this meter in memory, use existing one if not self.labels.get(encoded): diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 6e8a7f157d4..3b1b66b2b77 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -100,7 +100,7 @@ def test_get_label_set(self): meter = metrics.Meter() kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) - encoding = "|#a:z,environment:staging" + encoding = ",a=z,environment=staging" self.assertEqual(label_set.encoded, encoding) def test_get_label_set_empty(self): From 30e5e5a04a002dc3ec5029b697271ca0198c7a69 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 19 Nov 2019 11:16:13 -0800 Subject: [PATCH 12/15] abc --- opentelemetry-api/src/opentelemetry/metrics/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index aad70d5ba3d..465020606d2 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -26,6 +26,7 @@ """ +import abc from typing import Callable, Dict, Optional, Sequence, Tuple, Type, TypeVar from opentelemetry.util import loader @@ -67,7 +68,7 @@ def record(self, value: ValueT) -> None: """ -class LabelSet: +class LabelSet(abc.ABC): """A canonicalized set of labels useful for preaggregation Re-usable LabelSet objects provide a potential optimization for scenarios From f37512828711fbe54f4eea76c899d6530290f16a Mon Sep 17 00:00:00 2001 From: Leighton Date: Wed, 27 Nov 2019 16:51:07 -0800 Subject: [PATCH 13/15] Apply Chris' changes --- .../src/opentelemetry/sdk/metrics/__init__.py | 54 +++---------------- .../tests/metrics/test_metrics.py | 39 ++++---------- 2 files changed, 18 insertions(+), 75 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 395669de1e0..fcb7e6207e5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -26,15 +26,8 @@ class LabelSet(metrics_api.LabelSet): """See `opentelemetry.metrics.LabelSet.""" - def __init__( - self, - meter: "Meter" = None, - labels: Dict[str, str] = None, - encoded: str = "", - ): - self.meter = meter + def __init__(self, labels: Dict[str, str] = None): self.labels = labels - self.encoded = encoded class BaseHandle: @@ -110,7 +103,6 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], - meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = False, @@ -119,7 +111,6 @@ def __init__( self.description = description self.unit = unit self.value_type = value_type - self.meter = meter self.label_keys = label_keys self.enabled = enabled self.monotonic = monotonic @@ -127,31 +118,14 @@ def __init__( def get_handle(self, label_set: LabelSet) -> BaseHandle: """See `opentelemetry.metrics.Metric.get_handle`.""" - lable_set_for = self._label_set_for(label_set) - handle = self.handles.get(lable_set_for.encoded) + handle = self.handles.get(label_set) if not handle: handle = self.HANDLE_TYPE( self.value_type, self.enabled, self.monotonic ) - self.handles[lable_set_for.encoded] = handle + self.handles[label_set] = handle return handle - def _label_set_for(self, label_set: LabelSet) -> LabelSet: - """Returns an appropriate `LabelSet` based off this metric's `meter` - - If this metric's `meter` is the same as label_set's meter, that means - label_set was created from this metric's `meter` instance and the - proper handle can be found. If not, label_set was created using a - different `meter` implementation, in which the metric cannot interpret. - In this case, return the `EMPTY_LABEL_SET`. - - Args: - label_set: The `LabelSet` to check for the same `Meter` implentation. - """ - if label_set.meter and label_set.meter is self.meter: - return label_set - return EMPTY_LABEL_SET - def __repr__(self): return '{}(name="{}", description={})'.format( type(self).__name__, self.name, self.description @@ -176,7 +150,6 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], - meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = True, @@ -186,7 +159,6 @@ def __init__( description, unit, value_type, - meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -214,7 +186,6 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], - meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = True, monotonic: bool = False, @@ -224,7 +195,6 @@ def __init__( description, unit, value_type, - meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -252,7 +222,6 @@ def __init__( description: str, unit: str, value_type: Type[metrics_api.ValueT], - meter: "Meter", label_keys: Sequence[str] = None, enabled: bool = False, monotonic: bool = False, @@ -262,7 +231,6 @@ def __init__( description, unit, value_type, - meter, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -275,7 +243,7 @@ def record(self, label_set: LabelSet, value: metrics_api.ValueT) -> None: UPDATE_FUNCTION = record -# Singleton of meter.get_label_set() with zero arguments +# Used when getting a LabelSet with no key/values EMPTY_LABEL_SET = LabelSet() @@ -312,7 +280,6 @@ def create_metric( description, unit, value_type, - self, label_keys=label_keys, enabled=enabled, monotonic=monotonic, @@ -328,16 +295,11 @@ def get_label_set(self, labels: Dict[str, str]): """ if len(labels) == 0: return EMPTY_LABEL_SET - sorted_labels = OrderedDict(sorted(labels.items())) - # Uses statsd encoding for labels - encoded = "," + ",".join( - "%s=%s" % (key, value) for (key, value) in sorted_labels.items() - ) + # Use simple encoding for now until encoding API is implemented + encoded = tuple(sorted(labels.items())) # If LabelSet exists for this meter in memory, use existing one - if not self.labels.get(encoded): - self.labels[encoded] = LabelSet( - self, labels=sorted_labels, encoded=encoded - ) + if not encoded in self.labels: + self.labels[encoded] = LabelSet(labels=labels) return self.labels[encoded] diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 3b1b66b2b77..81e6dd2c9d5 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -72,7 +72,7 @@ def test_record_batch_exists(self): def test_create_metric(self): meter = metrics.Meter() counter = meter.create_metric( - "name", "desc", "unit", int, metrics.Counter, meter, () + "name", "desc", "unit", int, metrics.Counter, () ) self.assertTrue(isinstance(counter, metrics.Counter)) self.assertEqual(counter.value_type, int) @@ -81,7 +81,7 @@ def test_create_metric(self): def test_create_gauge(self): meter = metrics.Meter() gauge = meter.create_metric( - "name", "desc", "unit", float, metrics.Gauge, meter, () + "name", "desc", "unit", float, metrics.Gauge, () ) self.assertTrue(isinstance(gauge, metrics.Gauge)) self.assertEqual(gauge.value_type, float) @@ -90,7 +90,7 @@ def test_create_gauge(self): def test_create_measure(self): meter = metrics.Meter() measure = meter.create_metric( - "name", "desc", "unit", float, metrics.Measure, meter, () + "name", "desc", "unit", float, metrics.Measure, () ) self.assertTrue(isinstance(measure, metrics.Measure)) self.assertEqual(measure.value_type, float) @@ -100,8 +100,8 @@ def test_get_label_set(self): meter = metrics.Meter() kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) - encoding = ",a=z,environment=staging" - self.assertEqual(label_set.encoded, encoding) + encoded = tuple(sorted(kvp.items())) + self.assertIs(meter.labels[encoded], label_set) def test_get_label_set_empty(self): meter = metrics.Meter() @@ -114,7 +114,7 @@ def test_get_label_set_exists(self): kvp = {"environment": "staging", "a": "z"} label_set = meter.get_label_set(kvp) label_set2 = meter.get_label_set(kvp) - self.assertEqual(label_set, label_set2) + self.assertIs(label_set, label_set2) class TestMetric(unittest.TestCase): @@ -126,32 +126,13 @@ def test_get_handle(self): kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) - self.assertEqual(metric.handles.get(label_set.encoded), handle) - - def test_label_set_for(self): - meter = metrics.Meter() - metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) - kvp = {"environment": "staging", "a": "z"} - label_set = meter.get_label_set(kvp) - # pylint: disable=protected-access - self.assertEqual(metric._label_set_for(label_set), label_set) - - def test_label_set_for_different(self): - meter = metrics.Meter() - meter2 = metrics.Meter() - metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) - kvp = {"environment": "staging", "a": "z"} - label_set = meter2.get_label_set(kvp) - # pylint: disable=protected-access - self.assertEqual( - metric._label_set_for(label_set), metrics.EMPTY_LABEL_SET - ) + self.assertEqual(metric.handles.get(label_set), handle) class TestCounter(unittest.TestCase): def test_add(self): meter = metrics.Meter() - metric = metrics.Counter("name", "desc", "unit", int, meter, ("key",)) + metric = metrics.Counter("name", "desc", "unit", int, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) @@ -163,7 +144,7 @@ def test_add(self): class TestGauge(unittest.TestCase): def test_set(self): meter = metrics.Meter() - metric = metrics.Gauge("name", "desc", "unit", int, meter, ("key",)) + metric = metrics.Gauge("name", "desc", "unit", int, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) @@ -176,7 +157,7 @@ def test_set(self): class TestMeasure(unittest.TestCase): def test_record(self): meter = metrics.Meter() - metric = metrics.Measure("name", "desc", "unit", int, meter, ("key",)) + metric = metrics.Measure("name", "desc", "unit", int, ("key",)) kvp = {"key": "value"} label_set = meter.get_label_set(kvp) handle = metric.get_handle(label_set) From ce33371ecf117a5e53f0d39dd62d92e8597d4e6f Mon Sep 17 00:00:00 2001 From: Leighton Date: Wed, 27 Nov 2019 17:04:29 -0800 Subject: [PATCH 14/15] fix lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index fcb7e6207e5..bb495bc1be3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -298,7 +298,7 @@ def get_label_set(self, labels: Dict[str, str]): # Use simple encoding for now until encoding API is implemented encoded = tuple(sorted(labels.items())) # If LabelSet exists for this meter in memory, use existing one - if not encoded in self.labels: + if encoded not in self.labels: self.labels[encoded] = LabelSet(labels=labels) return self.labels[encoded] From 792449eaa5a68bf9dc6eab827f585e964eb27c50 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 2 Dec 2019 23:58:12 -0500 Subject: [PATCH 15/15] Use mypy==0.740 --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index e0e076fe77d..445ed696ed5 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,7 @@ python = deps = test: pytest coverage: pytest-cov - mypy,mypyinstalled: mypy~=0.740 + mypy,mypyinstalled: mypy==0.740 setenv = mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/