RUM-2780: Return no-op core when instance is already initialized#3271
RUM-2780: Return no-op core when instance is already initialized#3271ambushwork wants to merge 1 commit intodevelopfrom
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3271 +/- ##
===========================================
+ Coverage 71.38% 71.42% +0.04%
===========================================
Files 942 942
Lines 34823 34822 -1
Branches 5901 5901
===========================================
+ Hits 24857 24871 +14
+ Misses 8297 8283 -14
+ Partials 1669 1668 -1
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1efebaba2d
ℹ️ 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".
| { MESSAGE_ALREADY_INITIALIZED } | ||
| ) | ||
| return existing | ||
| return NoOpInternalSdkCore |
There was a problem hiding this comment.
Return existing core on duplicate initialization
Returning NoOpInternalSdkCore here breaks initialize() idempotency for callers that keep and reuse the returned SdkCore (for example, val core = Datadog.initialize(...); Rum.enable(..., core)). In a common defensive pattern where a library calls initialize() after the app already did, the second call now hands back a no-op core, so subsequent feature registration and user/context updates are silently dropped even though a real instance is already active.
Useful? React with 👍 / 👎.
hamorillo
left a comment
There was a problem hiding this comment.
🚀 LGTM! As you mentioned in the ticket, we need to block merging this PR until v4, though.
| * @param context your application context | ||
| * @param configuration the configuration for the SDK library | ||
| * @param trackingConsent as the initial state of the tracking consent flag | ||
| * @return the initialized SDK instance, or null if something prevents the SDK from |
There was a problem hiding this comment.
nitpick: Should we add that we can now return NoOpInternalSdkCore?
What does this PR do?
we should return a no-op instance and prevent leaks between orgs.
Motivation
RUM-2780
Review checklist (to be filled by reviewers)