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.
ℹ️ 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
| def filter(self, record: logging.LogRecord) -> bool: # type: ignore[override] | ||
| rollout_id = current_rollout_id.get() | ||
| if not rollout_id: | ||
| # No correlation context → do not emit to external sink | ||
| return False |
There was a problem hiding this comment.
The new ContextRolloutIdFilter returns False whenever current_rollout_id is unset, which prevents the handler from processing the record at all. This means that log records that explicitly supply a rollout ID (via extra={'rollout_id': …}) or rely on the handlers’ EP_ROLLOUT_ID environment-variable fallback will now be dropped before the handler can examine those values. Before this change, setting EP_ROLLOUT_ID was enough to route logs to Fireworks/Elasticsearch; after this commit, those logs silently disappear unless every call site wraps logging in rollout_logging_context. If support for env/explicit IDs is still required, the filter should treat an existing record.rollout_id or env fallback as valid rather than short‑circuiting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
https://github.com/codex address that feedback
1 participant
Uh oh!
There was an error while loading. Please reload this page.