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

♻️ [RUM-2445] do not buffer early recorder API calls #2563

Merged
merged 1 commit into from Jan 15, 2024

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Jan 10, 2024

Motivation

The goal of this commit is to simplify the rumPublicApi module by using the recorderApi directly, without buffering the calls.

Changes

This is possible because the recorder api already has logic to support early calls to start and stop. The getSessionReplayLink implementation had to be slightly changed to work similarly to start and stop.

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

The goal of this commit is to simplify the rumPublicApi module by using
the `recorderApi` directly, without buffering the calls.

This is possible because the recorder api already has logic to support
early calls to `start` and `stop`. The `getSessionReplayLink`
implementation had to be slightly changed to work similarly to `start`
and `stop`.
@codecov-commenter
Copy link

Codecov Report

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

Comparison is base (4f28486) 92.85% compared to head (9db8ed4) 92.87%.

Files Patch % Lines
packages/rum/src/boot/recorderApi.ts 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2563      +/-   ##
==========================================
+ Coverage   92.85%   92.87%   +0.02%     
==========================================
  Files         228      228              
  Lines        6744     6737       -7     
  Branches     1480     1480              
==========================================
- Hits         6262     6257       -5     
+ Misses        482      480       -2     

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

@BenoitZugmeyer
Copy link
Member Author

/to-staging

@dd-devflow
Copy link

dd-devflow bot commented Jan 10, 2024

🚂 Branch Integration: this build is next, merge in < 7m

Commit 9db8ed4f81 will soon be integrated into staging-02.

This build is next! (estimated merge in less than 7m)

you can cancel this operation by commenting your pull request with /to-staging -c!

dd-mergequeue bot added a commit that referenced this pull request Jan 10, 2024
…tly into staging-02

Co-authored-by: Benoît Zugmeyer <benoit.zugmeyer@datadoghq.com>
@dd-devflow
Copy link

dd-devflow bot commented Jan 10, 2024

🚂 Branch Integration: This commit was successfully integrated

Commit 9db8ed4f81 has been merged into staging-02 in merge commit 62b23a1f57.

Check out the triggered pipeline on Gitlab 🦊

@BenoitZugmeyer BenoitZugmeyer marked this pull request as ready for review January 11, 2024 14:16
@BenoitZugmeyer BenoitZugmeyer requested review from a team as code owners January 11, 2024 14:16
@BenoitZugmeyer BenoitZugmeyer merged commit 5ecea20 into main Jan 15, 2024
18 checks passed
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/consent--use-recorder-api-directly branch January 15, 2024 14:00
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.

None yet

4 participants