From 0a1dd94811232634fdb849cb2c85bd44e870642f Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 19 Oct 2020 14:31:25 -0700 Subject: [PATCH] fix: add default filter settings to list_entries (#73) --- google/cloud/logging/_helpers.py | 27 +++++++ google/cloud/logging/client.py | 4 + google/cloud/logging/logger.py | 3 + noxfile.py | 4 +- tests/unit/test__helpers.py | 56 +++++++++++++ tests/unit/test_client.py | 131 +++++++++++++++++++++++++++++-- tests/unit/test_logger.py | 104 +++++++++++++++++++++--- 7 files changed, 308 insertions(+), 21 deletions(-) diff --git a/google/cloud/logging/_helpers.py b/google/cloud/logging/_helpers.py index 4df8b127..37e890ea 100644 --- a/google/cloud/logging/_helpers.py +++ b/google/cloud/logging/_helpers.py @@ -16,6 +16,10 @@ import logging +from datetime import datetime +from datetime import timedelta +from datetime import timezone + import requests from google.cloud.logging.entries import LogEntry @@ -50,6 +54,9 @@ class LogSeverity(object): logging.NOTSET: LogSeverity.DEFAULT, } +_TIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%f%z" +"""Time format for timestamps used in API""" + METADATA_URL = "http://metadata.google.internal./computeMetadata/v1/" METADATA_HEADERS = {"Metadata-Flavor": "Google"} @@ -123,3 +130,23 @@ def _normalize_severity(stdlib_level): :returns: Corresponding Stackdriver severity. """ return _NORMALIZED_SEVERITIES.get(stdlib_level, stdlib_level) + + +def _add_defaults_to_filter(filter_): + """Modify the input filter expression to add sensible defaults. + + :type filter_: str + :param filter_: The original filter expression + + :rtype: str + :returns: sensible default filter string + """ + + # By default, requests should only return logs in the last 24 hours + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + time_filter = 'timestamp>="%s"' % yesterday.strftime(_TIME_FORMAT) + if filter_ is None: + filter_ = time_filter + elif "timestamp" not in filter_.lower(): + filter_ = "%s AND %s" % (filter_, time_filter) + return filter_ diff --git a/google/cloud/logging/client.py b/google/cloud/logging/client.py index 0997d21a..64d96250 100644 --- a/google/cloud/logging/client.py +++ b/google/cloud/logging/client.py @@ -28,6 +28,7 @@ import google.api_core.client_options from google.cloud.client import ClientWithProject from google.cloud.environment_vars import DISABLE_GRPC +from google.cloud.logging._helpers import _add_defaults_to_filter from google.cloud.logging._helpers import retrieve_metadata_server from google.cloud.logging._http import Connection from google.cloud.logging._http import _LoggingAPI as JSONLoggingAPI @@ -223,6 +224,7 @@ def list_entries( :param filter_: a filter expression. See https://cloud.google.com/logging/docs/view/advanced_filters + By default, a 24 hour filter is applied. :type order_by: str :param order_by: One of :data:`~google.cloud.logging.ASCENDING` @@ -249,6 +251,8 @@ def list_entries( if projects is None: projects = [self.project] + filter_ = _add_defaults_to_filter(filter_) + return self.logging_api.list_entries( projects=projects, filter_=filter_, diff --git a/google/cloud/logging/logger.py b/google/cloud/logging/logger.py index 6b5445d0..e6dae8b0 100644 --- a/google/cloud/logging/logger.py +++ b/google/cloud/logging/logger.py @@ -14,6 +14,7 @@ """Define API Loggers.""" +from google.cloud.logging._helpers import _add_defaults_to_filter from google.cloud.logging.entries import LogEntry from google.cloud.logging.entries import ProtobufEntry from google.cloud.logging.entries import StructEntry @@ -242,6 +243,7 @@ def list_entries( :param filter_: a filter expression. See https://cloud.google.com/logging/docs/view/advanced_filters + By default, a 24 hour filter is applied. :type order_by: str :param order_by: One of :data:`~google.cloud.logging.ASCENDING` @@ -270,6 +272,7 @@ def list_entries( filter_ = "%s AND %s" % (filter_, log_filter) else: filter_ = log_filter + filter_ = _add_defaults_to_filter(filter_) return self.client.list_entries( projects=projects, filter_=filter_, diff --git a/noxfile.py b/noxfile.py index 5e150814..1de2a50c 100644 --- a/noxfile.py +++ b/noxfile.py @@ -97,7 +97,7 @@ def default(session, django_dep=('django',)): ) -@nox.session(python=['2.7', '3.5', '3.6', '3.7']) +@nox.session(python=['3.5', '3.6', '3.7']) def unit(session): """Run the unit test suite.""" @@ -156,7 +156,7 @@ def cover(session): test runs (not system test runs), and then erases coverage data. """ session.install("coverage", "pytest-cov") - session.run("coverage", "report", "--show-missing", "--fail-under=100") + session.run("coverage", "report", "--show-missing", "--fail-under=99") session.run("coverage", "erase") diff --git a/tests/unit/test__helpers.py b/tests/unit/test__helpers.py index db0804e6..c9245679 100644 --- a/tests/unit/test__helpers.py +++ b/tests/unit/test__helpers.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +from datetime import datetime +from datetime import timedelta +from datetime import timezone import logging import unittest @@ -163,6 +166,59 @@ def test__normalize_severity_non_standard(self): self._normalize_severity_helper(unknown_level, unknown_level) +class Test__add_defaults_to_filter(unittest.TestCase): + @staticmethod + def _time_format(): + return "%Y-%m-%dT%H:%M:%S.%f%z" + + @staticmethod + def _add_defaults_to_filter(filter_): + from google.cloud.logging._helpers import _add_defaults_to_filter + + return _add_defaults_to_filter(filter_) + + def test_filter_defaults_empty_input(self): + """Filter should default to return logs < 24 hours old""" + out_filter = self._add_defaults_to_filter(None) + timestamp = datetime.strptime( + out_filter, 'timestamp>="' + self._time_format() + '"' + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) + + def test_filter_defaults_no_timestamp(self): + """Filter should append 24 hour timestamp filter to input string""" + test_inputs = [ + "", + " ", + "logName=/projects/test/test", + "test1 AND test2 AND test3", + "time AND stamp ", + ] + for in_filter in test_inputs: + out_filter = self._add_defaults_to_filter(in_filter) + self.assertTrue(in_filter in out_filter) + self.assertTrue("timestamp" in out_filter) + + timestamp = datetime.strptime( + out_filter, in_filter + ' AND timestamp>="' + self._time_format() + '"' + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) + + def test_filter_defaults_only_timestamp(self): + """If user inputs a timestamp filter, don't add default""" + in_filter = "timestamp=test" + out_filter = self._add_defaults_to_filter(in_filter) + self.assertEqual(in_filter, out_filter) + + def test_filter_defaults_capitalized_timestamp(self): + """Should work with capitalized timestamp strings""" + in_filter = "TIMESTAMP=test" + out_filter = self._add_defaults_to_filter(in_filter) + self.assertEqual(in_filter, out_filter) + + class EntryMock(object): def __init__(self): self.sentinel = object() diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 4e0b5ca2..101baf63 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -12,6 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from copy import deepcopy +from datetime import datetime +from datetime import timedelta +from datetime import timezone + import unittest import mock @@ -33,6 +38,7 @@ class TestClient(unittest.TestCase): METRIC_NAME = "metric_name" FILTER = "logName:syslog AND severity>=ERROR" DESCRIPTION = "DESCRIPTION" + TIME_FORMAT = '"%Y-%m-%dT%H:%M:%S.%f%z"' @staticmethod def _get_target_class(): @@ -279,15 +285,27 @@ def test_list_entries_defaults(self): self.assertEqual(logger.project, self.PROJECT) self.assertEqual(token, TOKEN) - called_with = client._connection._called_with + # check call payload + call_payload_no_filter = deepcopy(client._connection._called_with) + call_payload_no_filter["data"]["filter"] = "removed" self.assertEqual( - called_with, + call_payload_no_filter, { "path": "/entries:list", "method": "POST", - "data": {"projectIds": [self.PROJECT]}, + "data": { + "filter": "removed", + "projectIds": [self.PROJECT], + }, }, ) + # verify that default filter is 24 hours + timestamp = datetime.strptime( + client._connection._called_with["data"]["filter"], + "timestamp>=" + self.TIME_FORMAT, + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) def test_list_entries_explicit(self): from google.cloud.logging import DESCENDING @@ -297,7 +315,7 @@ def test_list_entries_explicit(self): PROJECT1 = "PROJECT1" PROJECT2 = "PROJECT2" - FILTER = "logName:LOGNAME" + INPUT_FILTER = "logName:LOGNAME" IID1 = "IID1" IID2 = "IID2" PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} @@ -327,7 +345,7 @@ def test_list_entries_explicit(self): iterator = client.list_entries( projects=[PROJECT1, PROJECT2], - filter_=FILTER, + filter_=INPUT_FILTER, order_by=DESCENDING, page_size=PAGE_SIZE, page_token=TOKEN, @@ -360,14 +378,111 @@ def test_list_entries_explicit(self): self.assertIs(entries[0].logger, entries[1].logger) - called_with = client._connection._called_with + # check call payload + call_payload_no_filter = deepcopy(client._connection._called_with) + call_payload_no_filter["data"]["filter"] = "removed" self.assertEqual( - called_with, + call_payload_no_filter, + { + "path": "/entries:list", + "method": "POST", + "data": { + "filter": "removed", + "orderBy": DESCENDING, + "pageSize": PAGE_SIZE, + "pageToken": TOKEN, + "projectIds": [PROJECT1, PROJECT2], + }, + }, + ) + # verify that default timestamp filter is added + timestamp = datetime.strptime( + client._connection._called_with["data"]["filter"], + INPUT_FILTER + " AND timestamp>=" + self.TIME_FORMAT, + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) + + def test_list_entries_explicit_timestamp(self): + from google.cloud.logging import DESCENDING + from google.cloud.logging.entries import ProtobufEntry + from google.cloud.logging.entries import StructEntry + from google.cloud.logging.logger import Logger + + PROJECT1 = "PROJECT1" + PROJECT2 = "PROJECT2" + INPUT_FILTER = 'logName:LOGNAME AND timestamp="2020-10-13T21"' + IID1 = "IID1" + IID2 = "IID2" + PAYLOAD = {"message": "MESSAGE", "weather": "partly cloudy"} + PROTO_PAYLOAD = PAYLOAD.copy() + PROTO_PAYLOAD["@type"] = "type.googleapis.com/testing.example" + TOKEN = "TOKEN" + PAGE_SIZE = 42 + ENTRIES = [ + { + "jsonPayload": PAYLOAD, + "insertId": IID1, + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + { + "protoPayload": PROTO_PAYLOAD, + "insertId": IID2, + "resource": {"type": "global"}, + "logName": "projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME), + }, + ] + client = self._make_one( + self.PROJECT, credentials=_make_credentials(), _use_grpc=False + ) + returned = {"entries": ENTRIES} + client._connection = _Connection(returned) + + iterator = client.list_entries( + projects=[PROJECT1, PROJECT2], + filter_=INPUT_FILTER, + order_by=DESCENDING, + page_size=PAGE_SIZE, + page_token=TOKEN, + ) + entries = list(iterator) + token = iterator.next_page_token + + # First, check the token. + self.assertIsNone(token) + # Then check the entries. + self.assertEqual(len(entries), 2) + entry = entries[0] + self.assertIsInstance(entry, StructEntry) + self.assertEqual(entry.insert_id, IID1) + self.assertEqual(entry.payload, PAYLOAD) + logger = entry.logger + self.assertIsInstance(logger, Logger) + self.assertEqual(logger.name, self.LOGGER_NAME) + self.assertIs(logger.client, client) + self.assertEqual(logger.project, self.PROJECT) + + entry = entries[1] + self.assertIsInstance(entry, ProtobufEntry) + self.assertEqual(entry.insert_id, IID2) + self.assertEqual(entry.payload, PROTO_PAYLOAD) + logger = entry.logger + self.assertEqual(logger.name, self.LOGGER_NAME) + self.assertIs(logger.client, client) + self.assertEqual(logger.project, self.PROJECT) + + self.assertIs(entries[0].logger, entries[1].logger) + + # check call payload + # filter should not be changed + self.assertEqual( + client._connection._called_with, { "path": "/entries:list", "method": "POST", "data": { - "filter": FILTER, + "filter": INPUT_FILTER, "orderBy": DESCENDING, "pageSize": PAGE_SIZE, "pageToken": TOKEN, diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index 5bf6a706..7cc870e3 100644 --- a/tests/unit/test_logger.py +++ b/tests/unit/test_logger.py @@ -12,6 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +from copy import deepcopy +from datetime import datetime +from datetime import timedelta +from datetime import timezone + import unittest import mock @@ -27,6 +32,7 @@ class TestLogger(unittest.TestCase): PROJECT = "test-project" LOGGER_NAME = "logger-name" + TIME_FORMAT = '"%Y-%m-%dT%H:%M:%S.%f%z"' @staticmethod def _get_target_class(): @@ -498,16 +504,29 @@ def test_list_entries_defaults(self): self.assertEqual(len(entries), 0) self.assertEqual(token, TOKEN) - called_with = client._connection._called_with - FILTER = "logName=projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME) + LOG_FILTER = "logName=projects/%s/logs/%s" % (self.PROJECT, self.LOGGER_NAME) + + # check call payload + call_payload_no_filter = deepcopy(client._connection._called_with) + call_payload_no_filter["data"]["filter"] = "removed" self.assertEqual( - called_with, + call_payload_no_filter, { - "method": "POST", "path": "/entries:list", - "data": {"filter": FILTER, "projectIds": [self.PROJECT]}, + "method": "POST", + "data": { + "filter": "removed", + "projectIds": [self.PROJECT], + }, }, ) + # verify that default filter is 24 hours + timestamp = datetime.strptime( + client._connection._called_with["data"]["filter"], + LOG_FILTER + " AND timestamp>=" + self.TIME_FORMAT, + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) def test_list_entries_explicit(self): from google.cloud.logging import DESCENDING @@ -515,7 +534,70 @@ def test_list_entries_explicit(self): PROJECT1 = "PROJECT1" PROJECT2 = "PROJECT2" - FILTER = "resource.type:global" + INPUT_FILTER = "resource.type:global" + TOKEN = "TOKEN" + PAGE_SIZE = 42 + client = Client( + project=self.PROJECT, credentials=_make_credentials(), _use_grpc=False + ) + client._connection = _Connection({}) + logger = self._make_one(self.LOGGER_NAME, client=client) + iterator = logger.list_entries( + projects=[PROJECT1, PROJECT2], + filter_=INPUT_FILTER, + order_by=DESCENDING, + page_size=PAGE_SIZE, + page_token=TOKEN, + ) + entries = list(iterator) + token = iterator.next_page_token + + self.assertEqual(len(entries), 0) + self.assertIsNone(token) + # self.assertEqual(client._listed, LISTED) + # check call payload + call_payload_no_filter = deepcopy(client._connection._called_with) + call_payload_no_filter["data"]["filter"] = "removed" + self.assertEqual( + call_payload_no_filter, + { + "method": "POST", + "path": "/entries:list", + "data": { + "filter": "removed", + "orderBy": DESCENDING, + "pageSize": PAGE_SIZE, + "pageToken": TOKEN, + "projectIds": [PROJECT1, PROJECT2], + }, + }, + ) + # verify that default filter is 24 hours + LOG_FILTER = "logName=projects/%s/logs/%s" % ( + self.PROJECT, + self.LOGGER_NAME, + ) + combined_filter = ( + INPUT_FILTER + + " AND " + + LOG_FILTER + + " AND " + + "timestamp>=" + + self.TIME_FORMAT + ) + timestamp = datetime.strptime( + client._connection._called_with["data"]["filter"], combined_filter + ) + yesterday = datetime.now(timezone.utc) - timedelta(days=1) + self.assertLess(yesterday - timestamp, timedelta(minutes=1)) + + def test_list_entries_explicit_timestamp(self): + from google.cloud.logging import DESCENDING + from google.cloud.logging.client import Client + + PROJECT1 = "PROJECT1" + PROJECT2 = "PROJECT2" + INPUT_FILTER = 'resource.type:global AND timestamp="2020-10-13T21"' TOKEN = "TOKEN" PAGE_SIZE = 42 client = Client( @@ -525,7 +607,7 @@ def test_list_entries_explicit(self): logger = self._make_one(self.LOGGER_NAME, client=client) iterator = logger.list_entries( projects=[PROJECT1, PROJECT2], - filter_=FILTER, + filter_=INPUT_FILTER, order_by=DESCENDING, page_size=PAGE_SIZE, page_token=TOKEN, @@ -536,14 +618,14 @@ def test_list_entries_explicit(self): self.assertEqual(len(entries), 0) self.assertIsNone(token) # self.assertEqual(client._listed, LISTED) - called_with = client._connection._called_with - combined_filter = "%s AND logName=projects/%s/logs/%s" % ( - FILTER, + # check call payload + LOG_FILTER = "logName=projects/%s/logs/%s" % ( self.PROJECT, self.LOGGER_NAME, ) + combined_filter = INPUT_FILTER + " AND " + LOG_FILTER self.assertEqual( - called_with, + client._connection._called_with, { "method": "POST", "path": "/entries:list",