Skip to content

Disable background worker when executing rake tasks#1509

Merged
st0012 merged 3 commits intomasterfrom
fix-#1508
Jul 23, 2021
Merged

Disable background worker when executing rake tasks#1509
st0012 merged 3 commits intomasterfrom
fix-#1508

Conversation

@st0012
Copy link
Copy Markdown
Contributor

@st0012 st0012 commented Jul 22, 2021

This fixes #1508

(I know disabling background worker by setting background_worker_threads = 0 looks hacky, but I don't want to introduce another config option for background toggling atm.)

@st0012 st0012 added this to the 4.7.0 milestone Jul 22, 2021
@st0012 st0012 self-assigned this Jul 22, 2021
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 22, 2021 •
edited by codecov bot
Loading

Codecov Report

❌ Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.74%. Comparing base (157f958) to head (3406ed1).
⚠️ Report is 775 commits behind head on master.

Files with missing lines Patch % Lines
sentry-ruby/lib/sentry/rake.rb 50.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1509      +/-   ##
==========================================
+ Coverage   98.21%   98.74%   +0.52%     
==========================================
  Files         218      123      -95     
  Lines       10560     6761    -3799     
==========================================
- Hits        10372     6676    -3696     
+ Misses        188       85     -103

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@st0012 st0012 mentioned this pull request Jul 22, 2021
Closed
@y-yagi
Copy link
Copy Markdown
Contributor

y-yagi commented Jul 23, 2021

@st0012 Thanks for your quick fix! But, unfortunately, this can't fix my issue.

Rake::Application#top_level calls before the initialization process of a Rails application. So if users configure Sentry inside an initializer(that mentioned in doc), Sentry.initialized? inside Rake::Application#top_level returns false always and not using Sentry.get_current_hub.with_background_worker_disabled.

@st0012
Copy link
Copy Markdown
Contributor Author

st0012 commented Jul 23, 2021

@y-yagi I see. I now see that the Rails rake task's initialization order is like this:

Rails rake_tasks block
top_level
Sentry SDK initializer
inside the task

I'll dig into this more.

@st0012
Copy link
Copy Markdown
Contributor Author

st0012 commented Jul 23, 2021

@y-yagi I've updated the fix. It should work for both vanilla rake and Rails rake tasks.

st0012 added 3 commits July 23, 2021 12:21
@st0012
Add Hub#with_background_worker_disabled
7bf5f9e 
This helper temporarily disables background event dispatching when
executing the block.
@st0012
Wrap rake task's execution with Hub#with_background_worker_disabled
6e462e6 
This makes sure all events, whether manually triggered or handled by the
SDK, can be sent synchronously.
@st0012
Update changelog
3406ed1
@y-yagi
Copy link
Copy Markdown
Contributor

y-yagi commented Jul 23, 2021

@st0012 Thanks! I confirmed this fixes my issue.

@st0012
Copy link
Copy Markdown
Contributor Author

st0012 commented Jul 23, 2021

@y-yagi sorry for the trouble and thanks for helping me verify the fix 🙂

@st0012 st0012 merged commit 4dc603b into master Jul 23, 2021
@st0012 st0012 deleted the fix-#1508 branch July 23, 2021 06:37
@st0012 st0012 modified the milestones: 4.7.0, 4.6.2 Jul 23, 2021
@y-yagi
Copy link
Copy Markdown
Contributor

y-yagi commented Jul 23, 2021

@st0012 No problem! Thanks for your quick fix ❤️

@st0012
Copy link
Copy Markdown
Contributor Author

st0012 commented Jul 23, 2021

@y-yagi fyi it's been released in version 4.6.2

@y-yagi y-yagi mentioned this pull request Jul 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@st0012 st0012

Labels

sentry-ruby

Projects

None yet

Milestone

4.6.2

Development

Successfully merging this pull request may close these issues.

Sentry.capture_message can't send a message from Rake

3 participants

@st0012 @codecov-commenter @y-yagi