RUM-386: Process feature context on the context thread#2704
Merged
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## nogorodnikov/rum-8170/feature-context-sync #2704 +/- ##
==============================================================================
- Coverage 69.98% 69.90% -0.09%
==============================================================================
Files 816 817 +1
Lines 30662 30721 +59
Branches 5146 5149 +3
==============================================================================
+ Hits 21458 21473 +15
- Misses 7768 7808 +40
- Partials 1436 1440 +4
🚀 New features to boost your workflow:
|
0xnm
force-pushed
the
nogorodnikov/rum-386/process-feature-context-on-context-thread
branch
2 times, most recently
from
June 6, 2025 06:34
52bda93 to
7536927
Compare
0xnm
marked this pull request as ready for review
June 6, 2025 07:18
ambushwork
approved these changes
Jun 6, 2025
| contextProvider = DatadogContextProvider(coreFeature) { | ||
| features.keys.map { | ||
| it to { | ||
| // already on the context thread |
Contributor
There was a problem hiding this comment.
Just a suggestion. No change needed.
For these things it might be worth having some sort of assertIsContextThread() functions that crash if it is not the right thread. And make them crash only in our sample app.
We could put these in many places.
I see that we currently have isDeveloperModeEnabled. But this could be used by the customer setUseDeveloperModeWhenDebuggable.
You might also want to crash in debug mode here instead of logging. But if we have some kind of monitors on these logs in our sample, it is fine.
Contributor
There was a problem hiding this comment.
instead of logging
in addition to logging
0xnm
force-pushed
the
nogorodnikov/rum-386/process-feature-context-on-context-thread
branch
from
June 6, 2025 09:26
7536927 to
6d10fd8
Compare
aleksandr-gringauz
approved these changes
Jun 6, 2025
0xnm
force-pushed
the
nogorodnikov/rum-386/process-feature-context-on-context-thread
branch
from
June 6, 2025 09:49
6d10fd8 to
bb9a9a8
Compare
ambushwork
approved these changes
Jun 6, 2025
0xnm
deleted the
nogorodnikov/rum-386/process-feature-context-on-context-thread
branch
June 11, 2025 09:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR changes the way feature context is read and updated. Now, by default, all calls to update feature context or to read it will delegate to the context processing thread, this will guarantee that sequence of the updates is corresponding to the sequence of the SDK API calls causing it.
There is also a way to update/read feature context from the caller thread, because sometimes calls are already done from the context thread (and sometimes we are ok doing such calls not from the context thread).
Since feature context is becoming a shared resource, it is guarded by the locks (write lock and read lock, read more about ReentrantReadWriteLock).
To avoid deadlock, write lock has a timeout during which it will try to acquire lock. If it is not possible to do during the timeout, update operation will be aborted.
This change now guarantees that, for example, there is the following API sequence:
Then there won't be a situation when Logs/Traces read Context A instead of Context B just because they are processed faster than RUM.
This invariant is covered by the integration test.
Review checklist (to be filled by reviewers)