From e26bbef8e0bb3da0fd25411667089c41866aef57 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 22 Oct 2020 21:25:59 +0000 Subject: [PATCH] fixed more 0.14b0 breakages and updated tests to catch them --- get_mock_server.sh | 7 ++- .../exporter/cloud_monitoring/__init__.py | 9 +-- .../tests/test_cloud_monitoring.py | 32 +++++++---- ...t_cloud_monitoring_exporter_integration.py | 56 ++++++++++++------- .../base_exporter_integration_test.py | 22 +++++++- tox.ini | 4 +- 6 files changed, 89 insertions(+), 41 deletions(-) diff --git a/get_mock_server.sh b/get_mock_server.sh index 07e07e24..8d78ac3c 100755 --- a/get_mock_server.sh +++ b/get_mock_server.sh @@ -1,4 +1,10 @@ #!/bin/bash + +# Use this envvar when testing a local mock_server binary +if [ "$SKIP_GET_MOCK_SERVER" == "true" ]; then + exit +fi + VERSION=v2-alpha BINARY=mock_server-x64-linux-$VERSION if ! [ -e $1/$BINARY ]; then @@ -7,4 +13,3 @@ if ! [ -e $1/$BINARY ]; then fi ln -sf $1/$BINARY $1/mock_server - diff --git a/opentelemetry-exporter-google-cloud/src/opentelemetry/exporter/cloud_monitoring/__init__.py b/opentelemetry-exporter-google-cloud/src/opentelemetry/exporter/cloud_monitoring/__init__.py index fb9fcfba..8563ed0a 100644 --- a/opentelemetry-exporter-google-cloud/src/opentelemetry/exporter/cloud_monitoring/__init__.py +++ b/opentelemetry-exporter-google-cloud/src/opentelemetry/exporter/cloud_monitoring/__init__.py @@ -233,7 +233,7 @@ def _set_start_end_times(self, point_dict, record, metric_descriptor): == MetricDescriptor.MetricKind.CUMULATIVE ): if ( - record.instrument.meter.batcher.stateful + record.instrument.meter.processor.stateful or updated_key not in self._last_updated ): # The aggregation has not reset since the exporter @@ -279,9 +279,10 @@ def export( if not metric_descriptor: continue series = TimeSeries( - resource=self._get_monitored_resource( - record.instrument.meter.resource - ) + resource=self._get_monitored_resource(record.resource), + # TODO: remove after fix for + # https://github.com/googleinterns/cloud-operations-api-mock/issues/58 + metric_kind=metric_descriptor.metric_kind, ) series.metric.type = metric_descriptor.type for key, value in record.labels: diff --git a/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring.py b/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring.py index 816ee1ec..03aa5a6c 100644 --- a/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring.py +++ b/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring.py @@ -15,6 +15,7 @@ import unittest from collections import OrderedDict +from typing import Optional from unittest import mock from google.api.label_pb2 import LabelDescriptor @@ -28,6 +29,7 @@ WRITE_INTERVAL, CloudMonitoringMetricsExporter, ) +from opentelemetry.sdk.metrics import MeterProvider from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( HistogramAggregator, @@ -46,10 +48,13 @@ def __init__(self, stateful): self.stateful = stateful -class MockMeter: - def __init__(self, resource=Resource.create_empty(), stateful=True): - self.resource = resource - self.batcher = MockBatcher(stateful) +def mock_meter(stateful: Optional[bool] = None): + # create an autospec of Meter from an instance in order to capture instance + # variables (meter.processor) + meter = MeterProvider(stateful).get_meter(__name__) + meter_mock = mock.create_autospec(meter, spec_set=True) + meter_mock.processor.stateful = meter.processor.stateful + return meter_mock class MockMetric: @@ -64,7 +69,7 @@ def __init__( self.name = name self.description = description self.value_type = value_type - self.meter = meter or MockMeter(stateful=stateful) + self.meter = meter or mock_meter(stateful) # pylint: disable=protected-access @@ -299,16 +304,16 @@ def test_export(self): exporter.export( [ MetricRecord( - MockMetric(meter=MockMeter(resource=resource)), + MockMetric(meter=mock_meter()), (("label1", "value1"), ("label2", 1),), sum_agg_one, - Resource.create_empty(), + resource, ), MetricRecord( - MockMetric(meter=MockMeter(resource=resource)), + MockMetric(meter=mock_meter()), (("label1", "value2"), ("label2", 2),), sum_agg_one, - Resource.create_empty(), + resource, ), ] ) @@ -318,6 +323,7 @@ def test_export(self): ) series1 = TimeSeries(resource=expected_resource) + series1.metric_kind = MetricDescriptor.MetricKind.CUMULATIVE series1.metric.type = "custom.googleapis.com/OpenTelemetry/name" series1.metric.labels["label1"] = "value1" series1.metric.labels["label2"] = "1" @@ -329,6 +335,7 @@ def test_export(self): point.interval.start_time.nanos = 0 series2 = TimeSeries(resource=expected_resource) + series2.metric_kind = MetricDescriptor.MetricKind.CUMULATIVE series2.metric.type = "custom.googleapis.com/OpenTelemetry/name" series2.metric.labels["label1"] = "value2" series2.metric.labels["label2"] = "2" @@ -382,6 +389,7 @@ def test_export(self): ] ) series3 = TimeSeries() + series3.metric_kind = MetricDescriptor.MetricKind.CUMULATIVE series3.metric.type = "custom.googleapis.com/OpenTelemetry/name" series3.metric.labels["label1"] = "changed_label" series3.metric.labels["label2"] = "2" @@ -432,7 +440,7 @@ def test_export_value_observer(self): exporter.export( [ MetricRecord( - MockMetric(meter=MockMeter()), + MockMetric(meter=mock_meter()), (), aggregator, Resource.create_empty(), @@ -441,6 +449,7 @@ def test_export_value_observer(self): ) series = TimeSeries() + series.metric_kind = MetricDescriptor.MetricKind.GAUGE series.metric.type = "custom.googleapis.com/OpenTelemetry/name" point = series.points.add() point.value.int64_value = 5 @@ -485,7 +494,7 @@ def test_export_histogram(self): exporter.export( [ MetricRecord( - MockMetric(meter=MockMeter()), + MockMetric(meter=mock_meter()), (), aggregator, Resource.create_empty(), @@ -494,6 +503,7 @@ def test_export_histogram(self): ) series = TimeSeries() + series.metric_kind = MetricDescriptor.MetricKind.CUMULATIVE series.metric.type = "custom.googleapis.com/OpenTelemetry/name" point = { "interval": { diff --git a/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring_exporter_integration.py b/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring_exporter_integration.py index 0aa8e166..8d3e5052 100644 --- a/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring_exporter_integration.py +++ b/opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring_exporter_integration.py @@ -13,23 +13,25 @@ # limitations under the License. +import logging +from unittest.mock import MagicMock, patch + import grpc from google.cloud.monitoring_v3 import MetricServiceClient from google.cloud.monitoring_v3.gapic.transports import ( metric_service_grpc_transport, ) from opentelemetry.exporter.cloud_monitoring import ( - NANOS_PER_SECOND, - WRITE_INTERVAL, CloudMonitoringMetricsExporter, ) from opentelemetry.sdk import metrics -from opentelemetry.sdk.metrics.export import MetricRecord, MetricsExportResult -from opentelemetry.sdk.metrics.export.aggregate import SumAggregator +from opentelemetry.sdk.metrics.export.controller import PushController from opentelemetry.sdk.resources import Resource from test_common import BaseExporterIntegrationTest +logger = logging.getLogger(__name__) + class TestCloudMonitoringSpanExporter(BaseExporterIntegrationTest): def test_export(self): @@ -37,32 +39,44 @@ def test_export(self): transport = metric_service_grpc_transport.MetricServiceGrpcTransport( channel=channel ) + client = MagicMock(wraps=MetricServiceClient(transport=transport)) exporter = CloudMonitoringMetricsExporter( - self.project_id, client=MetricServiceClient(transport=transport) + self.project_id, client=client ) - meter = metrics.MeterProvider().get_meter(__name__) + meter_provider = metrics.MeterProvider( + resource=Resource.create( + { + "cloud.account.id": "some_account_id", + "cloud.provider": "gcp", + "cloud.zone": "us-east1-b", + "host.id": 654321, + "gcp.resource_type": "gce_instance", + } + ) + ) + meter = meter_provider.get_meter(__name__) counter = meter.create_metric( - name="name", + # TODO: "opentelemetry/" prefix is a hack for + # https://github.com/googleinterns/cloud-operations-api-mock/issues/56 + name="opentelemetry/name", description="desc", unit="1", value_type=int, metric_type=metrics.Counter, ) + # interval doesn't matter, we don't start the thread and just run + # tick() instead + controller = PushController(meter, exporter, 10) - sum_agg = SumAggregator() - sum_agg.checkpoint = 1 - sum_agg.last_update_timestamp = (WRITE_INTERVAL + 2) * NANOS_PER_SECOND + counter.add(10, {"env": "test"}) - result = exporter.export( - [ - MetricRecord( - counter, - labels=(), - aggregator=sum_agg, - resource=Resource.create_empty(), - ) - ] - ) + with patch( + "opentelemetry.exporter.cloud_monitoring.logger" + ) as mock_logger: + controller.tick() - self.assertEqual(result, MetricsExportResult.SUCCESS) + # run tox tests with `-- -log-cli-level=0` to see mock calls made + logger.debug(client.create_time_series.mock_calls) + mock_logger.warning.assert_not_called() + mock_logger.error.assert_not_called() diff --git a/test-common/src/test_common/base_exporter_integration_test.py b/test-common/src/test_common/base_exporter_integration_test.py index 85ac430d..7be82915 100644 --- a/test-common/src/test_common/base_exporter_integration_test.py +++ b/test-common/src/test_common/base_exporter_integration_test.py @@ -16,12 +16,28 @@ def setUp(self) -> None: # Start the mock server. args = ["mock_server", "-address", self.address] self.mock_server_process = subprocess.Popen( - args, stderr=subprocess.PIPE + args, stderr=subprocess.PIPE, stdout=subprocess.PIPE ) # Block until the mock server starts (it will output the address after starting). - if self.mock_server_process.stderr is None: - raise RuntimeError("stderr is None") + if ( + self.mock_server_process.stderr is None + or self.mock_server_process.stdout is None + ): + raise RuntimeError("stderr or stdout is None") self.mock_server_process.stderr.readline() def tearDown(self) -> None: self.mock_server_process.kill() + if ( + self.mock_server_process.stderr is None + or self.mock_server_process.stdout is None + ): + raise RuntimeError("stderr or stdout is None") + stdout = self.mock_server_process.stdout.read().decode() + stderr = self.mock_server_process.stderr.read().decode() + if stderr or stdout: + self.fail( + "Mock server should not have had any output, got stdout:\n%s\n\nstderr:\n%s", + stdout, + stderr, + ) diff --git a/tox.ini b/tox.ini index 0fcc37bc..22a8c8d5 100644 --- a/tox.ini +++ b/tox.ini @@ -68,12 +68,14 @@ changedir = test-exporter: opentelemetry-exporter-google-cloud test-tools: opentelemetry-tools-google-cloud +passenv = SKIP_GET_MOCK_SERVER + commands_pre = test: pip install . test: {toxinidir}/get_mock_server.sh {envbindir} commands = - test: pytest --junitxml={[constants]test_results_dir}/{envname}/junit.xml + test: pytest --junitxml={[constants]test_results_dir}/{envname}/junit.xml {posargs} whitelist_externals = bash