-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
♻️ [RUM-6813] Split the recorder API module #3181
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3181 +/- ##
==========================================
- Coverage 93.46% 93.41% -0.06%
==========================================
Files 280 282 +2
Lines 7698 7715 +17
Branches 1723 1728 +5
==========================================
+ Hits 7195 7207 +12
- Misses 503 508 +5 ☔ View full report in Codecov by Sentry. |
Bundles Sizes Evolution
🚀 CPU Performance
🧠 Memory Performance
|
packages/rum/src/boot/recorderApi.ts
Outdated
if (configuration.startSessionReplayRecordingManually) { | ||
state.setStatus(RecorderStatus.Stopped) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought: I know this is not from your PR, but this is really incorrect, because it discards any previous state, so in the scenario below, Session Replay won't start even though it is started manually:
DD_RUM.startSessionReplayRecording()
DD_RUM.init({ startSessionReplayRecordingManually: true })
Extracted helper functions to check recorder status
/to-staging |
Devflow running:
|
Integrated commit sha: e103093 Co-authored-by: Aymeric Mortemousque <[email protected]>
Motivation
The recorderApi module is a bit large and we want to add more logic before starting to lazy load the recorder. It is a good time to extract the "pre start" logic like we did with RUM and Logs: #2575.
Changes
Strategy
interface for behaviors that differs before and after starting the SDK.createPreStartStrategy
to initialize the strategy before the SDK is startedcreatePostStartStrategy
to update the strategy after the SDK is startedTesting
I have gone over the contributing documentation.