Skip to content
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

feat: Session Replay Dynamic Loading #832

Merged
merged 20 commits into from
Jan 9, 2024
Merged

feat: Session Replay Dynamic Loading #832

merged 20 commits into from
Jan 9, 2024

Conversation

metal-messiah
Copy link
Member

@metal-messiah metal-messiah commented Dec 6, 2023

Allow Session Replay modules to load earlier in the page lifecycle if a replay is ongoing

Overview

This PR enables earlier loading of SR recording libs by doing the following:

  1. Isolating all the recording-specific logic to a shared module
  • Abstracting the "meta data" to the event cycle itself instead of the top level class (things like hasError, hasMeta, etc.)
  • Creating a helper class to keep the cycle state easier to manage
  1. Checking the session state at load time to see if a recording is in progress (state 1 or 2)
  • Reads the local storage directly bypassing the need to set up the session entity
  1. Async Importing the new shared recording module in the SR instrumentation file if deemed allowed
  2. Passing the recorder along to the aggregate file if early imported
  3. using the passed recorder in the agg file if exists, if not, lazy loading the shared recorder and initializing it after page load

Changes to note:

  • Constants have been moved
  • Tests have been slightly modified to support the new constants structure and new method for getting events from the recorder
  • A problem was noted during this exercise:
    • The agg tried to "prevent" the harvester from harvesting if it estimated that the payload was too large. This was unreliable to rely on solely, so an extra step was added.
    • If the agg estimates that the payload is too large, now it will still try to compress it and re-check the payload size before aborting completely.
    • This required changing some assumptions in certain shut-down tests
  • It became evident that under our restrictions of a 1mb snapshot payload, that inline-images was never going to be reliable. The code supporting that (src code, tests) was ripped out.
  • SRI is unreliable and causes issues with SR chunks. It was commented out until a more reliable solution is reached

Related Issue(s)

https://new-relic.atlassian.net/browse/NR-186806

Testing

  • Existing tests have been modified to support the new architecture, but the test conditions should remain intact and pass
  • New test has been added to check if preloaded recorder captures and reports SR data

Copy link

github-actions bot commented Dec 6, 2023

Asset Size Report

Merging this pull request will result in the following asset size changes:

Agent Asset Previous Size New Size Diff
lite loader 30.36 kB / 10.54 kB (gzip) 30.55 kB / 10.66 kB (gzip) 0.64% / 1.12% (gzip)
lite async-chunk 45.68 kB / 15.09 kB (gzip) 45.63 kB / 15.04 kB (gzip) -0.12% / -0.34% (gzip)
pro loader 47.96 kB / 16.15 kB (gzip) 49.18 kB / 16.73 kB (gzip) 2.53% / 3.64% (gzip)
pro async-chunk 74.58 kB / 23.78 kB (gzip) 72.87 kB / 23.03 kB (gzip) -2.29% / -3.14% (gzip)
spa loader 54.42 kB / 18.13 kB (gzip) 55.63 kB / 18.71 kB (gzip) 2.23% / 3.23% (gzip)
spa async-chunk 89.35 kB / 28.18 kB (gzip) 87.64 kB / 27.43 kB (gzip) -1.92% / -2.66% (gzip)
lite-polyfills loader 122.19 kB / 39.6 kB (gzip) 122.43 kB / 39.72 kB (gzip) 0.2% / 0.31% (gzip)
lite-polyfills async-chunk 58.08 kB / 17.29 kB (gzip) 57.95 kB / 17.13 kB (gzip) -0.22% / -0.94% (gzip)
pro-polyfills loader 141.75 kB / 45.36 kB (gzip) 141.98 kB / 45.48 kB (gzip) 0.17% / 0.27% (gzip)
pro-polyfills async-chunk 101.85 kB / 27.38 kB (gzip) 101.67 kB / 27.49 kB (gzip) -0.18% / 0.4% (gzip)
spa-polyfills loader 149.77 kB / 47.5 kB (gzip) 150.01 kB / 47.62 kB (gzip) 0.16% / 0.24% (gzip)
spa-polyfills async-chunk 117.42 kB / 31.97 kB (gzip) 117.23 kB / 32.47 kB (gzip) -0.15% / 1.56% (gzip)

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (9ad6312) 78.37% compared to head (ead2c01) 78.59%.

Files Patch % Lines
src/features/session_replay/aggregate/index.js 93.33% 4 Missing and 1 partial ⚠️
src/features/session_replay/shared/recorder.js 93.67% 5 Missing ⚠️
src/features/session_replay/instrument/index.js 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #832      +/-   ##
==========================================
+ Coverage   78.37%   78.59%   +0.21%     
==========================================
  Files         142      144       +2     
  Lines        6359     6410      +51     
  Branches     1229     1250      +21     
==========================================
+ Hits         4984     5038      +54     
+ Misses       1169     1167       -2     
+ Partials      206      205       -1     
Flag Coverage Δ
integration-tests 88.60% <95.28%> (+0.13%) ⬆️
unit-tests 55.32% <84.35%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metal-messiah metal-messiah changed the title feat: [WIP] Session Replay Dynamic Loading feat: Session Replay Dynamic Loading Dec 8, 2023
@metal-messiah metal-messiah marked this pull request as ready for review December 8, 2023 17:53
Copy link

github-actions bot commented Dec 8, 2023

Static Badge

Last ran on January 09, 2024 11:42:14 CST
Checking merge of (ead2c01) into main (9ad6312)

Copy link
Contributor

@patrickhousley patrickhousley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor comments but otherwise looks good.

@metal-messiah metal-messiah merged commit 1af7b89 into main Jan 9, 2024
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants