From 36f97804732b99401200946d8dbb32cea5d9abb9 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 12 Nov 2021 15:05:17 -0800 Subject: [PATCH 1/4] fix: allow reading logs without an associated logger --- google/cloud/logging_v2/entries.py | 14 +++-- tests/unit/test_entries.py | 85 +++++++++++++++++++++++++++++- tests/unit/test_logger.py | 34 ++++++++++++ 3 files changed, 127 insertions(+), 6 deletions(-) diff --git a/google/cloud/logging_v2/entries.py b/google/cloud/logging_v2/entries.py index 0af5a46f7..ade6110d5 100644 --- a/google/cloud/logging_v2/entries.py +++ b/google/cloud/logging_v2/entries.py @@ -46,11 +46,12 @@ ) -def logger_name_from_path(path): +def logger_name_from_path(path, project=None): """Validate a logger URI path and get the logger name. Args: path (str): URI path for a logger API request + project (str): The project the path is expected to belong to Returns: str: Logger name parsed from ``path``. @@ -59,7 +60,7 @@ def logger_name_from_path(path): ValueError: If the ``path`` is ill-formed of if the project from ``path`` does not agree with the ``project`` passed in. """ - return _name_from_project_path(path, None, _LOGGER_TEMPLATE) + return _name_from_project_path(path, project, _LOGGER_TEMPLATE) def _int_or_none(value): @@ -165,8 +166,13 @@ def from_api_repr(cls, resource, client, *, loggers=None): logger_fullname = resource["logName"] logger = loggers.get(logger_fullname) if logger is None: - logger_name = logger_name_from_path(logger_fullname) - logger = loggers[logger_fullname] = client.logger(logger_name) + # attempt to create a logger if possible + try: + logger_name = logger_name_from_path(logger_fullname, client.project) + logger = loggers[logger_fullname] = client.logger(logger_name) + except ValueError: + # log name is not scoped to a project. Leave logger as None + pass payload = cls._extract_payload(resource) insert_id = resource.get("insertId") timestamp = resource.get("timestamp") diff --git a/tests/unit/test_entries.py b/tests/unit/test_entries.py index b8795b8ce..236ee30cc 100644 --- a/tests/unit/test_entries.py +++ b/tests/unit/test_entries.py @@ -18,10 +18,10 @@ class Test_logger_name_from_path(unittest.TestCase): - def _call_fut(self, path): + def _call_fut(self, path, project=None): from google.cloud.logging_v2.entries import logger_name_from_path - return logger_name_from_path(path) + return logger_name_from_path(path, project) def test_w_simple_name(self): LOGGER_NAME = "LOGGER_NAME" @@ -37,6 +37,30 @@ def test_w_name_w_all_extras(self): logger_name = self._call_fut(PATH) self.assertEqual(logger_name, LOGGER_NAME) + def test_w_wrong_project(self): + LOGGER_NAME = "LOGGER_NAME" + IN_PROJECT = "in-project" + PATH_PROJECT = "path-project" + PATH = "projects/%s/logs/%s" % (PATH_PROJECT, LOGGER_NAME) + with self.assertRaises(ValueError): + logger_name = self._call_fut(PATH, IN_PROJECT) + + def test_invalid_inputs(self): + invalid_list = [ + "", + "abc/123/logs/456", + "projects//logs/", + "projects/123/logs", + "projects/123logs/", + "projects123/logs", + "project/123", + "projects123logs456", + "/logs/123", + ] + for path in invalid_list: + with self.assertRaises(ValueError): + self._call_fut(path) + class Test__int_or_none(unittest.TestCase): def _call_fut(self, value): @@ -315,6 +339,63 @@ def test_from_api_repr_w_loggers_w_logger_match(self): self.assertEqual(entry.operation, OPERATION) self.assertIsNone(entry.payload) + def test_from_api_repr_w_folder_path(self): + from datetime import datetime + from datetime import timedelta + from google.cloud._helpers import UTC + + client = _Client(self.PROJECT) + IID = "IID" + NOW = datetime.utcnow().replace(tzinfo=UTC) + LATER = NOW + timedelta(seconds=1) + TIMESTAMP = _datetime_to_rfc3339_w_nanos(NOW) + RECEIVED = _datetime_to_rfc3339_w_nanos(LATER) + LOG_NAME = "folders/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME) + LABELS = {"foo": "bar", "baz": "qux"} + TRACE = "12345678-1234-5678-1234-567812345678" + SPANID = "000000000000004a" + FILE = "my_file.py" + LINE_NO = 123 + FUNCTION = "my_function" + SOURCE_LOCATION = {"file": FILE, "line": str(LINE_NO), "function": FUNCTION} + OP_ID = "OP_ID" + PRODUCER = "PRODUCER" + OPERATION = {"id": OP_ID, "producer": PRODUCER, "first": True, "last": False} + API_REPR = { + "logName": LOG_NAME, + "insertId": IID, + "timestamp": TIMESTAMP, + "receiveTimestamp": RECEIVED, + "labels": LABELS, + "trace": TRACE, + "spanId": SPANID, + "traceSampled": True, + "sourceLocation": SOURCE_LOCATION, + "operation": OPERATION, + } + LOGGER = object() + klass = self._get_target_class() + + entry = klass.from_api_repr(API_REPR, client) + + self.assertEqual(entry.log_name, LOG_NAME) + self.assertIsNone(entry.logger) + self.assertEqual(entry.insert_id, IID) + self.assertEqual(entry.timestamp, NOW) + self.assertEqual(entry.received_timestamp, LATER) + self.assertEqual(entry.labels, LABELS) + self.assertEqual(entry.trace, TRACE) + self.assertEqual(entry.span_id, SPANID) + self.assertTrue(entry.trace_sampled) + + source_location = entry.source_location + self.assertEqual(source_location["file"], FILE) + self.assertEqual(source_location["line"], LINE_NO) + self.assertEqual(source_location["function"], FUNCTION) + + self.assertEqual(entry.operation, OPERATION) + self.assertIsNone(entry.payload) + def test_to_api_repr_w_source_location_no_line(self): from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 5f0868ba2..54fa44892 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -937,6 +937,40 @@ def test_list_entries_limit(self): }, ) + def test_list_entries_folder(self): + from google.cloud.logging import TextEntry + from google.cloud.logging import Logger + from google.cloud.logging import Client + + + client = Client( + project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False + ) + FOLDER_ID = "123" + LOG_NAME= f"folders/{FOLDER_ID}/logs/cloudaudit.googleapis.com%2Fdata_access" + + ENTRIES = [ + { + "textPayload": "hello world", + "insertId": "1", + "resource": {"type": "global"}, + "logName": LOG_NAME, + }, + ] + returned = {"entries": ENTRIES} + client._connection = _Connection(returned) + + iterator = client.list_entries( + resource_names=[f"folder/{FOLDER_ID}"], + ) + entries = list(iterator) + # Check the entries. + self.assertEqual(len(entries), 1) + entry = entries[0] + self.assertIsInstance(entry, TextEntry) + self.assertIsNone(entry.logger) + self.assertEqual(entry.log_name, LOG_NAME) + class TestBatch(unittest.TestCase): From 8e19c6cffdd85313fe01a0f5dab26b5c84299505 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 12 Nov 2021 15:07:14 -0800 Subject: [PATCH 2/4] fixed lint issues --- tests/unit/test_entries.py | 3 +-- tests/unit/test_logger.py | 8 ++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/unit/test_entries.py b/tests/unit/test_entries.py index 236ee30cc..6f3af684f 100644 --- a/tests/unit/test_entries.py +++ b/tests/unit/test_entries.py @@ -43,7 +43,7 @@ def test_w_wrong_project(self): PATH_PROJECT = "path-project" PATH = "projects/%s/logs/%s" % (PATH_PROJECT, LOGGER_NAME) with self.assertRaises(ValueError): - logger_name = self._call_fut(PATH, IN_PROJECT) + self._call_fut(PATH, IN_PROJECT) def test_invalid_inputs(self): invalid_list = [ @@ -373,7 +373,6 @@ def test_from_api_repr_w_folder_path(self): "sourceLocation": SOURCE_LOCATION, "operation": OPERATION, } - LOGGER = object() klass = self._get_target_class() entry = klass.from_api_repr(API_REPR, client) diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 54fa44892..1eae1cda6 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -939,15 +939,13 @@ def test_list_entries_limit(self): def test_list_entries_folder(self): from google.cloud.logging import TextEntry - from google.cloud.logging import Logger from google.cloud.logging import Client - client = Client( project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False ) FOLDER_ID = "123" - LOG_NAME= f"folders/{FOLDER_ID}/logs/cloudaudit.googleapis.com%2Fdata_access" + LOG_NAME = f"folders/{FOLDER_ID}/logs/cloudaudit.googleapis.com%2Fdata_access" ENTRIES = [ { @@ -960,9 +958,7 @@ def test_list_entries_folder(self): returned = {"entries": ENTRIES} client._connection = _Connection(returned) - iterator = client.list_entries( - resource_names=[f"folder/{FOLDER_ID}"], - ) + iterator = client.list_entries(resource_names=[f"folder/{FOLDER_ID}"],) entries = list(iterator) # Check the entries. self.assertEqual(len(entries), 1) From 258d1e9ccb393a84986be72c2130409f6e558eb4 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Fri, 12 Nov 2021 15:18:26 -0800 Subject: [PATCH 3/4] updated docstring --- google/cloud/logging_v2/entries.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google/cloud/logging_v2/entries.py b/google/cloud/logging_v2/entries.py index ade6110d5..cb485da61 100644 --- a/google/cloud/logging_v2/entries.py +++ b/google/cloud/logging_v2/entries.py @@ -156,7 +156,8 @@ def from_api_repr(cls, resource, client, *, loggers=None): Client which holds credentials and project configuration. loggers (Optional[dict]): A mapping of logger fullnames -> loggers. If not - passed, the entry will have a newly-created logger. + passed, the entry will have a newly-created logger if possible, + or an empty logger field if not. Returns: google.cloud.logging.entries.LogEntry: Log entry parsed from ``resource``. From 98910ae80273bef3425a820887727421ceced765 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 7 Dec 2021 12:27:00 -0800 Subject: [PATCH 4/4] updated submodule --- tests/environment | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/environment b/tests/environment index a201d48f1..2ff3cb26a 160000 --- a/tests/environment +++ b/tests/environment @@ -1 +1 @@ -Subproject commit a201d48f163db4ff799305188d380f620ebf345a +Subproject commit 2ff3cb26a09d574715573bcf0a2c480eeac73311