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.

Sign up for GitHub

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

Jump to bottom

RUM-7176 [SR] Record SwiftUI raster images #2123

Merged
merged 3 commits into from
Dec 3, 2024
Merged

RUM-7176 [SR] Record SwiftUI raster images #2123

merged 3 commits into from
Dec 3, 2024

Conversation

mariedm
Copy link
Member

@mariedm mariedm commented Nov 27, 2024 •
edited
Loading

What and why?

This PR introduces support for recording SwiftUI raster images in Session Replay.

How?

  • Leverages reflection to inspect SwiftUI.GraphicsImage and extract CGImage.
  • Implements heuristics similar to Android, categorizing images smaller than 100 points as "likely bundled.".
  • Generates an Image wireframe for bundled images and a Placeholder wireframe for likely bundled images.
  • Adds unit tests for image reflection and heuristics

Note: Global privacy settings and overrides are not yet applied and will be addressed in separate PRs.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines [internal]) and run make api-surface)

@mariedm mariedm changed the base branch from develop to maxep/sr/swift-ui November 27, 2024 10:00
@mariedm mariedm changed the title mariedm/rum 7176 record swftui images RUM-7176 [SR] Record SwiftUI rasterized images Nov 27, 2024
@mariedm mariedm changed the title RUM-7176 [SR] Record SwiftUI rasterized images RUM-7176 [SR] Record SwiftUI raster images Nov 27, 2024
@mariedm mariedm force-pushed the mariedm/rum-7176-record-swftui-images branch 2 times, most recently from a1b5d33 to 89ed044 Compare November 27, 2024 14:27
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Nov 27, 2024 •
edited
Loading

Datadog Report

Branch report: mariedm/rum-7176-record-swftui-images
Commit report: 6c6220f
Test service: dd-sdk-ios

✅ 0 Failed, 3570 Passed, 0 Skipped, 2m 26.68s Total Time
🔻 Test Sessions change in coverage: 5 decreased, 4 increased, 5 no change

🔻 Code Coverage Decreases vs Default Branch (5)

  • test DatadogCoreTests iOS 71.76% (-0.08%) - Details
  • test DatadogRUMTests tvOS 81.19% (-0.03%) - Details
  • test DatadogInternalTests iOS 80.38% (-0.02%) - Details
  • test DatadogCrashReportingTests tvOS 26.66% (-0.02%) - Details
  • test DatadogTraceTests tvOS 54.24% (-0.02%) - Details

mariedm
mariedm commented Nov 27, 2024
View reviewed changes
...ewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/SwiftUI/SwiftUIWireframesBuilder.swift
)
}
case .unknown:
return context.builder.createPlaceholderWireframe(
Copy link
Member Author

Choose a reason for hiding this comment

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

We could send some telemetry to gather which image types we don't support.

Copy link
Member

Choose a reason for hiding this comment

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

We already have a ticket for adding telemetry 👍

@mariedm mariedm force-pushed the mariedm/rum-7176-record-swftui-images branch from 89ed044 to 150267a Compare November 28, 2024 09:59
@mariedm mariedm marked this pull request as ready for review November 28, 2024 10:11
@mariedm mariedm requested review from a team as code owners November 28, 2024 10:11
@mariedm mariedm force-pushed the mariedm/rum-7176-record-swftui-images branch 2 times, most recently from 064c86c to f7f21c2 Compare November 28, 2024 13:34
@mariedm mariedm requested a review from maxep November 28, 2024 16:08
@mariedm mariedm force-pushed the mariedm/rum-7176-record-swftui-images branch 2 times, most recently from 36b8751 to f5e26dc Compare November 28, 2024 16:47
@mariedm mariedm changed the base branch from maxep/sr/swift-ui to develop November 28, 2024 16:53
@mariedm mariedm force-pushed the mariedm/rum-7176-record-swftui-images branch from f5e26dc to 952023c Compare December 2, 2024 12:58
maxep
maxep reviewed Dec 2, 2024
View reviewed changes
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

Looks great! I left some comment.

...Sources/Recorder/ViewTreeSnapshotProducer/ViewTreeSnapshot/NodeRecorders/SwiftUI/Image.swift Outdated Show resolved Hide resolved
DatadogSessionReplay/Sources/Recorder/Utilities/SwfitUIImage+SessionReplay.swift Outdated Show resolved Hide resolved
DatadogSessionReplay/Tests/Mocks/ReflectionMocks.swift Outdated Show resolved Hide resolved
maxep
maxep previously approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

Nice, well done!

DatadogSessionReplay/Sources/Recorder/Utilities/CGImage+SessionReplay.swift Outdated Show resolved Hide resolved
@mariedm
Copy link
Member Author

mariedm commented Dec 3, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Dec 3, 2024 •
edited
Loading

Devflow running: /merge

View all feedbacks in Devflow UI.

2024-12-03 11:46:20 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-12-03 12:26:51 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in develop is 28m.

2024-12-03 12:27:23 UTC ℹ️ MergeQueue: This merge request was already merged

This pull request was merged directly.

@mariedm mariedm merged commit a4c6a1c into develop Dec 3, 2024
17 checks passed
@mariedm mariedm deleted the mariedm/rum-7176-record-swftui-images branch December 3, 2024 12:27
@maxep maxep mentioned this pull request Dec 11, 2024
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxep maxep maxep approved these changes

Assignees
No one assigned
Labels
mergequeue-status: done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mariedm @maxep