Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(spotlight): Don't give up on Spotlight on 3 errors #3856

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

BYK
Copy link
Member

@BYK BYK commented Dec 5, 2024

Current Spotlight error handling logic gives up sending events to Spotlight after 3 errors. This doesn't make much sense because:

  1. Since there is no back off or retry mechanism, even a very brief server hiccup or restart turns off Spotlight reporting
  2. Once this shut off kicks in, there is no way to turn it back on except for a server restart

I added a note for future work for retries and some short buffer.

Current Spotlight error handling logic gives up sending events to Spotlight after 3 errors. This doesn't make much sense because:

1. Since there is no back off or retry mechanism, even a very brief server hiccup or restart turns off Spotlight reporting
2. Once this shut off kicks in, there is no way to turn it back on except for a server restart

I added a note for future work for retries and some short buffer.
@BYK BYK requested a review from sentrivana December 5, 2024 13:56
@BYK BYK enabled auto-merge (squash) December 5, 2024 13:56
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.94%. Comparing base (5891717) to head (b9c47db).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #3856       +/-   ##
===========================================
+ Coverage   66.07%   79.94%   +13.87%     
===========================================
  Files         137      137               
  Lines       15403    15399        -4     
  Branches     2618     2617        -1     
===========================================
+ Hits        10177    12311     +2134     
+ Misses       4346     2217     -2129     
+ Partials      880      871        -9     
Files with missing lines Coverage Δ
sentry_sdk/spotlight.py 56.36% <ø> (+1.10%) ⬆️

... and 42 files with indirect coverage changes

@BYK BYK merged commit 5a09770 into master Dec 5, 2024
135 checks passed
@BYK BYK deleted the byk/fix/spotlight-dont-giveup branch December 5, 2024 14:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants