[codex] fix runtime guard stale log filtering - #188
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0a120c95a
ℹ️ 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".
| for key in ("runtime_target_enabled", "RUNTIME_TARGET_ENABLED"): | ||
| if key in target: | ||
| return _coerce_bool(target.get(key), True) | ||
| if key in runtime_target: | ||
| return _coerce_bool(runtime_target.get(key), True) |
There was a problem hiding this comment.
When CLOUD_RUN_SERVICE_TARGETS_JSON uses the same schema as the env-sync plan, RUNTIME_TARGET_ENABLED can live under a target's env block or defaults, but this new guard only checks the top-level target and runtime_target. In that configuration, e.g. {"env":{"RUNTIME_TARGET_ENABLED":"false"}}, _load_services() still includes the disabled service and the runtime guard continues querying and alerting on it.
Useful? React with 👍 / 👎.
| service_since | ||
| for service, service_since in service_since_by_name.items() | ||
| if any(alias and alias in job_name for alias in _service_job_aliases(service)) | ||
| ] | ||
| return max(matches) if matches else fallback |
There was a problem hiding this comment.
When multiple monitored services share a prefix, such as foo-service and foo-bar-service, this substring match makes foo-bar-service-scheduler match both foo and foo-bar-service; because the function then uses max(matches), a real failure for foo-bar-service before foo's later revision time is skipped as stale. Match the job id to a concrete scheduler name or alias boundary instead of using unbounded substring containment.
Useful? React with 👍 / 👎.
| text = str(value).strip().lower() | ||
| if not text: | ||
| return default | ||
| return text in {"1", "true", "yes", "y", "on"} |
There was a problem hiding this comment.
When a target has a typo such as "RUNTIME_TARGET_ENABLED":"treu" or "runtime_target_enabled":"maybe", this helper treats the value as false, so _load_services() silently drops the target and the guard can report OK instead of surfacing a configuration error. The same setting is validated elsewhere as true/false, so invalid target JSON should not disable monitoring.
Useful? React with 👍 / 👎.
| entry_timestamp = _parse_timestamp(entry.get("timestamp")) | ||
| entry_since = _scheduler_entry_since(entry, service_since_by_name, since) | ||
| if entry_timestamp and entry_timestamp < entry_since: |
There was a problem hiding this comment.
When RUNTIME_GUARD_SCHEDULER_JOB_PATTERN is configured for a job name that does not contain the Cloud Run service alias, entries matching that explicit pattern reach this loop but _scheduler_entry_since() falls back to the global lookback window. In that setup, scheduler failures from before the service's latest-ready revision are still reported as current, so the stale-log filter does not work for custom scheduler job names.
Useful? React with 👍 / 👎.
| if not _target_enabled(target): | ||
| continue |
There was a problem hiding this comment.
When CLOUD_RUN_SERVICE or CLOUD_RUN_SERVICES is still set for a service that is disabled in CLOUD_RUN_SERVICE_TARGETS_JSON, the service has already been appended before this continue, so the guard still queries and alerts on it. Since the runtime-guard workflow supplies the legacy service envs alongside target JSON, disabling a target will not suppress monitoring unless those legacy vars are also cleared.
Useful? React with 👍 / 👎.
1 participant
Summary
Test Plan