Avoid blocking context call at the Flags feature initialization#3194
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8238cc317
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sdkCore.getFeature(Feature.FLAGS_FEATURE_NAME) | ||
| ?.withContext(withFeatureContexts = setOf(Feature.RUM_FEATURE_NAME)) { datadogContext -> |
There was a problem hiding this comment.
updateEvaluationsForContext always sets state to Reconciling, but the subsequent sdkCore.getFeature(...)? .withContext(...) call has no fallback path. If the feature scope is unavailable (for example after Datadog.stopInstance) or withContext skips its callback because core is no longer initialized, this method exits without calling onSuccess/onFailure and without moving state out of Reconciling, so callers can wait indefinitely for a completion that never comes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
If SDK / feature / context is not available, then we don't need anything.
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3194 +/- ##
===========================================
+ Coverage 71.26% 71.49% +0.23%
===========================================
Files 929 928 -1
Lines 34442 34421 -21
Branches 5813 5813
===========================================
+ Hits 24542 24607 +65
+ Misses 8255 8208 -47
+ Partials 1645 1606 -39
🚀 New features to boost your workflow:
|
typotter
left a comment
typotter
left a comment
There was a problem hiding this comment.
Thanks for the performance bump and refactoring!
| .put("sdk_version", flagsContext.sdkVersion) | ||
| .put("sdk_version", datadogContext.sdkVersion) | ||
|
|
||
| private val DatadogContext.rumApplicationId: String? |
4 participants
What does this PR do?
A bit of performance improvement during the Flags initialization. As it is seen on the following image, when Flags feature is initialized there is a blocking call to get SDK context:
It takes the majority of the Flags feature initialization time and makes Flags feature the slowest to initialize.
This PR moves getting SDK context to the worker thread when it is really needed.
As a result, initialization now is cheaper.
Review checklist (to be filled by reviewers)