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-2707: Add Resources feature #1840

Merged
merged 3 commits into from Feb 21, 2024

Conversation

jonathanmos
Copy link
Contributor

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
Copy link

codecov-commenter commented Jan 25, 2024

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
@jonathanmos jonathanmos force-pushed the jmoskovich/rum-2707/resources-request-feature branch from e60f49f to e00f01b Compare February 7, 2024 14:04
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.

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
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. added few comments, but they are not blocking.

sdkCore.setEventReceiver(SESSION_REPLAY_FEATURE_NAME, this)
if (RESOURCES_ENDPOINT_ENABLED) {
Copy link
Contributor

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?

Copy link
Contributor 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

@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
22 checks passed
@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants