-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Remove lock name from LockRetry error message to prevent Sentry flooding #668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…flooding The lock name includes repo ID, commit ID, and lock type, creating unique error messages for each combination. This causes Sentry to create separate error groups instead of grouping them together, making it difficult to track the actual issue. Changes: - Removed lock name from the error message string in LockRetry exception - Lock name is still available as an exception attribute (lock_name) - Lock name is still captured in Sentry tags for searchability - Lock name is still logged in structured logging (extra fields) This allows Sentry to properly group all "lock acquisition failed" errors together while still maintaining full searchability via tags.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #668 +/- ##
=======================================
Coverage 92.38% 92.38%
=======================================
Files 1295 1295
Lines 47688 47688
Branches 1592 1592
=======================================
Hits 44056 44056
Misses 3323 3323
Partials 309 309
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…try grouping The previous fix (PR #668) removed the lock name but left repo and commit in the error message. Since each repo/commit combination is unique, Sentry was still creating separate issues for each occurrence. Now the message is generic - all identifying info remains available in: - Exception attributes (lock_name, repoid, commitid) - Sentry context (lock_acquisition) - Sentry tags (lock_name, lock_type) Fixes CCMRG-2031
…try grouping The previous fix (PR #668) removed the lock name but left repo and commit in the error message. Since each repo/commit combination is unique, Sentry was still creating separate issues for each occurrence. Now the message is generic - all identifying info remains available in: - Exception attributes (lock_name, repoid, commitid) - Sentry context (lock_acquisition) - Sentry tags (lock_name, lock_type) Fixes CCMRG-2031
…try grouping The previous fix (PR #668) removed the lock name but left repo and commit in the error message. Since each repo/commit combination is unique, Sentry was still creating separate issues for each occurrence. Now the message is generic - all identifying info remains available in: - Exception attributes (lock_name, repoid, commitid) - Sentry context (lock_acquisition) - Sentry tags (lock_name, lock_type) Fixes CCMRG-2031
Problem
The lock acquisition error message was including the lock name, which contains:
bundle_analysis_processing)19963992)347f87344b5dd1f29de2ddccbf43b98e45b62093)bundle_analysis)This creates a unique error message for every repo/commit combination, causing Sentry to create separate error groups for what is essentially the same issue. This floods Sentry with thousands of "unique" errors when it's really just multiple occurrences of the same lock acquisition failure.
Example before:
Example after:
Solution
exc.lock_name)tags.lock_name)extra.lock_name)This allows Sentry to properly group all "lock acquisition failed" errors together while maintaining full searchability and debuggability via tags and attributes.
Test Plan
Note
Simplifies lock acquisition failure messaging for better grouping.
LockRetryerror string to omitlock_namewhen max retries are exceeded, retainingrepoidandcommitidWritten by Cursor Bugbot for commit 56ba2a4. This will update automatically on new commits. Configure here.