From 3761a8f2d3a04fc738d9441bad66174877bdd86d Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 21 Feb 2019 16:54:45 -0800 Subject: [PATCH 1/4] Merge lists of resources --- opencensus/common/resource.py | 32 ++++++++++++++++++++---------- tests/unit/common/test_resource.py | 15 ++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/opencensus/common/resource.py b/opencensus/common/resource.py index 7c036fa94..e1e3fb33e 100644 --- a/opencensus/common/resource.py +++ b/opencensus/common/resource.py @@ -49,22 +49,32 @@ _UNQUOTE_RE = re.compile(r'^([\'"]?)([^\1]*)(\1)$') -def merge_resources(r1, r2): - """Merge two resources to get a new resource. +def merge_resources(resource_list): + """Merge multiple resources to get a new resource. - :type r1: :class:`Resource` - :param r1: The first resource to merge, takes priority in conflicts. + Resources earlier in the list take precedence: if multiple resources share + a label key, use the value from the first resource in the list with that + key. The combined resource's type will be the first non-null type in the + list. - :type r2: :class:`Resource` - :param r2: The second resource to merge. + :type resource_list: list(:class:`Resource`) + :param resource_list: The list of resources to combine. :rtype: :class:`Resource` :return: The new combined resource. """ - type_ = r1.type or r2.type - labels = copy(r2.labels) - labels.update(r1.labels) - return Resource(type_, labels) + if not resource_list: + raise ValueError + rtype = None + for rr in resource_list: + if rr.type: + rtype = rr.type + break + last, rest = resource_list[::-1][0], resource_list[::-1][1:] + labels = copy(last.labels) + for rr in rest: + labels.update(rr.labels) + return Resource(rtype, labels) def check_ascii_256(string): @@ -150,7 +160,7 @@ def merge(self, other): :rtype: :class:`Resource` :return: The new combined resource. """ - return merge_resources(self, other) + return merge_resources([self, other]) def unquote(string): diff --git a/tests/unit/common/test_resource.py b/tests/unit/common/test_resource.py index 729da9979..de797249c 100644 --- a/tests/unit/common/test_resource.py +++ b/tests/unit/common/test_resource.py @@ -123,6 +123,21 @@ def test_merge_overwrite(self): class TestResourceModule(unittest.TestCase): + def test_merge_resource(self): + with self.assertRaises(ValueError): + resource_module.merge_resources(None) + with self.assertRaises(ValueError): + resource_module.merge_resources([]) + + r1 = Resource(None, {'lk1': 'lv11'}) + r2 = Resource('t2', {'lk1': 'lv12', 'lk2': 'lv22'}) + r3 = Resource('t3', {'lk2': 'lv23', 'lk3': 'lv33'}) + + merged = resource_module.merge_resources([r1, r2, r3]) + self.assertEqual(merged.type, 't2') + self.assertDictEqual( + merged.labels, {'lk1': 'lv11', 'lk2': 'lv22', 'lk3': 'lv33'}) + def test_check_ascii_256(self): self.assertIsNone(resource_module.check_ascii_256(None)) From 37206dda8ebd42c5910727adba2733a1f9050a94 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 21 Feb 2019 17:45:51 -0800 Subject: [PATCH 2/4] Let custom resource override detected resources --- .../monitored_resource/monitored_resource.py | 24 ++-- .../test_monitored_resource.py | 110 +++++++++++++++--- tests/unit/common/test_resource.py | 16 +++ 3 files changed, 124 insertions(+), 26 deletions(-) diff --git a/opencensus/common/monitored_resource/monitored_resource.py b/opencensus/common/monitored_resource/monitored_resource.py index 7dd062cb0..1bd2d2bc8 100644 --- a/opencensus/common/monitored_resource/monitored_resource.py +++ b/opencensus/common/monitored_resource/monitored_resource.py @@ -64,18 +64,26 @@ def get_instance(): :rtype: :class:`opencensus.common.resource.Resource` or None :return: A `Resource` configured for the current environment. """ + resources = [] + env_resource = resource.get_from_env() + if env_resource is not None: + resources.append(env_resource) + if is_gke_environment(): - return resource.Resource( + resources.append(resource.Resource( _GKE_CONTAINER, - gcp_metadata_config.GcpMetadataConfig().get_gke_metadata()) + gcp_metadata_config.GcpMetadataConfig().get_gke_metadata())) + if is_gce_environment(): - return resource.Resource( + resources.append(resource.Resource( _GCE_INSTANCE, - gcp_metadata_config.GcpMetadataConfig().get_gce_metadata()) - if is_aws_environment(): - return resource.Resource( + gcp_metadata_config.GcpMetadataConfig().get_gce_metadata())) + elif is_aws_environment(): + resources.append(resource.Resource( _AWS_EC2_INSTANCE, (aws_identity_doc_utils.AwsIdentityDocumentUtils() - .get_aws_metadata())) + .get_aws_metadata()))) - return None + if not resources: + return None + return resource.merge_resources(resources) diff --git a/tests/unit/common/monitored_resource_util/test_monitored_resource.py b/tests/unit/common/monitored_resource_util/test_monitored_resource.py index 4f158bd1d..a03ff9dcd 100644 --- a/tests/unit/common/monitored_resource_util/test_monitored_resource.py +++ b/tests/unit/common/monitored_resource_util/test_monitored_resource.py @@ -39,6 +39,27 @@ def mock_use_aws(use): return mock_mr_method('is_aws_environment', use) +@contextmanager +def no_oc_env(): + with mock.patch.dict(os.environ): + try: + del os.environ['OC_RESOURCE_TYPE'] + except KeyError: + pass + try: + del os.environ['OC_RESOURCE_LABELS'] + except KeyError: + pass + yield + + +def mock_oc_env(): + return mock.patch.dict('os.environ', { + 'OC_RESOURCE_TYPE': 'mock_resource_type', + 'OC_RESOURCE_LABELS': 'mock_label_key=mock_label_value' + }) + + @contextmanager def mock_gke_env(): with mock_use_gke(True): @@ -77,11 +98,20 @@ def test_gcp_gce_monitored_resource(self, gcp_metadata_mock): gcp_metadata_mock.return_value = mock.Mock() gcp_metadata_mock.return_value.get_gce_metadata.return_value =\ mocked_labels - with mock_gce_env(): - resource = monitored_resource.get_instance() + with no_oc_env(): + with mock_gce_env(): + resource = monitored_resource.get_instance() self.assertEquals(resource.get_type(), 'gce_instance') self.assertEquals(resource.get_labels(), mocked_labels) + with mock_oc_env(): + with mock_gce_env(): + resource = monitored_resource.get_instance() + self.assertEqual(resource.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, resource.get_labels()) + self.assertDictContainsSubset(mocked_labels, resource.get_labels()) + @mock.patch('opencensus.common.monitored_resource.monitored_resource' '.gcp_metadata_config.GcpMetadataConfig') def test_gcp_gke_monitored_resource(self, gcp_metadata_mock): @@ -99,11 +129,20 @@ def test_gcp_gke_monitored_resource(self, gcp_metadata_mock): gcp_metadata_mock.return_value = mock.Mock() gcp_metadata_mock.return_value.get_gke_metadata.return_value =\ mocked_labels - with mock_gke_env(): - resource = monitored_resource.get_instance() + with no_oc_env(): + with mock_gke_env(): + resource = monitored_resource.get_instance() self.assertEquals(resource.get_type(), 'gke_container') self.assertEquals(resource.get_labels(), mocked_labels) + with mock_oc_env(): + with mock_gke_env(): + resource = monitored_resource.get_instance() + self.assertEqual(resource.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, resource.get_labels()) + self.assertDictContainsSubset(mocked_labels, resource.get_labels()) + @mock.patch('opencensus.common.monitored_resource.monitored_resource' '.aws_identity_doc_utils.AwsIdentityDocumentUtils') def test_aws_monitored_resource(self, aws_metadata_mock): @@ -118,20 +157,35 @@ def test_aws_monitored_resource(self, aws_metadata_mock): aws_metadata_mock.return_value.get_aws_metadata.return_value =\ mocked_labels - with mock_aws_env(): - resource = monitored_resource.get_instance() + with no_oc_env(): + with mock_aws_env(): + resource = monitored_resource.get_instance() self.assertEquals(resource.get_type(), 'aws_ec2_instance') self.assertEquals(resource.get_labels(), mocked_labels) + with mock_oc_env(): + with mock_aws_env(): + resource = monitored_resource.get_instance() + self.assertEqual(resource.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, resource.get_labels()) + self.assertDictContainsSubset(mocked_labels, resource.get_labels()) + def test_gke_environment(self): patch = mock.patch.dict(os.environ, {'KUBERNETES_SERVICE_HOST': '127.0.0.1'}) - with patch: - mr = monitored_resource.get_instance() + with no_oc_env(): + with patch: + mr = monitored_resource.get_instance() + self.assertIsNotNone(mr) + self.assertEqual(mr.get_type(), "gke_container") - self.assertIsNotNone(mr) - self.assertEqual(mr.get_type(), "gke_container") + with mock_oc_env(): + mr = monitored_resource.get_instance() + self.assertEqual(mr.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, mr.get_labels()) def test_gce_environment(self): patch = mock.patch( @@ -139,11 +193,18 @@ def test_gce_environment(self): 'gcp_metadata_config.GcpMetadataConfig.' 'is_running_on_gcp', return_value=True) - with patch: - mr = monitored_resource.get_instance() + with no_oc_env(): + with patch: + mr = monitored_resource.get_instance() - self.assertIsNotNone(mr) - self.assertEqual(mr.get_type(), "gce_instance") + self.assertIsNotNone(mr) + self.assertEqual(mr.get_type(), "gce_instance") + + with mock_oc_env(): + mr = monitored_resource.get_instance() + self.assertEqual(mr.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, mr.get_labels()) @mock.patch('opencensus.common.monitored_resource.' 'gcp_metadata_config.GcpMetadataConfig.is_running_on_gcp', @@ -153,11 +214,17 @@ def test_gce_environment(self): 'is_running_on_aws', return_value=True) def test_aws_environment(self, aws_util_mock, gcp_metadata_mock): - mr = monitored_resource.get_instance() - + with no_oc_env(): + mr = monitored_resource.get_instance() self.assertIsNotNone(mr) self.assertEqual(mr.get_type(), "aws_ec2_instance") + with mock_oc_env(): + mr = monitored_resource.get_instance() + self.assertEqual(mr.get_type(), 'mock_resource_type') + self.assertDictContainsSubset( + {'mock_label_key': 'mock_label_value'}, mr.get_labels()) + @mock.patch('opencensus.common.monitored_resource.' 'gcp_metadata_config.GcpMetadataConfig.is_running_on_gcp', return_value=False) @@ -166,6 +233,13 @@ def test_aws_environment(self, aws_util_mock, gcp_metadata_mock): 'is_running_on_aws', return_value=False) def test_non_supported_environment(self, aws_util_mock, gcp_metadata_mock): - mr = monitored_resource.get_instance() - + with no_oc_env(): + mr = monitored_resource.get_instance() self.assertIsNone(mr) + + with mock_oc_env(): + mr = monitored_resource.get_instance() + self.assertIsNotNone(mr) + self.assertEqual(mr.get_type(), 'mock_resource_type') + self.assertDictEqual( + mr.get_labels(), {'mock_label_key': 'mock_label_value'}) diff --git a/tests/unit/common/test_resource.py b/tests/unit/common/test_resource.py index de797249c..dc9122921 100644 --- a/tests/unit/common/test_resource.py +++ b/tests/unit/common/test_resource.py @@ -19,6 +19,7 @@ except ImportError: from unittest import mock +import os import unittest from opencensus.common import resource as resource_module @@ -138,6 +139,13 @@ def test_merge_resource(self): self.assertDictEqual( merged.labels, {'lk1': 'lv11', 'lk2': 'lv22', 'lk3': 'lv33'}) + def test_merge_resource_no_type(self): + r1 = Resource(None) + r2 = Resource(None) + + merged = resource_module.merge_resources([r1, r2]) + self.assertEqual(merged.type, None) + def test_check_ascii_256(self): self.assertIsNone(resource_module.check_ascii_256(None)) @@ -171,12 +179,20 @@ def test_get_from_env_no_type(self): with mock.patch.dict('os.environ', { 'OC_RESOURCE_LABELS': 'k1=v1,k2=v2' }): + try: + del os.environ['OC_RESOURCE_TYPE'] + except KeyError: + pass self.assertIsNone(resource_module.get_from_env()) def test_get_from_env_no_labels(self): with mock.patch.dict('os.environ', { 'OC_RESOURCE_TYPE': 'opencensus.io/example', }): + try: + del os.environ['OC_RESOURCE_LABELS'] + except KeyError: + pass resource = resource_module.get_from_env() self.assertEqual(resource.type, 'opencensus.io/example') self.assertDictEqual(resource.labels, {}) From e32e009d6e1a9b0d278edf479ee919cb8d13042a Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 21 Feb 2019 17:47:01 -0800 Subject: [PATCH 3/4] Delete redundant tests --- .../test_monitored_resource.py | 54 ------------------- 1 file changed, 54 deletions(-) diff --git a/tests/unit/common/monitored_resource_util/test_monitored_resource.py b/tests/unit/common/monitored_resource_util/test_monitored_resource.py index a03ff9dcd..69e0cf2a3 100644 --- a/tests/unit/common/monitored_resource_util/test_monitored_resource.py +++ b/tests/unit/common/monitored_resource_util/test_monitored_resource.py @@ -171,60 +171,6 @@ def test_aws_monitored_resource(self, aws_metadata_mock): {'mock_label_key': 'mock_label_value'}, resource.get_labels()) self.assertDictContainsSubset(mocked_labels, resource.get_labels()) - def test_gke_environment(self): - patch = mock.patch.dict(os.environ, - {'KUBERNETES_SERVICE_HOST': '127.0.0.1'}) - - with no_oc_env(): - with patch: - mr = monitored_resource.get_instance() - self.assertIsNotNone(mr) - self.assertEqual(mr.get_type(), "gke_container") - - with mock_oc_env(): - mr = monitored_resource.get_instance() - self.assertEqual(mr.get_type(), 'mock_resource_type') - self.assertDictContainsSubset( - {'mock_label_key': 'mock_label_value'}, mr.get_labels()) - - def test_gce_environment(self): - patch = mock.patch( - 'opencensus.common.monitored_resource.' - 'gcp_metadata_config.GcpMetadataConfig.' - 'is_running_on_gcp', - return_value=True) - with no_oc_env(): - with patch: - mr = monitored_resource.get_instance() - - self.assertIsNotNone(mr) - self.assertEqual(mr.get_type(), "gce_instance") - - with mock_oc_env(): - mr = monitored_resource.get_instance() - self.assertEqual(mr.get_type(), 'mock_resource_type') - self.assertDictContainsSubset( - {'mock_label_key': 'mock_label_value'}, mr.get_labels()) - - @mock.patch('opencensus.common.monitored_resource.' - 'gcp_metadata_config.GcpMetadataConfig.is_running_on_gcp', - return_value=False) - @mock.patch('opencensus.common.monitored_resource.' - 'aws_identity_doc_utils.AwsIdentityDocumentUtils.' - 'is_running_on_aws', - return_value=True) - def test_aws_environment(self, aws_util_mock, gcp_metadata_mock): - with no_oc_env(): - mr = monitored_resource.get_instance() - self.assertIsNotNone(mr) - self.assertEqual(mr.get_type(), "aws_ec2_instance") - - with mock_oc_env(): - mr = monitored_resource.get_instance() - self.assertEqual(mr.get_type(), 'mock_resource_type') - self.assertDictContainsSubset( - {'mock_label_key': 'mock_label_value'}, mr.get_labels()) - @mock.patch('opencensus.common.monitored_resource.' 'gcp_metadata_config.GcpMetadataConfig.is_running_on_gcp', return_value=False) From 6144ee0cf6798ecf1d48335e05eed471816722d7 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 21 Feb 2019 20:03:11 -0800 Subject: [PATCH 4/4] Clean up some ugly code --- opencensus/common/resource.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opencensus/common/resource.py b/opencensus/common/resource.py index e1e3fb33e..44f70ea2c 100644 --- a/opencensus/common/resource.py +++ b/opencensus/common/resource.py @@ -70,9 +70,8 @@ def merge_resources(resource_list): if rr.type: rtype = rr.type break - last, rest = resource_list[::-1][0], resource_list[::-1][1:] - labels = copy(last.labels) - for rr in rest: + labels = {} + for rr in reversed(resource_list): labels.update(rr.labels) return Resource(rtype, labels)