Skip to content

RUM-2707: Add Resources feature#1840

Merged
jonathanmos merged 3 commits into
developfrom
jmoskovich/rum-2707/resources-request-feature
Feb 21, 2024
Merged

RUM-2707: Add Resources feature#1840
jonathanmos merged 3 commits into
developfrom
jmoskovich/rum-2707/resources-request-feature

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Jan 25, 2024

Copy link
Copy Markdown
Member

What does this PR do?

Adds a new resource feature, created inside the session replay feature, in order to handle resources captured during traversal

Motivation

Part 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 force-pushed the jmoskovich/rum-2707/resources-request-feature branch from 2a1c280 to 54160e9 Compare January 25, 2024 09:17
@codecov-commenter

codecov-commenter commented Jan 25, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1840 (92da6ed) into develop (cbd637a) will increase coverage by 0.07%.
Report is 52 commits behind head on develop.
The diff coverage is 86.84%.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #1840      +/-   ##
===========================================
+ Coverage    83.21%   83.28%   +0.07%     
===========================================
  Files          470      475       +5     
  Lines        16632    16721      +89     
  Branches      2494     2503       +9     
===========================================
+ Hits         13839    13925      +86     
- Misses        2103     2105       +2     
- Partials       690      691       +1
Files Coverage Δ
...dog/android/sessionreplay/SessionReplayRecorder.kt 94.81% <100.00%> (+0.07%) ⬆️
...ay/internal/async/ResourceRecordedDataQueueItem.kt 100.00% <100.00%> (ø)
...nreplay/internal/net/ResourceRequestBodyFactory.kt 62.75% <ø> (ø)
...replay/internal/processor/RecordedDataProcessor.kt 96.25% <100.00%> (+0.30%) ⬆️
...sessionreplay/internal/storage/NoOpRecordWriter.kt 100.00% <ø> (ø)
...sionreplay/internal/storage/NoOpResourcesWriter.kt 100.00% <100.00%> (ø)
...play/internal/storage/SessionReplayRecordWriter.kt 100.00% <100.00%> (ø)
...y/internal/storage/SessionReplayResourcesWriter.kt 90.00% <90.00%> (ø)
...android/sessionreplay/internal/ResourcesFeature.kt 88.89% <88.89%> (ø)
...ssionreplay/internal/processor/EnrichedResource.kt 90.48% <90.48%> (ø)
... and 1 more

... and 32 files with indirect coverage changes

@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch 3 times, most recently from 03d02e0 to e60f49f Compare January 29, 2024 12:21
@jonathanmos
jonathanmos marked this pull request as ready for review January 29, 2024 13:00
@jonathanmos
jonathanmos requested review from a team as code owners January 29, 2024 13:00
0xnm
0xnm reviewed Feb 5, 2024
View reviewed changes
Comment thread ...ession-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/ResourcesFeature.kt Outdated
Comment thread ...-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/ResourceWriter.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResource.kt Outdated
Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt Outdated
Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt Outdated
Comment thread ...ession-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/ResourcesFeature.kt Outdated
Comment thread ...ession-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/ResourcesFeature.kt
Comment thread ...ession-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/ResourcesFeature.kt Outdated
Comment thread ...kotlin/com/datadog/android/sessionreplay/internal/async/ResourceRecordedDataQueueItemTest.kt Outdated
Comment thread ...kotlin/com/datadog/android/sessionreplay/internal/storage/SessionReplayResourceWriterTest.kt
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch from e60f49f to e00f01b Compare February 7, 2024 14:04
mariusc83
mariusc83 reviewed Feb 8, 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.

LGTM apart from @0xnm comments

@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2704/resource-request-builder branch from 39be041 to 67c70a6 Compare February 8, 2024 14:08
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch 3 times, most recently from 11c1fc4 to 4dd4279 Compare February 12, 2024 08:40
0xnm
0xnm approved these changes Feb 13, 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. added few comments, but they are not blocking.

Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt Outdated
this.appContext = appContext

sdkCore.setEventReceiver(SESSION_REPLAY_FEATURE_NAME, this)
if (RESOURCES_ENDPOINT_ENABLED) {

@0xnm 0xnm Feb 13, 2024

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.

it is always false, so I guess I guess for now it is a feature flag?

@jonathanmos jonathanmos Feb 13, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes - it's a ff that we'll remove in a later pr

Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt
Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt
Comment thread ...on-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/SessionReplayFeature.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/EnrichedResource.kt Outdated
Comment thread ...on-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/ResourcesFeatureTest.kt
Comment thread ...otlin/com/datadog/android/sessionreplay/forge/ResourceRecordedDataQueueItemForgeryFactory.kt Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch 2 times, most recently from e5670ac to 604b282 Compare February 13, 2024 16:23
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2704/resource-request-builder branch from 23467e0 to cbd637a Compare February 21, 2024 12:20
@jonathanmos
jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch from 604b282 to 92da6ed Compare February 21, 2024 12:24
Base automatically changed from jmoskovich/rum-2704/resource-request-builder to develop February 21, 2024 12:43
@jonathanmos
jonathanmos merged commit 077adb0 into develop Feb 21, 2024
@jonathanmos
jonathanmos deleted the jmoskovich/rum-2707/resources-request-feature branch February 21, 2024 13:10
@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

@0xnm 0xnm 0xnm approved these changes
@mariusc83 mariusc83 Awaiting requested review from mariusc83
+1 more reviewer
@xgouchet xgouchet xgouchet approved these changes
Reviewers whose approvals may not affect merge requirements

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 @xgouchet @mariusc83 @0xnm