From 813b97cb936fa5acc2a4de567e2c84d746527e98 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 24 Mar 2021 13:06:00 -0700 Subject: [PATCH] fix: detect project from environment instead of from logger (#238) --- .../handlers/_monitored_resources.py | 46 ++++++++++--------- .../cloud/logging_v2/handlers/app_engine.py | 2 +- .../handlers/test__monitored_resources.py | 14 +++--- tests/unit/handlers/test_app_engine.py | 11 +++-- 4 files changed, 42 insertions(+), 31 deletions(-) diff --git a/google/cloud/logging_v2/handlers/_monitored_resources.py b/google/cloud/logging_v2/handlers/_monitored_resources.py index bd05c252..ad1de4d2 100644 --- a/google/cloud/logging_v2/handlers/_monitored_resources.py +++ b/google/cloud/logging_v2/handlers/_monitored_resources.py @@ -51,14 +51,16 @@ _GKE_CLUSTER_NAME = "instance/attributes/cluster-name" """Attribute in metadata server when in GKE environment.""" +_PROJECT_NAME = "project/project-id" +"""Attribute in metadata server when in GKE environment.""" + -def _create_functions_resource(project): +def _create_functions_resource(): """Create a standardized Cloud Functions resource. - Args: - project (str): The project ID to pass on to the resource Returns: google.cloud.logging.Resource """ + project = retrieve_metadata_server(_PROJECT_NAME) region = retrieve_metadata_server(_REGION_ID) if _FUNCTION_NAME in os.environ: function_name = os.environ.get(_FUNCTION_NAME) @@ -77,15 +79,14 @@ def _create_functions_resource(project): return resource -def _create_kubernetes_resource(project): +def _create_kubernetes_resource(): """Create a standardized Kubernetes resource. - Args: - project (str): The project ID to pass on to the resource Returns: google.cloud.logging.Resource """ zone = retrieve_metadata_server(_ZONE_ID) cluster_name = retrieve_metadata_server(_GKE_CLUSTER_NAME) + project = retrieve_metadata_server(_PROJECT_NAME) resource = Resource( type="k8s_container", @@ -98,15 +99,14 @@ def _create_kubernetes_resource(project): return resource -def _create_compute_resource(project): +def _create_compute_resource(): """Create a standardized Compute Engine resource. - Args: - project (str): The project ID to pass on to the resource Returns: google.cloud.logging.Resource """ instance = retrieve_metadata_server(_GCE_INSTANCE_ID) zone = retrieve_metadata_server(_ZONE_ID) + project = retrieve_metadata_server(_PROJECT_NAME) resource = Resource( type="gce_instance", labels={ @@ -118,14 +118,13 @@ def _create_compute_resource(project): return resource -def _create_cloud_run_resource(project): +def _create_cloud_run_resource(): """Create a standardized Cloud Run resource. - Args: - project (str): The project ID to pass on to the resource Returns: google.cloud.logging.Resource """ region = retrieve_metadata_server(_REGION_ID) + project = retrieve_metadata_server(_PROJECT_NAME) resource = Resource( type="cloud_run_revision", labels={ @@ -139,14 +138,13 @@ def _create_cloud_run_resource(project): return resource -def _create_app_engine_resource(project): +def _create_app_engine_resource(): """Create a standardized App Engine resource. - Args: - project (str): The project ID to pass on to the resource Returns: google.cloud.logging.Resource """ zone = retrieve_metadata_server(_ZONE_ID) + project = retrieve_metadata_server(_PROJECT_NAME) resource = Resource( type="gae_app", labels={ @@ -160,13 +158,19 @@ def _create_app_engine_resource(project): def _create_global_resource(project): + """Create a global resource. + Args: + project (str): The project ID to pass on to the resource + Returns: + google.cloud.logging.Resource + """ return Resource(type="global", labels={"project_id": project}) def detect_resource(project=""): """Return the default monitored resource based on the local environment. Args: - project (str): The project ID to pass on to the resource + project (str): The project ID to pass on to the resource (if needed) Returns: google.cloud.logging.Resource: The default resource based on the environment """ @@ -175,21 +179,21 @@ def detect_resource(project=""): if all([env in os.environ for env in _GAE_ENV_VARS]): # App Engine Flex or Standard - return _create_app_engine_resource(project) + return _create_app_engine_resource() elif gke_cluster_name is not None: # Kubernetes Engine - return _create_kubernetes_resource(project) + return _create_kubernetes_resource() elif all([env in os.environ for env in _LEGACY_FUNCTION_ENV_VARS]) or all( [env in os.environ for env in _FUNCTION_ENV_VARS] ): # Cloud Functions - return _create_functions_resource(project) + return _create_functions_resource() elif all([env in os.environ for env in _CLOUD_RUN_ENV_VARS]): # Cloud Run - return _create_cloud_run_resource(project) + return _create_cloud_run_resource() elif gce_instance_name is not None: # Compute Engine - return _create_compute_resource(project) + return _create_compute_resource() else: # use generic global resource return _create_global_resource(project) diff --git a/google/cloud/logging_v2/handlers/app_engine.py b/google/cloud/logging_v2/handlers/app_engine.py index 7d16ab07..bc7daa9d 100644 --- a/google/cloud/logging_v2/handlers/app_engine.py +++ b/google/cloud/logging_v2/handlers/app_engine.py @@ -77,7 +77,7 @@ def get_gae_resource(self): Returns: google.cloud.logging_v2.resource.Resource: Monitored resource for GAE. """ - return _create_app_engine_resource(self.project_id) + return _create_app_engine_resource() def get_gae_labels(self): """Return the labels for GAE app. diff --git a/tests/unit/handlers/test__monitored_resources.py b/tests/unit/handlers/test__monitored_resources.py index 00fade39..5acced15 100644 --- a/tests/unit/handlers/test__monitored_resources.py +++ b/tests/unit/handlers/test__monitored_resources.py @@ -61,6 +61,8 @@ def _mock_metadata(self, endpoint): or endpoint == _monitored_resources._GCE_INSTANCE_ID ): return self.NAME + elif endpoint == _monitored_resources._PROJECT_NAME: + return self.PROJECT else: return None @@ -75,7 +77,7 @@ def test_create_legacy_functions_resource(self): os.environ[_monitored_resources._CLOUD_RUN_SERVICE_ID] = self.NAME with patch: - legacy_func_resource = _create_functions_resource(self.PROJECT) + legacy_func_resource = _create_functions_resource() self.assertIsInstance(legacy_func_resource, Resource) self.assertEqual(legacy_func_resource.type, "cloud_function") @@ -90,7 +92,7 @@ def test_create_modern_functions_resource(self): ) os.environ[_monitored_resources._FUNCTION_NAME] = self.NAME with patch: - func_resource = _create_functions_resource(self.PROJECT) + func_resource = _create_functions_resource() self.assertIsInstance(func_resource, Resource) self.assertEqual(func_resource.type, "cloud_function") @@ -105,7 +107,7 @@ def test_create_kubernetes_resource(self): wraps=self._mock_metadata, ) with patch: - resource = _create_kubernetes_resource(self.PROJECT) + resource = _create_kubernetes_resource() self.assertIsInstance(resource, Resource) self.assertEqual(resource.type, "k8s_container") @@ -120,7 +122,7 @@ def test_compute_resource(self): ) with patch: - resource = _create_compute_resource(self.PROJECT) + resource = _create_compute_resource() self.assertIsInstance(resource, Resource) self.assertEqual(resource.type, "gce_instance") self.assertEqual(resource.labels["project_id"], self.PROJECT) @@ -136,7 +138,7 @@ def test_cloud_run_resource(self): os.environ[_monitored_resources._CLOUD_RUN_REVISION_ID] = self.VERSION os.environ[_monitored_resources._CLOUD_RUN_CONFIGURATION_ID] = self.CONFIG with patch: - resource = _create_cloud_run_resource(self.PROJECT) + resource = _create_cloud_run_resource() self.assertIsInstance(resource, Resource) self.assertEqual(resource.type, "cloud_run_revision") self.assertEqual(resource.labels["project_id"], self.PROJECT) @@ -153,7 +155,7 @@ def test_app_engine_resource(self): os.environ[_monitored_resources._GAE_SERVICE_ENV] = self.NAME os.environ[_monitored_resources._GAE_VERSION_ENV] = self.VERSION with patch: - resource = _create_app_engine_resource(self.PROJECT) + resource = _create_app_engine_resource() self.assertIsInstance(resource, Resource) self.assertEqual(resource.type, "gae_app") self.assertEqual(resource.labels["project_id"], self.PROJECT) diff --git a/tests/unit/handlers/test_app_engine.py b/tests/unit/handlers/test_app_engine.py index 1ac9c5dd..65e80457 100644 --- a/tests/unit/handlers/test_app_engine.py +++ b/tests/unit/handlers/test_app_engine.py @@ -40,17 +40,19 @@ def test_constructor_w_gae_standard_env(self): with mock.patch( "os.environ", new={ - app_engine._GAE_PROJECT_ENV_STANDARD: "test_project", app_engine._GAE_SERVICE_ENV: "test_service", app_engine._GAE_VERSION_ENV: "test_version", }, + ), mock.patch( + "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", + return_value=self.PROJECT, ): handler = self._make_one(client, transport=_Transport) self.assertIs(handler.client, client) self.assertEqual(handler.name, app_engine._DEFAULT_GAE_LOGGER_NAME) self.assertEqual(handler.resource.type, "gae_app") - self.assertEqual(handler.resource.labels["project_id"], "test_project") + self.assertEqual(handler.resource.labels["project_id"], self.PROJECT) self.assertEqual(handler.resource.labels["module_id"], "test_service") self.assertEqual(handler.resource.labels["version_id"], "test_version") self.assertIs(handler.stream, sys.stderr) @@ -73,6 +75,9 @@ def test_constructor_w_gae_flex_env(self): app_engine._GAE_SERVICE_ENV: "test_service_2", app_engine._GAE_VERSION_ENV: "test_version_2", }, + ), mock.patch( + "google.cloud.logging_v2.handlers._monitored_resources.retrieve_metadata_server", + return_value=self.PROJECT, ): handler = self._make_one( client, name=name, transport=_Transport, stream=stream @@ -81,7 +86,7 @@ def test_constructor_w_gae_flex_env(self): self.assertIs(handler.client, client) self.assertEqual(handler.name, name) self.assertEqual(handler.resource.type, "gae_app") - self.assertEqual(handler.resource.labels["project_id"], "test_project_2") + self.assertEqual(handler.resource.labels["project_id"], self.PROJECT) self.assertEqual(handler.resource.labels["module_id"], "test_service_2") self.assertEqual(handler.resource.labels["version_id"], "test_version_2") self.assertIs(handler.stream, stream)