Skip to content

Add GCP test and coverage targets#804

Merged
toumorokoshi merged 11 commits intoopen-telemetry:masterfrom
AndrewAXue:cloud_montoring_tests
Jun 11, 2020
Merged

Add GCP test and coverage targets#804
toumorokoshi merged 11 commits intoopen-telemetry:masterfrom
AndrewAXue:cloud_montoring_tests

Conversation

@AndrewAXue
Copy link
Contributor

@AndrewAXue AndrewAXue commented Jun 10, 2020

I forgot to add the tests for Cloud Monitoring exporter into tox.ini :/

@AndrewAXue AndrewAXue requested a review from a team June 10, 2020 19:52
@AndrewAXue AndrewAXue changed the title add test coverage add test coverage for Cloud Monitoring exporter Jun 10, 2020
Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

I'm seeing test failures still:

% pytest ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py -k test_get_metric_descriptor
ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py::TestCloudMonitoringMetricsExporter::test_get_metric_descriptor FAILED       [100%]

========================================================================== FAILURES ===========================================================================
________________________________________________ TestCloudMonitoringMetricsExporter.test_get_metric_descriptor ________________________________________________

self = <tests.test_cloud_monitoring.TestCloudMonitoringMetricsExporter testMethod=test_get_metric_descriptor>

    def test_get_metric_descriptor(self):
        client = mock.Mock()
        exporter = CloudMonitoringMetricsExporter(client=client)
        exporter.project_name = self.project_name

        self.assertIsNone(
>           exporter._get_metric_descriptor(
                MetricRecord(MockMetric(), (), UnsupportedAggregator())
            )
        )

ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py:107:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <opentelemetry.exporter.cloud_monitoring.CloudMonitoringMetricsExporter object at 0x10b042310>
record = <opentelemetry.sdk.metrics.export.MetricRecord object at 0x109723d90>

    def _get_metric_descriptor(
        self, record: MetricRecord
    ) -> Optional[MetricDescriptor]:
        """ We can map Metric to MetricDescriptor using Metric.name or
        MetricDescriptor.type. We create the MetricDescriptor if it doesn't
        exist already and cache it. Note that recreating MetricDescriptors is
        a no-op if it already exists.

        :param record:
        :return:
        """
>       instrument = record.instrument
E       AttributeError: 'MetricRecord' object has no attribute 'instrument'

ext/opentelemetry-exporter-cloud-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py:70: AttributeError

Also a warning about using app default credentials, looks like some calls to GCP aren't getting mocked?

@AndrewAXue AndrewAXue force-pushed the cloud_montoring_tests branch from d55f780 to 43c797e Compare June 10, 2020 23:49
@AndrewAXue
Copy link
Contributor Author

I'm seeing test failures still:

% pytest ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py -k test_get_metric_descriptor
ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py::TestCloudMonitoringMetricsExporter::test_get_metric_descriptor FAILED       [100%]

========================================================================== FAILURES ===========================================================================
________________________________________________ TestCloudMonitoringMetricsExporter.test_get_metric_descriptor ________________________________________________

self = <tests.test_cloud_monitoring.TestCloudMonitoringMetricsExporter testMethod=test_get_metric_descriptor>

    def test_get_metric_descriptor(self):
        client = mock.Mock()
        exporter = CloudMonitoringMetricsExporter(client=client)
        exporter.project_name = self.project_name

        self.assertIsNone(
>           exporter._get_metric_descriptor(
                MetricRecord(MockMetric(), (), UnsupportedAggregator())
            )
        )

ext/opentelemetry-exporter-cloud-monitoring/tests/test_cloud_monitoring.py:107:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <opentelemetry.exporter.cloud_monitoring.CloudMonitoringMetricsExporter object at 0x10b042310>
record = <opentelemetry.sdk.metrics.export.MetricRecord object at 0x109723d90>

    def _get_metric_descriptor(
        self, record: MetricRecord
    ) -> Optional[MetricDescriptor]:
        """ We can map Metric to MetricDescriptor using Metric.name or
        MetricDescriptor.type. We create the MetricDescriptor if it doesn't
        exist already and cache it. Note that recreating MetricDescriptors is
        a no-op if it already exists.

        :param record:
        :return:
        """
>       instrument = record.instrument
E       AttributeError: 'MetricRecord' object has no attribute 'instrument'

ext/opentelemetry-exporter-cloud-monitoring/src/opentelemetry/exporter/cloud_monitoring/__init__.py:70: AttributeError

Also a warning about using app default credentials, looks like some calls to GCP aren't getting mocked?

The calls to GCP were getting mocked, but theres another thing we do
_, self.project_id = google.auth.default()
which is what prints that message. I've removed this call.

@toumorokoshi
Copy link
Member

@c24t want to give a quick final pass and we'll merge?

Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @AndrewAXue!

@toumorokoshi toumorokoshi merged commit ba2e62d into open-telemetry:master Jun 11, 2020
@c24t c24t changed the title add test coverage for Cloud Monitoring exporter Add GCP test and coverage targets Jun 11, 2020
@AndrewAXue AndrewAXue deleted the cloud_montoring_tests branch June 11, 2020 21:56
@AndrewAXue AndrewAXue restored the cloud_montoring_tests branch June 16, 2020 15:10
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
* chore: remove binary format

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants