Skip to content

fix(logging): only skip the default 24 hour filter for timestamp restrictions #18714

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.

Sign up for GitHub

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

Open
abhishekacharya200 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(logging): only skip the default 24 hour filter for timestamp restrictions #18714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion packages/google-cloud-logging/google/cloud/logging_v2/_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"""Common logging helpers."""

import logging
import re
from datetime import datetime, timedelta, timezone

import requests
Expand Down Expand Up @@ -56,6 +57,12 @@ class LogSeverity(object):
_TIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%f%z"
"""Time format for timestamps used in API"""

_TIME_RESTRICTION_RE = re.compile(
r"(?<![\w.])(?:receive)?timestamp\s*[<>=!:]", re.IGNORECASE
)
"""Matches a comparison on the top-level ``timestamp`` or ``receiveTimestamp``
field, but not on fields like ``jsonPayload.timestamp`` or ``timestamp_ms``."""

METADATA_URL = "http://metadata.google.internal./computeMetadata/v1/"
METADATA_HEADERS = {"Metadata-Flavor": "Google"}

Expand Down Expand Up @@ -146,6 +153,6 @@ def _add_defaults_to_filter(filter_):
time_filter = f'timestamp>="{yesterday.strftime(_TIME_FORMAT)}"'
if filter_ is None:
filter_ = time_filter
elif "timestamp" not in filter_.lower():
elif not _TIME_RESTRICTION_RE.search(filter_):
filter_ = f"{filter_} AND {time_filter}"
return filter_
28 changes: 28 additions & 0 deletions packages/google-cloud-logging/tests/unit/test__helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,34 @@ def test_filter_defaults_capitalized_timestamp(self):
out_filter = self._add_defaults_to_filter(in_filter)
self.assertEqual(in_filter, out_filter)

def test_filter_defaults_user_time_restriction(self):
"""Any comparison on timestamp or receiveTimestamp skips the default"""
test_inputs = [
'timestamp >= "2020-10-13T21:00:00Z"',
'severity>=ERROR AND timestamp<"2020-10-13"',
'NOT timestamp:"2020-10-13"',
'receiveTimestamp>"2020-10-13"',
]
for in_filter in test_inputs:
out_filter = self._add_defaults_to_filter(in_filter)
self.assertEqual(in_filter, out_filter)

def test_filter_defaults_timestamp_substring(self):
"""Mentioning timestamp elsewhere should still add the default"""
test_inputs = [
"jsonPayload.timestamp_ms>0",
'jsonPayload.timestamp>="2020-10-13"',
'textPayload:"timestamp"',
'labels.event_timestamp="x"',
]
for in_filter in test_inputs:
out_filter = self._add_defaults_to_filter(in_filter)
timestamp = datetime.strptime(
out_filter, in_filter + ' AND timestamp>="' + self._time_format() + '"'
)
Comment on lines +240 to +242

@gemini-code-assist gemini-code-assist Bot Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using datetime.strptime with the entire in_filter as part of the format string is fragile. If in_filter contains any % characters (which are common in search queries, e.g., percent-encoded characters or wildcards), strptime will attempt to interpret them as format directives and raise a ValueError.

Instead, extract the timestamp substring from out_filter first, and then parse only the timestamp portion.

Suggested change
timestamp = datetime.strptime(
out_filter, in_filter + ' AND timestamp>="' + self._time_format() + '"'
)
prefix = in_filter + ' AND timestamp>="'
self.assertTrue(out_filter.startswith(prefix))
self.assertTrue(out_filter.endswith('"'))
timestamp_str = out_filter[len(prefix):-1]
timestamp = datetime.strptime(timestamp_str, self._time_format())

yesterday = datetime.now(timezone.utc) - timedelta(days=1)
self.assertLess(yesterday - timestamp, timedelta(minutes=1))


class EntryMock(object):
def __init__(self):
Expand Down
Loading