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

fixed more 0.14b0 breakages and updated tests to catch them #83

Merged
merged 1 commit into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion get_mock_server.sh
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -7,4 +13,3 @@ if ! [ -e $1/$BINARY ]; then
fi

ln -sf $1/$BINARY $1/mock_server

4 changes: 4 additions & 0 deletions opentelemetry-exporter-google-cloud/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

- Fix breakages for opentelemetry-python v0.14b0
([#79](https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/pull/79),
[#83](https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/pull/83))

## Version 0.13b0

Released 2020-09-17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
# https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/issues/84
metric_kind=metric_descriptor.metric_kind,
)
series.metric.type = metric_descriptor.type
for key, value in record.labels:
Expand Down
32 changes: 21 additions & 11 deletions opentelemetry-exporter-google-cloud/tests/test_cloud_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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,
),
]
)
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -432,7 +440,7 @@ def test_export_value_observer(self):
exporter.export(
[
MetricRecord(
MockMetric(meter=MockMeter()),
MockMetric(meter=mock_meter()),
(),
aggregator,
Resource.create_empty(),
Expand All @@ -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
Expand Down Expand Up @@ -485,7 +494,7 @@ def test_export_histogram(self):
exporter.export(
[
MetricRecord(
MockMetric(meter=MockMeter()),
MockMetric(meter=mock_meter()),
(),
aggregator,
Resource.create_empty(),
Expand All @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,56 +13,70 @@
# 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):
channel = grpc.insecure_channel(self.address)
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",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we decided to keep this one? It's incompatible with semantic conventions, but might be okay as an additional attribute.

Also, IIRC you shared a link where AWS has introduced attributes / resources specific to AWS to the spec? Shall we try doing the same if we want to support GCP-specific resource attributes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Have we decided to keep this one? It's incompatible with semantic conventions, but might be okay as an additional attribute.

I think it's better to use the semantic conventions if we aren't missing any functionality #85. Do you have any preference on adding our own GCP resource attributes instead? (Here's the AWS PR for reference open-telemetry/opentelemetry-specification#1099))

}
)
)
meter = meter_provider.get_meter(__name__)
counter = meter.create_metric(
name="name",
# TODO: remove "opentelemetry/" prefix which is a hack
# https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/issues/84
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()
3 changes: 3 additions & 0 deletions opentelemetry-tools-google-cloud/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- Fix breakages for opentelemetry-python v0.14b0
([#79](https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/pull/79))

## Version 0.13b0

Released 2020-09-17
Expand Down
22 changes: 19 additions & 3 deletions test-common/src/test_common/base_exporter_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
4 changes: 3 additions & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down