fix: seed RUM instanceId on the render thread to avoid a cross-thread race#192
Conversation
e0cbe04 to
1d1134e
Compare
… race
RumAgent runs as a Task. Its setup() generated the SDK instanceId and wrote
the whole datadogRumContext to m.global via setField at task startup. Because
the field was only added on the render thread moments earlier, the task's
write could run before that addition had propagated, producing:
Tried to set nonexistent field "datadogRumContext" of a "Node" node
and, on the losing side of the race, dropping the instanceId from the context.
That id keys the `_last_view_<instanceId>` file used to attach the last viewed
screen to crash reports, so losing it silently breaks crash-to-view attribution.
Fix: generate the instanceId on the render thread in initialize(), seed the
fully-populated datadogRumContext (applicationId + instanceId) before the
RumAgent task is created, and pass the id to RumAgent via a new interface
field. setup() now reads it instead of writing the context cross-thread. The
instanceId lifecycle is unchanged: still one fresh id per initialize().
Adds a regression test asserting initialize() seeds datadogRumContext.instanceId
synchronously and that the RumAgent receives the same id.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1d1134e to
26e85ad
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26e85ad586
ℹ️ 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".
| m.rumScope = fields.rumScope 'allow tests to inject a mock | ||
| m.telemetryScope = fields.telemetryScope 'allow tests to inject a mock | ||
| m.applicationId = fields.applicationId | ||
| m.instanceId = fields.instanceId |
There was a problem hiding this comment.
Restore UUID fallback for unset RumAgent instance IDs
When a RumAgent is created directly rather than through datadogroku_initialize (which the existing RumAgent unit setup does, and the component is shipped in dist), this field is never populated. Since setup no longer generates a UUID, m.instanceId becomes empty/invalid; sendCrash then passes a non-unique id to RumCrashReporterTask and there is no seeded RUM context for view scopes, so last-view files are not keyed per SDK initialization and crash-to-view attribution is lost for those instances. Keep the injected value, but fall back to GetRandomUUID() when fields.instanceId is unset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We don't support direct creation of RumAgent, and configuring the sdk by calling datadogroku_initialize is well documented in the README file.
What
RumAgentruns as aTask. Itssetup()generated the SDKinstanceIdand wrote the wholedatadogRumContexttom.globalviasetFieldat task startup. Because the field had only been added on the render thread moments earlier (ininitialize()), the task's write could run before that addition had propagated, producing:On the losing side of the race the
instanceIdcould be dropped from the context. That id keys the_last_view_<instanceId>file used to attach the last viewed screen to crash reports (RumViewScopewrites it,RumCrashReporterTaskreads it), so losing it silently breaks crash-to-view attribution.Fix
instanceIdon the render thread ininitialize().datadogRumContext(applicationId+instanceId) before theRumAgenttask is created.RumAgentvia a newinstanceIdinterface field;setup()now reads it instead of writing the context cross-thread (the racysetFieldis gone).No cross-thread write at startup → no warning, and
instanceIdcan no longer be dropped.This also reduced rendevzous on sample app start from 49 -> 48.