From 499ba1e263addbf9e6b0763ad8802c286f1c2310 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 31 Jan 2023 11:31:53 -0800 Subject: [PATCH 1/5] fix: properly handle None from metadata server --- .../logging_v2/handlers/_monitored_resources.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_monitored_resources.py b/google/cloud/logging_v2/handlers/_monitored_resources.py index 144258749..a5b8dfee3 100644 --- a/google/cloud/logging_v2/handlers/_monitored_resources.py +++ b/google/cloud/logging_v2/handlers/_monitored_resources.py @@ -71,8 +71,8 @@ def _create_functions_resource(): resource = Resource( type="cloud_function", labels={ - "project_id": project, - "function_name": function_name, + "project_id": project if project else "", + "function_name": function_name if function_name else "", "region": region.split("/")[-1] if region else "", }, ) @@ -91,7 +91,7 @@ def _create_kubernetes_resource(): resource = Resource( type="k8s_container", labels={ - "project_id": project, + "project_id": project if project else "", "location": zone if zone else "", "cluster_name": cluster_name if cluster_name else "", }, @@ -110,7 +110,7 @@ def _create_compute_resource(): resource = Resource( type="gce_instance", labels={ - "project_id": project, + "project_id": project if project else "", "instance_id": instance if instance else "", "zone": zone if zone else "", }, @@ -128,7 +128,7 @@ def _create_cloud_run_resource(): resource = Resource( type="cloud_run_revision", labels={ - "project_id": project, + "project_id": project if project else "", "service_name": os.environ.get(_CLOUD_RUN_SERVICE_ID, ""), "revision_name": os.environ.get(_CLOUD_RUN_REVISION_ID, ""), "location": region.split("/")[-1] if region else "", @@ -148,7 +148,7 @@ def _create_app_engine_resource(): resource = Resource( type="gae_app", labels={ - "project_id": project, + "project_id": project if project else "", "module_id": os.environ.get(_GAE_SERVICE_ENV, ""), "version_id": os.environ.get(_GAE_VERSION_ENV, ""), "zone": zone if zone else "", @@ -164,7 +164,7 @@ def _create_global_resource(project): Returns: google.cloud.logging.Resource """ - return Resource(type="global", labels={"project_id": project}) + return Resource(type="global", labels={"project_id": project if project else ""}) def detect_resource(project=""): From e82f9d5f188474f414048e87fc64376a683613ce Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Feb 2023 13:49:56 -0800 Subject: [PATCH 2/5] added unit tests --- .../handlers/test__monitored_resources.py | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/unit/handlers/test__monitored_resources.py b/tests/unit/handlers/test__monitored_resources.py index 5acced157..7979d7ef0 100644 --- a/tests/unit/handlers/test__monitored_resources.py +++ b/tests/unit/handlers/test__monitored_resources.py @@ -16,7 +16,7 @@ import mock import os - +import functools from google.cloud.logging_v2.handlers._monitored_resources import ( _create_functions_resource, @@ -66,6 +66,20 @@ def _mock_metadata(self, endpoint): else: return None + def _mock_metadata_no_project(self, endpoint): + if ( + endpoint == _monitored_resources._ZONE_ID + or endpoint == _monitored_resources._REGION_ID + ): + return self.LOCATION + elif ( + endpoint == _monitored_resources._GKE_CLUSTER_NAME + or endpoint == _monitored_resources._GCE_INSTANCE_ID + ): + return self.NAME + else: + return None + def setUp(self): os.environ.clear() @@ -100,6 +114,19 @@ def test_create_modern_functions_resource(self): self.assertEqual(func_resource.labels["function_name"], self.NAME) self.assertEqual(func_resource.labels["region"], self.LOCATION) + def test_functions_resource_no_name(self): + patch = mock.patch( + "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", + wraps=self._mock_metadata_no_project, + ) + with patch: + func_resource = _create_functions_resource() + + self.assertIsInstance(func_resource, Resource) + self.assertEqual(func_resource.type, "cloud_function") + self.assertEqual(func_resource.labels["project_id"], "") + self.assertEqual(func_resource.labels["function_name"], "") + def test_create_kubernetes_resource(self): patch = mock.patch( @@ -169,6 +196,21 @@ def test_global_resource(self): self.assertEqual(resource.type, "global") self.assertEqual(resource.labels["project_id"], self.PROJECT) + def test_with_no_project_from_server(self): + """ + Ensure project_id uses an empty string if not known + https://github.com/googleapis/python-logging/issues/710 + """ + patch = mock.patch( + "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", + wraps=self._mock_metadata_no_project, + ) + with patch: + _global_resource_patched = functools.partial(_create_global_resource, None) + resource_fns = [_global_resource_patched, _create_app_engine_resource, _create_cloud_run_resource, _create_compute_resource, _create_kubernetes_resource, _create_functions_resource] + for fn in resource_fns: + resource = fn() + self.assertEqual(resource.labels["project_id"], "") class Test_Resource_Detection(unittest.TestCase): @@ -189,6 +231,14 @@ def _mock_gce_metadata(self, endpoint): else: return None + def _mock_partial_metadata(self, endpoint): + if endpoint == _monitored_resources._ZONE_ID: + return "ZONE" + elif endpoint == _monitored_resources._GCE_INSTANCE_ID: + return "instance" + else: + return None + def setUp(self): os.environ.clear() @@ -249,3 +299,19 @@ def test_detection_unknown(self): resource = detect_resource(self.PROJECT) self.assertIsInstance(resource, Resource) self.assertEqual(resource.type, "global") + + def test_detect_partial_data(self): + """ + Test if the metadata server retrus partial data + """ + patch = mock.patch( + "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", + wraps=self._mock_partial_metadata, + ) + with patch: + resource = detect_resource(self.PROJECT) + self.assertIsInstance(resource, Resource) + self.assertEqual(resource.type, "gce_instance") + # project id not returned from metadata serve + # should be empty string + self.assertEqual(resource.labels["project_id"], "") From 1844a18572e3f2b57f834f481e6adf5a586a437a Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Feb 2023 13:51:51 -0800 Subject: [PATCH 3/5] ran blacken --- tests/unit/handlers/test__monitored_resources.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/unit/handlers/test__monitored_resources.py b/tests/unit/handlers/test__monitored_resources.py index 7979d7ef0..13bb51c26 100644 --- a/tests/unit/handlers/test__monitored_resources.py +++ b/tests/unit/handlers/test__monitored_resources.py @@ -115,6 +115,10 @@ def test_create_modern_functions_resource(self): self.assertEqual(func_resource.labels["region"], self.LOCATION) def test_functions_resource_no_name(self): + """ + Simulate functions environment with function name returned as None + https://github.com/googleapis/python-logging/pull/718 + """ patch = mock.patch( "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", wraps=self._mock_metadata_no_project, @@ -207,11 +211,19 @@ def test_with_no_project_from_server(self): ) with patch: _global_resource_patched = functools.partial(_create_global_resource, None) - resource_fns = [_global_resource_patched, _create_app_engine_resource, _create_cloud_run_resource, _create_compute_resource, _create_kubernetes_resource, _create_functions_resource] + resource_fns = [ + _global_resource_patched, + _create_app_engine_resource, + _create_cloud_run_resource, + _create_compute_resource, + _create_kubernetes_resource, + _create_functions_resource, + ] for fn in resource_fns: resource = fn() self.assertEqual(resource.labels["project_id"], "") + class Test_Resource_Detection(unittest.TestCase): PROJECT = "test-project" From 962a2eafcf51f72af54659ccf9201f7f0dbffb27 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Wed, 1 Feb 2023 21:52:12 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20po?= =?UTF-8?q?st-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- tests/unit/handlers/test__monitored_resources.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unit/handlers/test__monitored_resources.py b/tests/unit/handlers/test__monitored_resources.py index 7979d7ef0..e28ac1808 100644 --- a/tests/unit/handlers/test__monitored_resources.py +++ b/tests/unit/handlers/test__monitored_resources.py @@ -207,11 +207,19 @@ def test_with_no_project_from_server(self): ) with patch: _global_resource_patched = functools.partial(_create_global_resource, None) - resource_fns = [_global_resource_patched, _create_app_engine_resource, _create_cloud_run_resource, _create_compute_resource, _create_kubernetes_resource, _create_functions_resource] + resource_fns = [ + _global_resource_patched, + _create_app_engine_resource, + _create_cloud_run_resource, + _create_compute_resource, + _create_kubernetes_resource, + _create_functions_resource, + ] for fn in resource_fns: resource = fn() self.assertEqual(resource.labels["project_id"], "") + class Test_Resource_Detection(unittest.TestCase): PROJECT = "test-project" From 78b2fe2343acc9f36b86b1d0b54a3564fa000a31 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 1 Feb 2023 13:53:32 -0800 Subject: [PATCH 5/5] typo fix --- tests/unit/handlers/test__monitored_resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/handlers/test__monitored_resources.py b/tests/unit/handlers/test__monitored_resources.py index 13bb51c26..3c62cba88 100644 --- a/tests/unit/handlers/test__monitored_resources.py +++ b/tests/unit/handlers/test__monitored_resources.py @@ -314,7 +314,7 @@ def test_detection_unknown(self): def test_detect_partial_data(self): """ - Test if the metadata server retrus partial data + Test case where the metadata server returns partial data """ patch = mock.patch( "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server",