From def7440ac6964451f3202b5117e3060ec62045b0 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 11 Feb 2022 11:18:37 -0800 Subject: [PATCH] fix: remove unnecessary detect_resource calls from CloudLoggingHandler (#484) --- google/cloud/logging_v2/_helpers.py | 5 +++-- google/cloud/logging_v2/handlers/handlers.py | 2 +- .../handlers/transports/background_thread.py | 7 ++++++- google/cloud/logging_v2/handlers/transports/base.py | 13 +++++++++++++ google/cloud/logging_v2/handlers/transports/sync.py | 13 +++++++++++-- tests/unit/handlers/test_handlers.py | 2 +- .../handlers/transports/test_background_thread.py | 7 ++++--- tests/unit/handlers/transports/test_base.py | 7 +++++-- tests/unit/handlers/transports/test_sync.py | 7 ++++--- 9 files changed, 48 insertions(+), 15 deletions(-) diff --git a/google/cloud/logging_v2/_helpers.py b/google/cloud/logging_v2/_helpers.py index 51cc64868..75f84e50c 100644 --- a/google/cloud/logging_v2/_helpers.py +++ b/google/cloud/logging_v2/_helpers.py @@ -89,7 +89,7 @@ def entry_from_resource(resource, client, loggers): return LogEntry.from_api_repr(resource, client, loggers=loggers) -def retrieve_metadata_server(metadata_key): +def retrieve_metadata_server(metadata_key, timeout=5): """Retrieve the metadata key in the metadata server. See: https://cloud.google.com/compute/docs/storing-retrieving-metadata @@ -99,6 +99,7 @@ def retrieve_metadata_server(metadata_key): Key of the metadata which will form the url. You can also supply query parameters after the metadata key. e.g. "tags?alt=json" + timeout (number): number of seconds to wait for the HTTP request Returns: str: The value of the metadata key returned by the metadata server. @@ -106,7 +107,7 @@ def retrieve_metadata_server(metadata_key): url = METADATA_URL + metadata_key try: - response = requests.get(url, headers=METADATA_HEADERS) + response = requests.get(url, headers=METADATA_HEADERS, timeout=timeout) if response.status_code == requests.codes.ok: return response.text diff --git a/google/cloud/logging_v2/handlers/handlers.py b/google/cloud/logging_v2/handlers/handlers.py index 769146007..f6fa90d71 100644 --- a/google/cloud/logging_v2/handlers/handlers.py +++ b/google/cloud/logging_v2/handlers/handlers.py @@ -179,7 +179,7 @@ def __init__( resource = detect_resource(client.project) self.name = name self.client = client - self.transport = transport(client, name) + self.transport = transport(client, name, resource=resource) self.project_id = client.project self.resource = resource self.labels = labels diff --git a/google/cloud/logging_v2/handlers/transports/background_thread.py b/google/cloud/logging_v2/handlers/transports/background_thread.py index 1097830a8..f361e043c 100644 --- a/google/cloud/logging_v2/handlers/transports/background_thread.py +++ b/google/cloud/logging_v2/handlers/transports/background_thread.py @@ -29,6 +29,7 @@ from google.cloud.logging_v2 import _helpers from google.cloud.logging_v2.handlers.transports.base import Transport +from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE _DEFAULT_GRACE_PERIOD = 5.0 # Seconds _DEFAULT_MAX_BATCH_SIZE = 10 @@ -260,6 +261,8 @@ def __init__( grace_period=_DEFAULT_GRACE_PERIOD, batch_size=_DEFAULT_MAX_BATCH_SIZE, max_latency=_DEFAULT_MAX_LATENCY, + resource=_GLOBAL_RESOURCE, + **kwargs, ): """ Args: @@ -275,9 +278,11 @@ def __init__( than the grace_period. This means this is effectively the longest amount of time the background thread will hold onto log entries before sending them to the server. + resource (Optional[Resource|dict]): The default monitored resource to associate + with logs when not specified """ self.client = client - logger = self.client.logger(name) + logger = self.client.logger(name, resource=resource) self.worker = _Worker( logger, grace_period=grace_period, diff --git a/google/cloud/logging_v2/handlers/transports/base.py b/google/cloud/logging_v2/handlers/transports/base.py index bd52b4e75..a0c9aafa4 100644 --- a/google/cloud/logging_v2/handlers/transports/base.py +++ b/google/cloud/logging_v2/handlers/transports/base.py @@ -14,6 +14,8 @@ """Module containing base class for logging transport.""" +from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE + class Transport(object): """Base class for Google Cloud Logging handler transports. @@ -22,6 +24,17 @@ class Transport(object): client and name object, and must override :meth:`send`. """ + def __init__(self, client, name, resource=_GLOBAL_RESOURCE, **kwargs): + """ + Args: + client (~logging_v2.client.Client): + The Logging client. + name (str): The name of the lgoger. + resource (Optional[Resource|dict]): The default monitored resource to associate + with logs when not specified + """ + super().__init__() + def send(self, record, message, **kwargs): """Transport send to be implemented by subclasses. diff --git a/google/cloud/logging_v2/handlers/transports/sync.py b/google/cloud/logging_v2/handlers/transports/sync.py index 796f0d2ff..6f93b2e57 100644 --- a/google/cloud/logging_v2/handlers/transports/sync.py +++ b/google/cloud/logging_v2/handlers/transports/sync.py @@ -18,6 +18,7 @@ """ from google.cloud.logging_v2 import _helpers from google.cloud.logging_v2.handlers.transports.base import Transport +from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE class SyncTransport(Transport): @@ -26,8 +27,16 @@ class SyncTransport(Transport): Uses this library's Logging client to directly make the API call. """ - def __init__(self, client, name): - self.logger = client.logger(name) + def __init__(self, client, name, resource=_GLOBAL_RESOURCE, **kwargs): + """ + Args: + client (~logging_v2.client.Client): + The Logging client. + name (str): The name of the lgoger. + resource (Optional[Resource|dict]): The default monitored resource to associate + with logs when not specified + """ + self.logger = client.logger(name, resource=resource) def send(self, record, message, **kwargs): """Overrides transport.send(). diff --git a/tests/unit/handlers/test_handlers.py b/tests/unit/handlers/test_handlers.py index bbfacf59f..353e7d2f6 100644 --- a/tests/unit/handlers/test_handlers.py +++ b/tests/unit/handlers/test_handlers.py @@ -860,7 +860,7 @@ def __init__(self, project): class _Transport(object): - def __init__(self, client, name): + def __init__(self, client, name, resource=None): self.client = client self.name = name diff --git a/tests/unit/handlers/transports/test_background_thread.py b/tests/unit/handlers/transports/test_background_thread.py index f408de476..0c547d736 100644 --- a/tests/unit/handlers/transports/test_background_thread.py +++ b/tests/unit/handlers/transports/test_background_thread.py @@ -509,11 +509,12 @@ def commit(self): class _Logger(object): - def __init__(self, name): + def __init__(self, name, resource=None): self.name = name self._batch_cls = _Batch self._batch = None self._num_batches = 0 + self.resource = resource def batch(self): self._batch = self._batch_cls() @@ -530,6 +531,6 @@ def __init__(self, project, _http=None, credentials=None): self._credentials = credentials self._connection = mock.Mock(credentials=credentials, spec=["credentials"]) - def logger(self, name): # pylint: disable=unused-argument - self._logger = _Logger(name) + def logger(self, name, resource=None): # pylint: disable=unused-argument + self._logger = _Logger(name, resource=resource) return self._logger diff --git a/tests/unit/handlers/transports/test_base.py b/tests/unit/handlers/transports/test_base.py index 4cbfab02e..71ef1366a 100644 --- a/tests/unit/handlers/transports/test_base.py +++ b/tests/unit/handlers/transports/test_base.py @@ -29,10 +29,13 @@ def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) def test_send_is_abstract(self): - target = self._make_one() + target = self._make_one("client", "name") with self.assertRaises(NotImplementedError): target.send(None, None, resource=None) + def test_resource_is_valid_argunent(self): + self._make_one("client", "name", resource="resource") + def test_flush_is_abstract_and_optional(self): - target = self._make_one() + target = self._make_one("client", "name") target.flush() diff --git a/tests/unit/handlers/transports/test_sync.py b/tests/unit/handlers/transports/test_sync.py index cc8ffe284..bdc78d89a 100644 --- a/tests/unit/handlers/transports/test_sync.py +++ b/tests/unit/handlers/transports/test_sync.py @@ -91,8 +91,9 @@ def test_send_struct(self): class _Logger(object): from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE - def __init__(self, name): + def __init__(self, name, resource=_GLOBAL_RESOURCE): self.name = name + self.resource = resource def log( self, @@ -119,8 +120,8 @@ class _Client(object): def __init__(self, project): self.project = project - def logger(self, name): # pylint: disable=unused-argument - self._logger = _Logger(name) + def logger(self, name, resource=None): # pylint: disable=unused-argument + self._logger = _Logger(name, resource=resource) return self._logger