Filter runtime guard logs by ready revision#220
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c2223911d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log_filter = ( | ||
| 'resource.type="cloud_run_revision" ' | ||
| f'AND resource.labels.service_name="{service}" ' | ||
| f'AND timestamp >= "{since_text}"' | ||
| f'AND timestamp >= "{service_since_text}"' |
There was a problem hiding this comment.
Filter by the ready revision name
With the default RUNTIME_GUARD_IGNORE_PRE_READY_REVISION_LOGS=true, this query only tightens the timestamp bound and still reads every revision for the service. When a newly deployed Cloud Run revision emits ERROR logs but never becomes ready, latestReadyRevisionName remains on the previous revision, so those non-ready revision logs still match this filter and continue to trigger alerts/failures. Carry the ready revision name into the log filter, e.g. via resource.labels.revision_name, or otherwise exclude non-ready revisions.
Useful? React with 👍 / 👎.
| metadata = revision_payload.get("metadata") or {} | ||
| if not isinstance(metadata, dict): | ||
| return None | ||
| return _parse_timestamp(metadata.get("creationTimestamp")) |
There was a problem hiding this comment.
Use the Ready transition time as the cutoff
creationTimestamp is when the Revision object was created, not when it became Ready. If a revision logs startup/probe errors after creation and then eventually becomes the latest ready revision, those pre-ready logs are still after this cutoff and will be reported despite RUNTIME_GUARD_IGNORE_PRE_READY_REVISION_LOGS being enabled. Use the revision Ready condition transition time, or an equivalent readiness timestamp, as the lower bound instead.
Useful? React with 👍 / 👎.
| ) | ||
|
|
||
| for service in services: | ||
| service_since = _cloud_run_log_since(project, service, since) if ignore_pre_ready_logs else since |
There was a problem hiding this comment.
Keep the success check on the requested lookback
When RUNTIME_GUARD_REQUIRE_SUCCESS=true, this same narrowed query is also the only source for success_count. After a new ready revision is created, any successful requests earlier in the configured lookback window but before service_since are ignored, so the guard can emit no successful Cloud Run request ... in the last N minutes even though such a request exists. Use the original since window for the require-success check, or update the alert semantics to explicitly require success after the ready revision cutoff.
Useful? React with 👍 / 👎.
Summary
Validation