feat!: remove issue creation from logging integrations#1340
Conversation
Co-Authored-By: GPT-5 <agent-email-or-noreply@example.com>
Semver Impact of This PR🔴 Major (breaking changes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Breaking Changes 🛠
New Features ✨
Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9afe9a6. Configure here.
|
|
||
| type SentryHandler struct { | ||
| eventHandler *eventHandler | ||
| logHandler *logHandler |
There was a problem hiding this comment.
AttrFromContext never applied
Medium Severity
Removing the event handler dropped the only call to contextExtractor for Option.AttrFromContext, while the option and README still promise context-derived attributes on log entries. logHandler.Handle builds attributes from the record only, so configured extractors never run and context metadata is silently omitted from Sentry logs.
Reviewed by Cursor Bugbot for commit 9afe9a6. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| errorDepth = client.Options().MaxErrorDepth | ||
| } | ||
| } | ||
| s.SetException(err, errorDepth) | ||
| } | ||
|
|
||
| key = h.key(FieldUser) | ||
| switch user := extra[key].(type) { | ||
| case sentry.User: | ||
| delete(extra, key) | ||
| s.User = user |
There was a problem hiding this comment.
Bug: After SetHubProvider is called, Flush uses the new hub's client, while the logger still uses the original client. This prevents buffered logs from being flushed.
Severity: HIGH
Suggested Fix
The sentryLogger should not store a static reference to the hub. Instead, it should use the hubProvider from the hook to get the current hub whenever it needs to log a message. This ensures that both logging and flushing operations use the same, up-to-date hub and client, even if the provider is changed after initialization.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: logrus/logrusentry.go#L196-L206
Potential issue: The `sentryLogger` within the `logHook` is initialized with a hub and
its associated client, and log entries are buffered in this client. The `Flush` method
uses a `hubProvider` to get the hub and client to flush. If a user calls
`SetHubProvider` to change the hub after initialization, `Flush` will attempt to flush
the new client. However, the logger retains its reference to the original client where
the logs are buffered. This discrepancy causes `Flush` to operate on a different client
than the one holding the logs, leading to silent data loss as buffered logs are never
sent.
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Description
This removes all deprecated issue creation functionality to prepare for 0.48.0.
Issues
Changelog Entry Instructions
To add a custom changelog entry, uncomment the section above. Supports:
For more details: custom changelog entries
Reminders
feat:,fix:,ref:,meta:)