Skip to content

fix(logging): retain default window for incidental timestamp text - #18713

Open
AliAbouelazm wants to merge 2 commits into
googleapis:mainfrom
AliAbouelazm:fix/logging-timestamp-default-17338
Open

AliAbouelazm wants to merge 2 commits into
googleapis:mainfrom
AliAbouelazm:fix/logging-timestamp-default-17338

Conversation

@AliAbouelazm

Copy link
Copy Markdown

Fixes #17338.

The default 24-hour window was skipped whenever a filter contained timestamp, including JSON field names, labels, and quoted text. This change recognizes the actual timestamp field while keeping strings, escaped quotes, nested paths, and comments separate.

Existing timestamp comparisons and documented cast(timestamp, ...) expressions keep their behavior. Appended defaults also stay outside trailing comments, and comment-only filters receive a valid time constraint.

Validation:

  • 1,385 unit tests and 61 subtests pass on Python 3.12 and 3.14 with both protobuf implementations.
  • The changed helper has 100% statement and branch coverage. Aggregate coverage remains above the current 99% CI threshold and does not decrease from the baseline.
  • Ruff import/format checks, Flake8, setup metadata validation, package build, and Twine checks pass.
  • All tests ran offline. No live Google Cloud calls or resources were used.

No documentation change is needed because this restores the documented default time window.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request refactors the _add_defaults_to_filter helper in google-cloud-logging to use a regular expression-based tokenizer (_FILTER_TOKENS) for parsing logging filters. This ensures that default timestamp constraints are only skipped when there is an actual comparison or cast on the timestamp field, avoiding false positives from occurrences inside strings, comments, or other field paths. It also adds comprehensive unit tests to verify these scenarios. Feedback suggests handling empty or whitespace-only filters to prevent generating invalid filter strings starting with AND.

Comment thread packages/google-cloud-logging/google/cloud/logging_v2/_helpers.py Outdated
@AliAbouelazm
AliAbouelazm marked this pull request as ready for review October 16, 2026 07:47
@AliAbouelazm
AliAbouelazm requested a review from a team as a code owner October 16, 2026 07:47
@parthea parthea self-assigned this Oct 16, 2026
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Oct 16, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 16, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

list_entries: a filter containing the substring "timestamp" silently disables the default 24-hour window

3 participants