Skip to content
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

Allow toggling background sending on the fly #1447

Merged
merged 2 commits into from
May 20, 2021

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented May 20, 2021

In some cases, being able to disable background sending globally is crucial for both users and the SDK itself.

For example, Rails supports having different modes for running the application (server, console, runner, task...etc). In the runner or task mode, the process is killed right after script is returned, usually before the background worker have the chance to send the events. Here's an example issue: #1324

Although Rails provides callbacks for different modes, they're ran after the SDK initialization. And under the current setup, it's not possible to temporarily bypass background sending globally past the SDK initialization stage. So we can't disable the background workers in those callbacks.

To solve the problem, this commit makes it possible to bypass background worker even after it's been initialized.

In some cases, being able to disable background sending globally is
crucial for both users and the SDK itself.

For example, Rails supports having different modes for running the
application (server, console, runner, task...etc). In the runner or task mode,
the process is killed right after script is returned, usually before the
background worker have the chance to send the events. Here's an example
issue: #1324

Although Rails provides callbacks for different modes, they're ran after
the SDK initialization. And under the current setup, it's not possible
to temporarily bypass background sending globally past the
SDK initialization stage. So we can't disable the background workers
in those callbacks.

To solve the problem, this commit makes it possible to bypass background
worker even after it's been initialized.
@st0012 st0012 added this to the 4.5.0 milestone May 20, 2021
@st0012 st0012 self-assigned this May 20, 2021
@st0012 st0012 added this to In progress in 4.x via automation May 20, 2021
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2021

Codecov Report

Merging #1447 (6086d17) into master (5ce32c1) will increase coverage by 0.61%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1447      +/-   ##
==========================================
+ Coverage   98.22%   98.83%   +0.61%     
==========================================
  Files         213      118      -95     
  Lines       10034     6202    -3832     
==========================================
- Hits         9856     6130    -3726     
+ Misses        178       72     -106     
Impacted Files Coverage Δ
sentry-ruby/lib/sentry/client.rb 97.18% <100.00%> (ø)
...ntry-ruby/spec/sentry/client/event_sending_spec.rb 99.51% <100.00%> (+0.01%) ⬆️
sentry-raven/lib/raven/base.rb
...en/lib/raven/processor/removecircularreferences.rb
sentry-raven/lib/raven/interface.rb
sentry-raven/lib/raven/utils/request_id.rb
...entry-raven/lib/raven/integrations/rack-timeout.rb
...b/raven/integrations/sidekiq/cleanup_middleware.rb
sentry-raven/spec/raven/integrations/rack_spec.rb
sentry-raven/lib/raven/event.rb
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ce32c1...6086d17. Read the comment docs.

@st0012 st0012 merged commit 87bc697 into master May 20, 2021
4.x automation moved this from In progress to Done May 20, 2021
@st0012 st0012 deleted the allow-toggling-background-on-the-fly branch May 20, 2021 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.x
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants