Skip to content

RUM-1194 DataUploadWorker is scheduled every time and on non - roaming network#1647

Merged
mariusc83 merged 1 commit into
developfrom
mconstanting/rum-1194/schedule-upload-worker-all-the-time
Sep 29, 2023
Merged

RUM-1194 DataUploadWorker is scheduled every time and on non - roaming network#1647
mariusc83 merged 1 commit into
developfrom
mconstanting/rum-1194/schedule-upload-worker-all-the-time

Conversation

@mariusc83

@mariusc83 mariusc83 commented Sep 28, 2023

Copy link
Copy Markdown
Member

What does this PR do?

Previously our data upload worker was scheduled only if the device was offline and on any type of network. We are currently scheduling this all the time and only if the network is unmetered (WIFI or any other form of unmetered network)
We also updated the default inBackground state in the BatchMetricsDispatcher as true to handle correctly the metrics sent from the scheduled upload worker.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

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)

@mariusc83 mariusc83 self-assigned this Sep 28, 2023
val workManager = WorkManager.getInstance(context)
val constraints = Constraints.Builder()
.setRequiredNetworkType(NetworkType.CONNECTED)
.setRequiredNetworkType(NetworkType.UNMETERED)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

should we let it go no matter what the network type is ? I am thinking in situations where the client is on ROAMING and we are sending data under the hood which might not be wise.

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.

Looks like WorkManager has a constraint of NOT_ROAMING - could we use this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

oh good catch...we could indeed use that which is better, we care about ROAMING specifically here

@mariusc83
mariusc83 force-pushed the mconstanting/rum-1194/schedule-upload-worker-all-the-time branch from 6a6741b to 0d28be2 Compare September 28, 2023 13:48
@mariusc83
mariusc83 marked this pull request as ready for review September 28, 2023 13:48
@mariusc83
mariusc83 requested a review from a team as a code owner September 28, 2023 13:48
@codecov-commenter

codecov-commenter commented Sep 28, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1647 (ef7b0c0) into develop (54ae3fc) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1647   +/-   ##
========================================
  Coverage    83.68%   83.68%           
========================================
  Files          456      456           
  Lines        15696    15690    -6     
  Branches      2347     2345    -2     
========================================
- Hits         13135    13130    -5     
- Misses        1927     1935    +8     
+ Partials       634      625    -9     
Files Coverage Δ
...ain/kotlin/com/datadog/android/core/DatadogCore.kt 84.07% <ø> (-0.09%) ⬇️
...ore/internal/lifecycle/ProcessLifecycleCallback.kt 88.89% <100.00%> (+3.17%) ⬆️
...id/core/internal/metrics/BatchMetricsDispatcher.kt 98.53% <100.00%> (ø)
...og/android/core/internal/utils/WorkManagerUtils.kt 90.91% <100.00%> (ø)

... and 16 files with indirect coverage changes

@mariusc83
mariusc83 force-pushed the mconstanting/rum-1194/schedule-upload-worker-all-the-time branch from 0d28be2 to 0a866d5 Compare September 28, 2023 14:41
@mariusc83
mariusc83 force-pushed the mconstanting/rum-1194/schedule-upload-worker-all-the-time branch from 0a866d5 to ef7b0c0 Compare September 28, 2023 14:45
@mariusc83 mariusc83 changed the title RUM-1194 DataUploadWorker is scheduled every time and on unmetered network RUM-1194 DataUploadWorker is scheduled every time and on non - roaming network Sep 28, 2023
@mariusc83
mariusc83 merged commit 2876dee into develop Sep 29, 2023
@mariusc83
mariusc83 deleted the mconstanting/rum-1194/schedule-upload-worker-all-the-time branch September 29, 2023 08:54
@xgouchet xgouchet added this to the 2.2.0 milestone Dec 13, 2023
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.

5 participants