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

[PROF-3285] User-friendly handling of slow submissions on shutdown #1601

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Jul 19, 2021

dd-trace-rb installs a VM at_exit handler that allows it to shut down all of its background tasks cleanly, including attempting to submit and remaining traces and profiles.

This finishing of background tasks is already protected by timeouts (to make sure we don't just hang if talking to the agent is slow), but can still take a few seconds.

When a user presses ctrl+c, they usually expect their process to finish in a timely manner, and may assume the process hung if it doesn't finish immediately. To clarify what's going on (and tell users that they can press ctrl+c again to force an exit), I've added a nice message that is printed whenever shut down takes longer than 200ms.

Whenever shutdown is fast enough, no message is printed, to avoid extra noise.

dd-trace-rb installs a VM `at_exit` handler that allows it to shut down
all of its background tasks cleanly, including attempting to submit
and remaining traces and profiles.

This finishing of background tasks is already protected by timeouts (to
make sure we don't just hang if talking to the agent is slow), but can
still take a few seconds.

When a user presses ctrl+c, they usually expect their process to finish
in a timely manner, and may assume the process hung if it doesn't
finish immediately. To clarify what's going on (and tell users that
they **can** press ctrl+c again to force an exit), I've added a nice
message that is printed whenever shut down takes longer than 200ms.

Whenever shutdown is fast enough, no message is printed, to avoid
extra noise.
@ivoanjo ivoanjo requested review from dbenamydd and a team July 19, 2021 09:50
@codecov-commenter
Copy link

Codecov Report

Merging #1601 (df44afb) into master (0ef7c5f) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1601      +/-   ##
==========================================
- Coverage   98.27%   98.27%   -0.01%     
==========================================
  Files         900      900              
  Lines       43118    43185      +67     
==========================================
+ Hits        42375    42440      +65     
- Misses        743      745       +2     
Impacted Files Coverage Δ
lib/ddtrace.rb 98.50% <0.00%> (-1.50%) ⬇️
spec/ddtrace/profiling/ext/forking_spec.rb 99.38% <0.00%> (-0.62%) ⬇️
lib/ddtrace/profiling.rb 100.00% <0.00%> (ø)
lib/ddtrace/configuration.rb 100.00% <0.00%> (ø)
spec/ddtrace/profiling_spec.rb 100.00% <0.00%> (ø)
spec/ddtrace/configuration_spec.rb 100.00% <0.00%> (ø)
spec/ddtrace/profiling/integration_spec.rb 94.77% <0.00%> (+0.06%) ⬆️

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 006048c...df44afb. Read the comment docs.

@marcotc
Copy link
Member

marcotc commented Jul 21, 2021

Very nice, user friendly improvement!

@marcotc marcotc merged commit 9190785 into master Jul 21, 2021
@marcotc marcotc deleted the ivoanjo/userfriendly-slow-shutdown branch July 21, 2021 23:04
@github-actions github-actions bot added this to the 0.52.0 milestone Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants