Skip to content

RUM-2709: Implement resource capture during traversal#1854

Merged
jonathanmos merged 4 commits into
developfrom
jmoskovich/rum-2709/record-resources
Feb 21, 2024
Merged

RUM-2709: Implement resource capture during traversal#1854
jonathanmos merged 4 commits into
developfrom
jmoskovich/rum-2709/record-resources

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Feb 14, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

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

codecov-commenter commented Feb 14, 2024 •
edited
Loading

Copy link
Copy Markdown

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
0xnm
0xnm reviewed Feb 14, 2024
View reviewed changes

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

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.

Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResource.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResource.kt Outdated
Comment thread ...src/main/kotlin/com/datadog/android/sessionreplay/internal/net/ResourceRequestBodyFactory.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/async/NoopDataQueueHandler.kt
Comment thread ...eplay/src/test/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeatureTest.kt Outdated
mariusc83
mariusc83 reviewed Feb 15, 2024
View reviewed changes

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

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.

mariusc83
mariusc83 reviewed Feb 15, 2024
View reviewed changes

@mariusc83 mariusc83 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

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
0xnm
0xnm approved these changes Feb 15, 2024
View reviewed changes

@0xnm 0xnm left a comment

Copy link
Copy Markdown
Member

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.

Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResource.kt Outdated
Comment thread ...c/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/Base64Serializer.kt Outdated
Comment thread ...test/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandlerTest.kt Outdated
Comment thread ...src/test/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResourceTest.kt Outdated
@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
@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

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@0xnm 0xnm 0xnm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.7.x

Development

Successfully merging this pull request may close these issues.

5 participants

@jonathanmos @codecov-commenter @mariusc83 @0xnm @xgouchet