From cbf8a3c0e4f0903790c6ff0ab2ad4fa660987daf Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 23 Jul 2020 16:43:11 +0000 Subject: [PATCH 1/5] OpenCensus agent exporter fixes - Pass start_timestamp to the timeseries in OC exporter for stateful metrics - Convert label values to strings, just in case --- .../CHANGELOG.md | 3 +++ .../metrics_exporter/__init__.py | 19 +++++++++++++++-- .../test_otcollector_metrics_exporter.py | 21 +++++++++++++------ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/ext/opentelemetry-ext-opencensusexporter/CHANGELOG.md b/ext/opentelemetry-ext-opencensusexporter/CHANGELOG.md index 4d1c9a97b82..52596a45596 100644 --- a/ext/opentelemetry-ext-opencensusexporter/CHANGELOG.md +++ b/ext/opentelemetry-ext-opencensusexporter/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Send start_timestamp and convert labels to strings + ([#937](https://github.com/open-telemetry/opentelemetry-python/pull/937)) + ## 0.8b0 Released 2020-05-27 diff --git a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py index bb1a1ee888c..d6547637a7d 100644 --- a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py +++ b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py @@ -18,6 +18,7 @@ from typing import Sequence import grpc +from google.protobuf.timestamp_pb2 import Timestamp from opencensus.proto.agent.metrics.v1 import ( metrics_service_pb2, metrics_service_pb2_grpc, @@ -65,6 +66,8 @@ def __init__( self.client = client self.node = utils.get_node(service_name, host_name) + self.exporter_start_timestamp = Timestamp() + self.exporter_start_timestamp.GetCurrentTime() def export( self, metric_records: Sequence[MetricRecord] @@ -89,7 +92,9 @@ def shutdown(self) -> None: def generate_metrics_requests( self, metrics: Sequence[MetricRecord] ) -> metrics_service_pb2.ExportMetricsServiceRequest: - collector_metrics = translate_to_collector(metrics) + collector_metrics = translate_to_collector( + metrics, self.exporter_start_timestamp + ) service_request = metrics_service_pb2.ExportMetricsServiceRequest( node=self.node, metrics=collector_metrics ) @@ -99,6 +104,7 @@ def generate_metrics_requests( # pylint: disable=too-many-branches def translate_to_collector( metric_records: Sequence[MetricRecord], + exporter_start_timestamp: Timestamp, ) -> Sequence[metrics_pb2.Metric]: collector_metrics = [] for metric_record in metric_records: @@ -109,7 +115,8 @@ def translate_to_collector( label_keys.append(metrics_pb2.LabelKey(key=label_tuple[0])) label_values.append( metrics_pb2.LabelValue( - has_value=label_tuple[1] is not None, value=label_tuple[1] + has_value=label_tuple[1] is not None, + value=str(label_tuple[1]), ) ) @@ -121,9 +128,17 @@ def translate_to_collector( label_keys=label_keys, ) + # If cumulative and stateful, explicitly set the start_timestamp to + # exporter start time. + if metric_record.instrument.meter.batcher.stateful: + start_timestamp = exporter_start_timestamp + else: + start_timestamp = None + timeseries = metrics_pb2.TimeSeries( label_values=label_values, points=[get_collector_point(metric_record)], + start_timestamp=start_timestamp, ) collector_metrics.append( metrics_pb2.Metric( diff --git a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py index f538e5acecd..51da1153d82 100644 --- a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py +++ b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py @@ -41,7 +41,7 @@ def setUpClass(cls): # pylint: disable=protected-access metrics.set_meter_provider(MeterProvider()) cls._meter = metrics.get_meter(__name__) - cls._labels = {"environment": "staging"} + cls._labels = {"environment": "staging", "number": 321} cls._key_labels = get_labels_as_key(cls._labels) def test_constructor(self): @@ -119,7 +119,7 @@ def test_export(self): client=mock_client, host_name=host_name ) test_metric = self._meter.create_metric( - "testname", "testdesc", "unit", int, Counter, ["environment"] + "testname", "testdesc", "unit", int, Counter, self._labels.keys(), ) record = MetricRecord( test_metric, self._key_labels, aggregate.SumAggregator(), @@ -142,13 +142,16 @@ def test_export(self): def test_translate_to_collector(self): test_metric = self._meter.create_metric( - "testname", "testdesc", "unit", int, Counter, ["environment"] + "testname", "testdesc", "unit", int, Counter, self._labels.keys() ) aggregator = aggregate.SumAggregator() aggregator.update(123) aggregator.take_checkpoint() record = MetricRecord(test_metric, self._key_labels, aggregator,) - output_metrics = metrics_exporter.translate_to_collector([record]) + start_timestamp = Timestamp() + output_metrics = metrics_exporter.translate_to_collector( + [record], start_timestamp, + ) self.assertEqual(len(output_metrics), 1) self.assertIsInstance(output_metrics[0], metrics_pb2.Metric) self.assertEqual(output_metrics[0].metric_descriptor.name, "testname") @@ -161,14 +164,20 @@ def test_translate_to_collector(self): metrics_pb2.MetricDescriptor.CUMULATIVE_INT64, ) self.assertEqual( - len(output_metrics[0].metric_descriptor.label_keys), 1 + len(output_metrics[0].metric_descriptor.label_keys), 2 ) self.assertEqual( output_metrics[0].metric_descriptor.label_keys[0].key, "environment", ) + self.assertEqual( + output_metrics[0].metric_descriptor.label_keys[1].key, "number", + ) self.assertEqual(len(output_metrics[0].timeseries), 1) - self.assertEqual(len(output_metrics[0].timeseries[0].label_values), 1) + self.assertEqual(len(output_metrics[0].timeseries[0].label_values), 2) + self.assertEqual( + output_metrics[0].timeseries[0].start_timestamp, start_timestamp + ) self.assertEqual( output_metrics[0].timeseries[0].label_values[0].has_value, True ) From 14ecb2b7a1eb4009b7c7aa37f3a0c0ab7f91991f Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Tue, 28 Jul 2020 19:51:12 +0000 Subject: [PATCH 2/5] Attach resources to metric --- .../metrics_exporter/__init__.py | 13 ++++++- .../test_otcollector_metrics_exporter.py | 35 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py index d6547637a7d..edb426cdae1 100644 --- a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py +++ b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py @@ -24,6 +24,7 @@ metrics_service_pb2_grpc, ) from opencensus.proto.metrics.v1 import metrics_pb2 +from opencensus.proto.resource.v1 import resource_pb2 import opentelemetry.ext.opencensusexporter.util as utils from opentelemetry.sdk.metrics import Counter, Metric @@ -142,7 +143,9 @@ def translate_to_collector( ) collector_metrics.append( metrics_pb2.Metric( - metric_descriptor=metric_descriptor, timeseries=[timeseries] + metric_descriptor=metric_descriptor, + timeseries=[timeseries], + resource=get_resource(metric_record), ) ) return collector_metrics @@ -177,3 +180,11 @@ def get_collector_point(metric_record: MetricRecord) -> metrics_pb2.Point: ) ) return point + + +def get_resource(metric_record: MetricRecord) -> resource_pb2.Resource: + resource_labels = metric_record.instrument.meter.resource.labels + return resource_pb2.Resource( + type=resource_labels.get("service.name"), + labels={k: str(v) for k, v in resource_labels.items()}, + ) diff --git a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py index 51da1153d82..6ac3674cc17 100644 --- a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py +++ b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py @@ -32,6 +32,7 @@ MetricsExportResult, aggregate, ) +from opentelemetry.sdk.resources import Resource # pylint: disable=no-member @@ -39,7 +40,14 @@ class TestCollectorMetricsExporter(unittest.TestCase): @classmethod def setUpClass(cls): # pylint: disable=protected-access - metrics.set_meter_provider(MeterProvider()) + cls._resource_labels = { + "service.name": "some_application", + "key_with_int_val": 321, + "key_with_true": True, + } + metrics.set_meter_provider( + MeterProvider(resource=Resource(cls._resource_labels)) + ) cls._meter = metrics.get_meter(__name__) cls._labels = {"environment": "staging", "number": 321} cls._key_labels = get_labels_as_key(cls._labels) @@ -173,6 +181,31 @@ def test_translate_to_collector(self): self.assertEqual( output_metrics[0].metric_descriptor.label_keys[1].key, "number", ) + + self.assertIsNotNone(output_metrics[0].resource) + self.assertEqual( + output_metrics[0].resource.type, + self._resource_labels["service.name"], + ) + self.assertEqual( + output_metrics[0].resource.labels["service.name"], + self._resource_labels["service.name"], + ) + self.assertIsInstance( + output_metrics[0].resource.labels["key_with_int_val"], str, + ) + self.assertEqual( + output_metrics[0].resource.labels["key_with_int_val"], + str(self._resource_labels["key_with_int_val"]), + ) + self.assertIsInstance( + output_metrics[0].resource.labels["key_with_true"], str, + ) + self.assertEqual( + output_metrics[0].resource.labels["key_with_true"], + str(self._resource_labels["key_with_true"]), + ) + self.assertEqual(len(output_metrics[0].timeseries), 1) self.assertEqual(len(output_metrics[0].timeseries[0].label_values), 2) self.assertEqual( From 44f3372dacb117f4b5dac1335c4d52bb9c83d7df Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Mon, 3 Aug 2020 21:30:20 +0000 Subject: [PATCH 3/5] Add correct OC resource type --- .../opencensus/metrics_exporter/__init__.py | 23 ++++++- .../test_otcollector_metrics_exporter.py | 67 +++++++++++++++++-- 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py b/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py index 2099ab9f1ae..5c612d68321 100644 --- a/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py @@ -15,7 +15,7 @@ """OpenCensus Collector Metrics Exporter.""" import logging -from typing import Sequence +from typing import Dict, Sequence import grpc from google.protobuf.timestamp_pb2 import Timestamp @@ -36,6 +36,14 @@ DEFAULT_ENDPOINT = "localhost:55678" +# In priority order. See collector impl https://bit.ly/2DvJW6y +OT_LABEL_PRESENCE_TO_RESOURCE_TYPE = [ + ("container.name", "container"), + ("k8s.pod.name", "k8s"), + ("host.name", "host"), + ("cloud.provider", "cloud"), +] + logger = logging.getLogger(__name__) @@ -185,6 +193,17 @@ def get_collector_point(metric_record: MetricRecord) -> metrics_pb2.Point: def get_resource(metric_record: MetricRecord) -> resource_pb2.Resource: resource_labels = metric_record.instrument.meter.resource.labels return resource_pb2.Resource( - type=resource_labels.get("service.name"), + type=infer_oc_resource_type(resource_labels), labels={k: str(v) for k, v in resource_labels.items()}, ) + + +def infer_oc_resource_type(resource_labels: Dict[str, str]) -> str: + """Convert from OT resource labels to OC resource type""" + for ( + ot_resource_key, + oc_resource_type, + ) in OT_LABEL_PRESENCE_TO_RESOURCE_TYPE: + if ot_resource_key in resource_labels: + return oc_resource_type + return "" diff --git a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py index 957cee8bd97..1ec1a574487 100644 --- a/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_metrics_exporter.py @@ -41,7 +41,7 @@ class TestCollectorMetricsExporter(unittest.TestCase): def setUpClass(cls): # pylint: disable=protected-access cls._resource_labels = { - "service.name": "some_application", + "key_with_str_value": "some string", "key_with_int_val": 321, "key_with_true": True, } @@ -50,7 +50,7 @@ def setUpClass(cls): ) cls._meter = metrics.get_meter(__name__) cls._labels = {"environment": "staging", "number": 321} - cls._key_labels = get_labels_as_key(cls._labels) + cls._key_labels = get_dict_as_key(cls._labels) def test_constructor(self): mock_get_node = mock.Mock() @@ -184,12 +184,11 @@ def test_translate_to_collector(self): self.assertIsNotNone(output_metrics[0].resource) self.assertEqual( - output_metrics[0].resource.type, - self._resource_labels["service.name"], + output_metrics[0].resource.type, "", ) self.assertEqual( - output_metrics[0].resource.labels["service.name"], - self._resource_labels["service.name"], + output_metrics[0].resource.labels["key_with_str_value"], + self._resource_labels["key_with_str_value"], ) self.assertIsInstance( output_metrics[0].resource.labels["key_with_int_val"], str, @@ -229,3 +228,59 @@ def test_translate_to_collector(self): self.assertEqual( output_metrics[0].timeseries[0].points[0].int64_value, 123 ) + + def test_infer_ot_resource_type(self): + # empty resource + self.assertEqual(metrics_exporter.infer_oc_resource_type({}), "") + + # container + self.assertEqual( + metrics_exporter.infer_oc_resource_type( + { + "k8s.cluster.name": "cluster1", + "k8s.pod.name": "pod1", + "k8s.namespace.name": "namespace1", + "container.name": "container-name1", + "cloud.account.id": "proj1", + "cloud.zone": "zone1", + } + ), + "container", + ) + + # k8s pod + self.assertEqual( + metrics_exporter.infer_oc_resource_type( + { + "k8s.cluster.name": "cluster1", + "k8s.pod.name": "pod1", + "k8s.namespace.name": "namespace1", + "cloud.zone": "zone1", + } + ), + "k8s", + ) + + # host + self.assertEqual( + metrics_exporter.infer_oc_resource_type( + { + "k8s.cluster.name": "cluster1", + "cloud.zone": "zone1", + "host.name": "node1", + } + ), + "host", + ) + + # cloud + self.assertEqual( + metrics_exporter.infer_oc_resource_type( + { + "cloud.provider": "gcp", + "host.id": "inst1", + "cloud.zone": "zone1", + } + ), + "cloud", + ) From 946b57dd37a9fc6e4ce740a952e155321461f874 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Tue, 11 Aug 2020 21:52:35 +0000 Subject: [PATCH 4/5] prefix constant with underscore, use tuple --- .../exporter/opencensus/metrics_exporter/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py b/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py index 5c612d68321..76986a8a59d 100644 --- a/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py +++ b/exporter/opentelemetry-exporter-opencensus/src/opentelemetry/exporter/opencensus/metrics_exporter/__init__.py @@ -37,12 +37,12 @@ DEFAULT_ENDPOINT = "localhost:55678" # In priority order. See collector impl https://bit.ly/2DvJW6y -OT_LABEL_PRESENCE_TO_RESOURCE_TYPE = [ +_OT_LABEL_PRESENCE_TO_RESOURCE_TYPE = ( ("container.name", "container"), ("k8s.pod.name", "k8s"), ("host.name", "host"), ("cloud.provider", "cloud"), -] +) logger = logging.getLogger(__name__) @@ -203,7 +203,7 @@ def infer_oc_resource_type(resource_labels: Dict[str, str]) -> str: for ( ot_resource_key, oc_resource_type, - ) in OT_LABEL_PRESENCE_TO_RESOURCE_TYPE: + ) in _OT_LABEL_PRESENCE_TO_RESOURCE_TYPE: if ot_resource_key in resource_labels: return oc_resource_type return "" From 09d1e539c70f07682617a4bb7745d9f367692728 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 13 Aug 2020 19:02:32 +0000 Subject: [PATCH 5/5] move changelog entry to bottom --- exporter/opentelemetry-exporter-opencensus/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/opentelemetry-exporter-opencensus/CHANGELOG.md b/exporter/opentelemetry-exporter-opencensus/CHANGELOG.md index 333aafcbe5e..13924e489b3 100644 --- a/exporter/opentelemetry-exporter-opencensus/CHANGELOG.md +++ b/exporter/opentelemetry-exporter-opencensus/CHANGELOG.md @@ -2,10 +2,10 @@ ## Unreleased -- Send start_timestamp and convert labels to strings - ([#937](https://github.com/open-telemetry/opentelemetry-python/pull/937)) - Change package name to opentelemetry-exporter-opencensus ([#953](https://github.com/open-telemetry/opentelemetry-python/pull/953)) +- Send start_timestamp and convert labels to strings + ([#937](https://github.com/open-telemetry/opentelemetry-python/pull/937)) ## 0.8b0