Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove unnecessary detect_resource calls from CloudLoggingHandler #484

Merged
merged 12 commits into from
Feb 11, 2022
5 changes: 3 additions & 2 deletions google/cloud/logging_v2/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
daniel-sanche marked this conversation as resolved.
Show resolved Hide resolved
"""Retrieve the metadata key in the metadata server.

See: https://cloud.google.com/compute/docs/storing-retrieving-metadata
Expand All @@ -99,14 +99,15 @@ 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.
"""
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
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/logging_v2/handlers/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down
13 changes: 13 additions & 0 deletions google/cloud/logging_v2/handlers/transports/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.

Expand Down
13 changes: 11 additions & 2 deletions google/cloud/logging_v2/handlers/transports/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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().
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/handlers/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 4 additions & 3 deletions tests/unit/handlers/transports/test_background_thread.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
7 changes: 5 additions & 2 deletions tests/unit/handlers/transports/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
7 changes: 4 additions & 3 deletions tests/unit/handlers/transports/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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


Expand Down