Repository navigation
fix(sentry): ignore LLM logging and use Sentry logs - #638
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors Sentry initialization to ignore the "agent.trajectory" logger alongside "agent.redis", and integrates Sentry logging into the custom logging setup. However, the use of sentry_sdk.logger.info will cause a runtime AttributeError as the sentry_sdk module does not expose a logger attribute; this should be updated to use sentry_sdk.capture_message instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if len(lines) > 1 and _buffered_handler is not None: | ||
| _buffered_handler.begin_group() | ||
| try: | ||
| sentry_sdk.logger.info(s) |
There was a problem hiding this comment.
Using sentry_sdk.logger.info(s) will raise an AttributeError at runtime because the sentry_sdk module does not expose a logger attribute. To explicitly send a log message to Sentry, use sentry_sdk.capture_message(s, level="info") instead.
| sentry_sdk.logger.info(s) | |
| sentry_sdk.capture_message(s, level="info") |
| if len(lines) > 1 and _buffered_handler is not None: | ||
| _buffered_handler.begin_group() | ||
| try: | ||
| sentry_sdk.logger.info(s) |
There was a problem hiding this comment.
I'm still not convinced we need this. Before my PR, this wasn't logged at all, and I think the LiteLLMIntegration already covers most of it.
There was a problem hiding this comment.
so are you more for skipping it altogether?
There was a problem hiding this comment.
and I think the
LiteLLMIntegrationalready covers most of it.
this is broken :/ I don't really know why, but if you check the “Conversations” in Sentry, there's nothing, even though I was able to find the chats from the spans :D I don't really get what's wrong with it, the only difference in between what is done in Ymir and what they suggest on the dashboard is the conversation ID:
import sentry_sdk
# Call this at the start of each conversation
sentry_sdk.ai.set_conversation_id("my-conversation-123")OTOH when you check the docs, they don't use that :D
I can probably check the versions if it's not because of that, but IDK.
There was a problem hiding this comment.
so are you more for skipping it altogether?
I'm fine either way, it could be beneficial to have more context if the "conversations" don't work.
There was a problem hiding this comment.
OK, I can disable for now, since it wasn't logged before and we can decide on the follow up?
Especially after looking at the failed CI, it needs the Sentry SDK too, so it's more annoying than I expected
There was a problem hiding this comment.
Ignoring for now, will either open a follow-up issue or a PR right away.
Based on the changes in the later mentioned PR, the current logging outputs logged strings line-by-line which causes issues with the Sentry logging integration as, for example, the dumped JSONs produce entries even for closing tokens such as `}`. As it has been discussed, it is not as trivial change, as was expected, to adjust the logging, therefore ignore these logs on Sentry side altogether for now. Related to packit#630 Signed-off-by: Matej Focko <mfocko@packit.dev>
14d3b59 to
b872c20
Compare
3 participants
Based on the changes in the later mentioned PR, the current logging outputs logged strings line-by-line which causes issues with the Sentry logging integration as, for example, the dumped JSONs produce entries even for closing tokens such as
}.Therefore adjust the setup to ignore the logging from the affected logger and log explicitly via Sentry SDK, so that it is formatted properly.
Related to #630