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-2709: Implement resource capture during traversal #1854

Merged
merged 4 commits into from Feb 21, 2024

Conversation

jonathanmos
Copy link
Contributor

@jonathanmos jonathanmos commented Feb 14, 2024

What does this PR do?

Captures the bitmaps during traversal and writes them to batches

Motivation

Third PR of the Resource endpoint

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@jonathanmos jonathanmos changed the base branch from develop to jmoskovich/rum-2707/resources-request-feature February 14, 2024 08:10
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2024

Codecov Report

Merging #1854 (79cacc9) into develop (077adb0) will increase coverage by 0.06%.
The diff coverage is 88.71%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1854      +/-   ##
===========================================
+ Coverage    83.23%   83.29%   +0.06%     
===========================================
  Files          478      479       +1     
  Lines        16884    16934      +50     
  Branches      2519     2526       +7     
===========================================
+ Hits         14052    14104      +52     
  Misses        2136     2136              
+ Partials       696      694       -2     
Files Coverage Δ
...adog/android/sessionreplay/SessionReplayPrivacy.kt 100.00% <100.00%> (ø)
...dog/android/sessionreplay/SessionReplayRecorder.kt 95.12% <100.00%> (+0.46%) ⬆️
...android/sessionreplay/internal/ResourcesFeature.kt 88.89% <100.00%> (ø)
...oid/sessionreplay/internal/SessionReplayFeature.kt 94.59% <100.00%> (ø)
...nreplay/internal/net/ResourceRequestBodyFactory.kt 63.46% <100.00%> (+0.72%) ⬆️
...sionreplay/internal/net/ResourcesRequestFactory.kt 72.00% <100.00%> (ø)
...ssionreplay/internal/processor/EnrichedResource.kt 90.48% <ø> (ø)
...nreplay/internal/async/RecordedDataQueueHandler.kt 98.04% <91.67%> (+0.26%) ⬆️
...eplay/internal/recorder/base64/Base64Serializer.kt 96.77% <94.74%> (+0.13%) ⬆️
...roid/sessionreplay/internal/utils/DrawableUtils.kt 97.06% <87.50%> (-1.38%) ⬇️
... and 1 more

... and 24 files with indirect coverage changes

@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2709/record-resources branch 2 times, most recently from 3194f1a to de66c39 Compare February 14, 2024 12:13
@jonathanmos jonathanmos marked this pull request as ready for review February 14, 2024 12:13
@jonathanmos jonathanmos requested review from a team as code owners February 14, 2024 12:13
@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2709/record-resources branch 2 times, most recently from 1d3c889 to 35c1f37 Compare February 14, 2024 12:43
Copy link
Contributor

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

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

I went though the production files and left some questions, didn't go through the tests yet.

Copy link
Collaborator

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

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

I partially reviewed it but would need first some answers to figure it out the rest.

Copy link
Collaborator

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

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

Once @0xnm comments will be addressed I am ok to approve it. For me the biggest blocker was the Base64Serializer but as you said it will be addressed in the next PR.

@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2709/record-resources branch from 03efd64 to a31af69 Compare February 15, 2024 15:52
Copy link
Contributor

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

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

lgtm! I left some comments, but nothing blocking.

@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch from 604b282 to 92da6ed Compare February 21, 2024 12:24
@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2709/record-resources branch from ef39f54 to 7dcc78c Compare February 21, 2024 12:29
Base automatically changed from jmoskovich/rum-2707/resources-request-feature to develop February 21, 2024 13:10
@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2709/record-resources branch from 7dcc78c to 79cacc9 Compare February 21, 2024 13:42
@jonathanmos jonathanmos merged commit 1e65536 into develop Feb 21, 2024
23 checks passed
@jonathanmos jonathanmos deleted the jmoskovich/rum-2709/record-resources branch February 21, 2024 14:43
@xgouchet xgouchet added this to the 2.7.0 milestone Apr 5, 2024
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.

None yet

5 participants