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

🐛 Adjust records generated during view change so their date matches the view date #1486

Merged

Conversation

amortemousque
Copy link
Collaborator

Motivation

Currently, when a new view is generated we get the current timestamp to define the view date.
Then we take a full snapshot, which consists in 3-4 records every time a record is emitted, we get the current timestamp to define its date

This causes an issue during replay, when seeking: when the user clicks on a View event, the player actually shows the previous view, because the records corresponding to the new view are a few milliseconds after the view start.

This PR set the view date to the fullsnapshot emitted records when a view change

Changes

Set the view date to the fullsnapshot emitted records when a view change

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@amortemousque amortemousque requested review from a team as code owners April 5, 2022 15:04
@amortemousque amortemousque changed the title 🐛 Set the view date to the fullsnapshot emitted records when a view change 🐛 Adjust records generated during view change so their date matches the view date Apr 5, 2022
@amortemousque amortemousque force-pushed the aymeric/adjust-records-generated-date-matches-view-date branch from 2c2b029 to 5c47382 Compare April 5, 2022 15:06
packages/rum/src/boot/startRecording.spec.ts Outdated Show resolved Hide resolved
packages/rum/src/types.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2022

Codecov Report

Merging #1486 (5fbfe2b) into main (63aac10) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1486   +/-   ##
=======================================
  Coverage   89.93%   89.93%           
=======================================
  Files         107      107           
  Lines        4302     4302           
  Branches      957      958    +1     
=======================================
  Hits         3869     3869           
  Misses        433      433           
Impacted Files Coverage Δ
packages/rum/src/domain/record/types.ts 100.00% <ø> (ø)
packages/rum/src/types.ts 100.00% <ø> (ø)
packages/rum/src/boot/startRecording.ts 100.00% <100.00%> (ø)
packages/rum/src/domain/record/record.ts 75.00% <100.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@amortemousque amortemousque force-pushed the aymeric/adjust-records-generated-date-matches-view-date branch from 5fbfe2b to fa6dfe3 Compare April 6, 2022 16:01
@amortemousque amortemousque merged commit 85e732d into main Apr 7, 2022
@amortemousque amortemousque deleted the aymeric/adjust-records-generated-date-matches-view-date branch April 7, 2022 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants