Skip to content

[RUM-9342] Add FGM ( Fine Grained Masking ) support#860

Merged
cdn34dd merged 1 commit into
developfrom
carlosnogueira/RUM-9342/fine-grained-masking
May 19, 2025
Merged

[RUM-9342] Add FGM ( Fine Grained Masking ) support#860
cdn34dd merged 1 commit into
developfrom
carlosnogueira/RUM-9342/fine-grained-masking

Conversation

@cdn34dd
Copy link
Copy Markdown
Contributor

@cdn34dd cdn34dd commented Apr 24, 2025

What does this PR do?

This PR makes Fine Grained Masking available on React Native by exposing Specific UI elements that can be used to wrap parts of an app in order to override the original privacy values.

Link to the RFC: here

These elements are available under the namespace SessionReplayView and can be imported directly from the session replay package.

There are a total of four different elements exposed:

SessionReplayView.Privacy → Works exactly the same as a regular RN View with the addition of having access to four privacy properties: textAndInputPrivacy, imagePrivacy, touchPrivacy, hide, which can be used to customize the privacy level of child components

SessionReplayView.MaskAll → As the name implies, sets the most restrictive access to all privacy options, where all of the options are set to .MaskAll or the equivalent counterpart. This component exposes to the user a showTouches prop of type boolean, so that they can still override touchPrivacy here.

SessionReplayView.MaskNone → Same as the component above, but has the least restrictive privacy options enabled by default (.MaskNone or equivalent). This component has no properties exposed to users.

SessionReplayView.Hide → This components hides all it’s children from session replay.

Additional Notes

In order to support both old and new architectures on android and iOS, there was the need to implement a Native UI component for each architecture on both android and iOS.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • 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)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

Comment thread packages/react-native-session-replay/src/types/DdView.ts Outdated
@cdn34dd cdn34dd changed the title Add FGM ( Fine Grained Masking ) support [RUM-9342] Add FGM ( Fine Grained Masking ) support May 2, 2025
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch 2 times, most recently from 74419e6 to 45c5208 Compare May 7, 2025 16:58
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented May 7, 2025

Datadog Report

Branch report: carlosnogueira/RUM-9342/fine-grained-masking
Commit report: 067b833
Test service: dd-sdk-reactnative

✅ 0 Failed, 669 Passed, 1 Skipped, 3.67s Total Time

@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch from 45c5208 to 0e4eb76 Compare May 8, 2025 09:19
Comment thread example-new-architecture/ios/Podfile.lock Outdated
Comment thread packages/react-native-session-replay/DatadogSDKReactNativeSessionReplay.podspec Outdated
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch 2 times, most recently from 605a935 to a13de0b Compare May 12, 2025 12:45
@datadog-datadog-prod-us1
Copy link
Copy Markdown

datadog-datadog-prod-us1 Bot commented May 12, 2025

Datadog Summary

❌ Code Quality    ✅ Code Security    ✅ Dependencies

Next Steps

Fix these code quality issues introduced by this PR:

Notice: kotlin-code-style/no-empty-lead-line-class
packages/react-native-session-replay/android/src/rnpre74/kotlin/com/datadog/reactnative/sessionreplay/DdSDKReactNativeSessionReplayPackage.kt:18

No blank lines at the start of a class

Notice: typescript-best-practices/boolean-prop-naming
packages/react-native-session-replay/src/specs/DdViewNativeComponent.ts:8

Consistent naming for boolean props

Notice: kotlin-best-practices/final-newline
packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/views/DdView.kt:59

Enforce final newline

and 3 more. View all

Test Optimization Report

Branch report: carlosnogueira/RUM-9342/fine-grained-masking
Commit report: aa9cf47
Test service: dd-sdk-reactnative

✅ 0 Failed, 669 Passed, 1 Skipped, 3.84s Total Time


Was this helpful? Give us feedback!

Comment thread packages/react-native-session-replay/src/specs/DdViewNativeComponent.ts Outdated
Comment thread packages/react-native-session-replay/src/types/DdView.ts Outdated
@cdn34dd cdn34dd marked this pull request as ready for review May 12, 2025 13:52
@cdn34dd cdn34dd requested a review from a team as a code owner May 12, 2025 13:52
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch 2 times, most recently from a09d98f to 964c89e Compare May 13, 2025 15:50
sbarrio
sbarrio previously approved these changes May 14, 2025
Copy link
Copy Markdown
Contributor

@sbarrio sbarrio left a comment

Choose a reason for hiding this comment

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

Nicely done! 👏

@marco-saia-datadog marco-saia-datadog self-requested a review May 15, 2025 14:59
Comment thread packages/react-native-session-replay/DatadogSDKReactNativeSessionReplay.podspec Outdated
Comment thread packages/react-native-session-replay/DatadogSDKReactNativeSessionReplay.podspec Outdated
Comment on lines +152 to +155
if (reactNativeMinorVersion >= 74) {
java.srcDirs += ['src/rnpost74/kotlin']
} else {
java.srcDirs += ['src/rnpre74/kotlin']
}
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.

Why do we need two different implementations for these versions? What are the implications of using just BaseReactPackage as we do in the core package?

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.

The reason I am asking is because we are already differentiating between old-arch and new-arch, and between different RN versions. I wouldn't introduce additional source sets if not strictly necessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes I'm aware, but this has to do with deprecations between versions of RN, the way to support UI and Native modules became different after version 74, and we need to support both versions in order to have it build everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@marco-saia-datadog I've looked into this a bit further and while the version of the code we have for the pre74 version should in theory work everywhere, I don't think we should use it.

I checked what the RN code base does, and when comparing the ReactPackage class with the BaseReactPackage class they look pretty similar, but BaseReactPackage has getReactModuleInfoProvider, which we override to provide the module data explicitly.

Then in ReactPackageTurboModuleManagerDelegate.java when we use BaseReactPackage, we just make use of getReactModuleInfoProvider to initialize our module, as shown here.

If we use ReactPackage instead RN will rely on reflection and slow down our module initialization instead, as shown here.

So BaseReactPackage is essentially an optimization done to support new arch, and I think we should keep the split to make sure we're not slowing down our sdk.

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.

Thank you for looking into this 💪 Let's keep it for now then, but in my opinion we should keep an eye on this, as the slow-down might not be significant enough to justify the additional complexity in the codebase.

Comment thread packages/react-native-session-replay/android/build.gradle Outdated
Comment thread react-native.config.js Outdated
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch from 8248315 to ad8932a Compare May 19, 2025 06:31
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-9342/fine-grained-masking branch from ad8932a to 067b833 Compare May 19, 2025 07:14
@marco-saia-datadog marco-saia-datadog self-requested a review May 19, 2025 09:46
@cdn34dd cdn34dd merged commit b27f636 into develop May 19, 2025
8 checks passed
@cdn34dd cdn34dd deleted the carlosnogueira/RUM-9342/fine-grained-masking branch May 19, 2025 10:25
@cdn34dd cdn34dd mentioned this pull request May 22, 2025
4 tasks
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.

3 participants