Conversation
Replace the deprecated, unmaintained github.com/getsentry/raven-go SDK with the modern github.com/getsentry/sentry-go SDK. All changes are contained within the notifier package. Key changes: - InitSentry now uses sentry.Init with ClientOptions - Stack frame conversion builds sentry.Frame structs directly with proper Module/Function splitting for Sentry UI grouping - Event building extracted into buildSentryEvent helper, used by both doNotify and NotifyOnPanic - Environment/Release stored in package vars and stamped per-event so post-init calls to SetEnvironment/SetRelease take effect immediately - Close() now flushes sentry events with a 2s timeout No breaking changes to the public API.
📝 WalkthroughWalkthroughMigrates Sentry integration from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR migrates the notifier/ package from the deprecated github.com/getsentry/raven-go client to github.com/getsentry/sentry-go, updating Sentry initialization, event construction, shutdown flushing, and adding a dedicated Sentry test suite.
Changes:
- Replace Raven usage with Sentry-Go initialization and event capture, including a shared
buildSentryEventhelper. - Update shutdown behavior to flush Sentry events in
Close(). - Add Sentry-focused tests validating stacktrace conversion, level mapping, tags/extras, env/release, and panic reporting.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| notifier/notifier.go | Migrates runtime stack conversion + event building/capture to sentry-go, and flushes on close. |
| notifier/notifier_sentry_test.go | Adds a test harness capturing events via BeforeSend and validates Sentry event fields. |
| notifier/README.md | Updates generated function anchor links/line references to match moved code. |
| go.mod | Swaps raven-go dependency for sentry-go and updates indirect requirements. |
| go.sum | Removes Raven/certifi sums and adds Sentry-Go (and transitive) sums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
notifier/notifier.go (1)
184-191: Consider conditionalInAppbased on module path.All frames are marked
InApp: true, which tells Sentry to consider all frames as application code for grouping and display. This may cause noise from third-party library frames. Consider checking if the module path starts with your project's module prefix to setInAppappropriately.♻️ Suggested refinement
+// modulePrefix could be set via SetServerRoot or derived from go.mod +var modulePrefix = "github.com/go-coldbrew/" + frames = append(frames, sentry.Frame{ Function: function, Module: module, Filename: fr.File, AbsPath: fr.File, Lineno: fr.Line, - InApp: true, + InApp: strings.HasPrefix(module, modulePrefix), })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notifier/notifier.go` around lines 184 - 191, Update the sentry.Frame construction so InApp is set conditionally based on the module path instead of always true: when appending frames in the block that builds sentry.Frame (the frames = append(...) call), compute InApp by checking whether the module string starts with your project module prefix (e.g., using strings.HasPrefix(module, projectModulePrefix)) or similar heuristic and set sentry.Frame.InApp accordingly; ensure you reference the module variable and the sentry.Frame creation so third‑party frames are marked InApp=false while your code remains InApp=true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@notifier/notifier.go`:
- Around line 184-191: Update the sentry.Frame construction so InApp is set
conditionally based on the module path instead of always true: when appending
frames in the block that builds sentry.Frame (the frames = append(...) call),
compute InApp by checking whether the module string starts with your project
module prefix (e.g., using strings.HasPrefix(module, projectModulePrefix)) or
similar heuristic and set sentry.Frame.InApp accordingly; ensure you reference
the module variable and the sentry.Frame creation so third‑party frames are
marked InApp=false while your code remains InApp=true.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82262337-4a19-4239-946a-b242abd40b62
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.modnotifier/README.mdnotifier/notifier.gonotifier/notifier_sentry_test.go
- Update Close() doc comment to mention Sentry flush and 2s timeout - Avoid unnecessary map allocation in buildSentryEvent when tagData is empty - Use t.Cleanup in initTestSentry to reset globals automatically, removing manual defers from each test - Add t.Cleanup to TestInitSentry_ValidDSN for consistent state reset
There was a problem hiding this comment.
🧹 Nitpick comments (2)
notifier/notifier.go (1)
462-463: Minor: Hyphenate compound adjective."2 second timeout" should be "2-second timeout" per standard grammar rules for compound adjectives. This is the source for the auto-generated README.
Proposed fix
// Close closes the airbrake notifier and flushes pending Sentry events. -// Sentry events are flushed with a 2 second timeout. +// Sentry events are flushed with a 2-second timeout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notifier/notifier.go` around lines 462 - 463, Update the comment for the Close function to hyphenate the compound adjective: change the phrase "2 second timeout" to "2-second timeout" in the doc comment above the Close method in notifier.go so the generated README uses the correct grammar.notifier/README.md (1)
48-48: Minor grammar issue originates in source code.The phrase "2 second timeout" should be hyphenated as "2-second timeout" (compound adjective). Since this README is auto-generated from source comments, the fix should be applied to the doc comment in
notifier/notifier.goaround line 463.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@notifier/README.md` at line 48, Update the doc comment for the Close method in notifier/notifier.go (the comment that documents Close around line 463) to hyphenate the compound adjective by changing "2 second timeout" to "2-second timeout" so the generated README reads correctly; edit the comment text above the Close function declaration accordingly and regenerate docs if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@notifier/notifier.go`:
- Around line 462-463: Update the comment for the Close function to hyphenate
the compound adjective: change the phrase "2 second timeout" to "2-second
timeout" in the doc comment above the Close method in notifier.go so the
generated README uses the correct grammar.
In `@notifier/README.md`:
- Line 48: Update the doc comment for the Close method in notifier/notifier.go
(the comment that documents Close around line 463) to hyphenate the compound
adjective by changing "2 second timeout" to "2-second timeout" so the generated
README reads correctly; edit the comment text above the Close function
declaration accordingly and regenerate docs if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8c63747-b4f3-4ebd-a921-82232cd6fcc3
📒 Files selected for processing (3)
notifier/README.mdnotifier/notifier.gonotifier/notifier_sentry_test.go
✅ Files skipped from review due to trivial changes (1)
- notifier/notifier_sentry_test.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
github.com/getsentry/raven-gowith moderngithub.com/getsentry/sentry-goSDKnotifier/package — no public API changesbuildSentryEventhelper to deduplicate event construction betweendoNotifyandNotifyOnPanicsentry.FlushtoClose()for graceful shutdownnotifier_sentry_test.go)Test plan
make buildpassesmake testpasses (including 9 new Sentry tests)make lintpasses with 0 issuesgo.modno longer referencesraven-goorcertifi/gocertifimake buildandmake testpass (all 9 packages)Summary by CodeRabbit
Chores
Documentation
Tests