Skip to content

RUMM-2684: Observe uncaught exception in executors#1125

Merged
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2684/observe_uncaught_exceptions_in_executors
Nov 7, 2022
Merged

RUMM-2684: Observe uncaught exception in executors#1125
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2684/observe_uncaught_exceptions_in_executors

Conversation

@0xnm

@0xnm 0xnm commented Nov 4, 2022

Copy link
Copy Markdown
Member

What does this PR do?

Turns out we were swallowing uncaught exceptions raised during the execution of the tasks submitted via Executor#execute, ExecutorService#submit or ScheduledExecutorService#schedule calls, because the behavior is like the following:

  • If uncaught exception is raised for task submitted via Executor#execute, then worker thread is killed and exception is printed to the console.
  • If uncaught exception is raised for task submitted via ExecutorService#submit or ScheduledExecutorService#schedule, then worker thread is kept running and exception is thrown on the thread is which doing Future#get call (normally the thread getting the result). We were always ignoring the Future returned after the task is submitted, so we were swallowing such exceptions.

This change closes this gap by adding the hook to the ThreadPoolExecutor#afterExecute method, which allows to observe such uncaught exceptions. ThreadPoolExecutor#afterExecute is called on the thread which did the actual task execution.

The logic of this hook is taken from the afterExecute documentation.

Exceptions will be sent to the devLogger and to the internal telemetry as well.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@0xnm
0xnm requested a review from a team as a code owner November 4, 2022 09:21
Base automatically changed from nogorodnikov/rumm-2700/use-eventwritecontext-for-logs to feature/sdkv2 November 4, 2022 09:47

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter

codecov-commenter commented Nov 4, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1125 (b241f40) into feature/sdkv2 (991ace4) will decrease coverage by 0.08%.
The diff coverage is 82.86%.

@@                Coverage Diff                @@
##           feature/sdkv2    #1125      +/-   ##
=================================================
- Coverage          82.52%   82.44%   -0.08%     
=================================================
  Files                347      349       +2     
  Lines              11355    11383      +28     
  Branches            1912     1914       +2     
=================================================
+ Hits                9370     9384      +14     
- Misses              1412     1419       +7     
- Partials             573      580       +7     
Impacted Files Coverage Δ
.../datadog/android/core/internal/thread/ThreadExt.kt 73.33% <66.67%> (-10.00%) ⬇️
...n/com/datadog/android/core/internal/CoreFeature.kt 91.13% <100.00%> (+0.07%) ⬆️
...in/com/datadog/android/core/internal/SdkFeature.kt 88.18% <100.00%> (ø)
...ernal/thread/LoggingScheduledThreadPoolExecutor.kt 100.00% <100.00%> (ø)
.../core/internal/thread/LoggingThreadPoolExecutor.kt 100.00% <100.00%> (ø)
...lin/com/datadog/android/rum/internal/RumFeature.kt 93.00% <100.00%> (ø)
...id/v2/core/internal/storage/ConsentAwareStorage.kt 98.89% <100.00%> (ø)
...android/log/internal/domain/DatadogLogGenerator.kt 95.50% <0.00%> (-2.70%) ⬇️
...rsistence/file/batch/PlainBatchFileReaderWriter.kt 86.27% <0.00%> (-1.96%) ⬇️
...ain/java/com/datadog/opentracing/PendingTrace.java 58.62% <0.00%> (-1.72%) ⬇️
... and 8 more

@xgouchet xgouchet added the size-medium This PR is medium sized label Nov 7, 2022
@0xnm
0xnm merged commit e3791e5 into feature/sdkv2 Nov 7, 2022
@0xnm
0xnm deleted the nogorodnikov/rumm-2684/observe_uncaught_exceptions_in_executors branch November 7, 2022 13:43
@xgouchet xgouchet added this to the 1.16.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size-medium This PR is medium sized

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants