RUM-6307 Make sure ConsentAwareFileOrchestrator is thread safe

[mariusc83]

What does this PR do?

We had an issue in iOS where switching from PENDING to GRANTED in the tracking consent asynchronously causes the loss of some events that are about to be written from a different queue. In this PR we are making sure we are covering this use case with integration tests and also that we mark the delegateOrchestrator as Volatile in the ConsentAwareFileOrchestrator in case the Executor is re - spawning the worker thread.

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)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.31%. Comparing base (7ebe827) to head (979fc06).
Report is 6 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2309      +/-   ##
===========================================
- Coverage    70.32%   70.31%   -0.01%     
===========================================
  Files          736      736              
  Lines        27466    27472       +6     
  Branches      4607     4607              
===========================================
+ Hits         19315    19316       +1     
+ Misses        6871     6865       -6     
- Partials      1280     1291      +11     

Files with missing lines	Coverage Δ
...ence/file/advanced/ConsentAwareFileOrchestrator.kt	96.77% <ø> (ø)

... and 28 files with indirect coverage changes

[jonathanmos]

empty When

[0xnm]

I'm not quite clear why do we need to spin a new thread for every write request we are making, why not put forEach call into a single dedicated thread? By using .join inside every .forEach call you are anyway making .withWriteContext invocations sequential.

[mariusc83]

I wanted to increase the potential of a collision, basically the worse case scenario.

[0xnm]

But that doesn't increase the chance of collision, isn't it? There is no race condition, because of the .join - all write requests are strictly sequential.

[mariusc83]

ah yes, you are totally right, using join there will not work as it will force the main thread to wait until the thread in progress is finished. I always confuse those :(. Need a CountdownLatch in there.

[0xnm]

There is a problem here: CountDownLatch is initialized with value 1, and it will be set to 0 after the first batch write, and once all the batches written, value will be negative.

It means, that countDownLatch.await(TimeUnit.SECONDS.toMillis(5), TimeUnit.MILLISECONDS) will be unblocked when first batch is written, not when all batches are written.

We won't notice an issue, because we also have ConditionWatcher which wait for the data, but I suppose if the original intention was "write at least one batch in pending mode and then continue with tracking consent switch", then this await should be moved inside:

 val switchConsentThread = Thread {
    Datadog.setTrackingConsent(TrackingConsent.GRANTED)
}

Probably the switchConsentThread.join() is not needed as well, if we are relying on the waiting ConditionWatcher (but having it doesn't hurt).