Issue #1327: apply resourceEventMapper to auto tracked XHR resources#1328
Open
teplymax wants to merge 1 commit into
Open
Conversation
686202b to
440d774
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates React Native RUM resource instrumentation so resourceEventMapper can sanitize or drop auto-tracked XHR resources before native RUM startResource is invoked, aligning auto-tracked behavior with manually tracked resources and improving privacy controls (e.g., URL PII redaction).
Changes:
- Apply
resourceEventMapperearlier in the resource lifecycle (notably inDdRum.startResource) and propagate mapper updates to active XHR tracking. - Extend resource event shapes/types to include
resourceContextand exportResourceEventfor mapper compatibility. - Update XHR auto-tracking pipeline (
XHRProxy/ResourceReporter) and add/adjust unit tests for the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/XHRProxy.ts | Pass mapper into ResourceReporter and support partial tracking updates (sample rate vs mapper). |
| packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/DatadogRumResource/ResourceReporter.ts | Convert internal RUMResource to mapper-compatible shape; run mappers and allow URL override via resourceContext.responseURL. |
| packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/DatadogRumResource/internalDevResourceBlocklist.ts | Update mapper typing to match the new ResourceMapper signature. |
| packages/core/src/rum/instrumentation/resourceTracking/requestProxy/XHRProxy/DatadogRumResource/tests/ResourceReporter.test.ts | Add coverage for mapper-driven URL sanitization and runtime mapper updates. |
| packages/core/src/rum/instrumentation/resourceTracking/requestProxy/interfaces/RequestProxy.ts | Allow partial updates in onTrackingUpdate (optional sample rate + optional mapper). |
| packages/core/src/rum/instrumentation/resourceTracking/DdRumResourceTracking.tsx | Accept mapper at tracking start and add updateResourceEventMapper forwarding to the proxy. |
| packages/core/src/rum/instrumentation/resourceTracking/tests/DdRumResourceTracking.test.ts | Add coverage for registering/unregistering resource mapper after tracking starts. |
| packages/core/src/rum/eventMappers/resourceEventMapper.ts | Export ResourceEvent and include resourceContext in the native mapper type path. |
| packages/core/src/rum/DdRum.ts | Apply mapper at startResource (using resourceContext.responseURL) and forward mapper updates to resource tracking. |
| packages/core/src/rum/tests/DdRum.test.ts | Update expectations for drop behavior (no native start/stop when mapper returns null). |
| packages/core/src/DdSdkReactNative.tsx | Pass configured resourceEventMapper into DdRumResourceTracking.startTracking. |
| packages/core/src/tests/DdSdkReactNative.test.tsx | Adjust expectations for the updated resource tracking initialization and start behavior. |
Comments suppressed due to low confidence (1)
packages/core/src/rum/tests/DdRum.test.ts:1680
- Current tests only cover the case where the resource mapper returns null for both start and stop. Add a test for the case where the mapper keeps the event at
startResource()but returns null atstopResource()(e.g., based on statusCode), to ensure the native resource is still properly stopped/dropped and doesn't leak.
it('does not start or stop the resource if the mapper returns null', async () => {
const resourceEventMapper: ResourceEventMapper = resource => {
return null;
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| getTracingContext, | ||
| getTracingContextForPropagators | ||
| } from './instrumentation/resourceTracking/distributedTracing/distributedTracingHeaders'; | ||
| import { DdRumResourceTracking } from './instrumentation/resourceTracking/DdRumResourceTracking'; |
Comment on lines
519
to
+523
| registerResourceEventMapper(resourceEventMapper: ResourceEventMapper) { | ||
| this.resourceEventMapper = generateResourceEventMapper( | ||
| resourceEventMapper | ||
| ); | ||
| DdRumResourceTracking.updateResourceEventMapper(resourceEventMapper); |
Comment on lines
285
to
+289
| return bufferVoidNativeCall(() => | ||
| this.nativeRum.startResource( | ||
| key, | ||
| method, | ||
| url, | ||
| encodeAttributes(context), | ||
| startUrl, |
Comment on lines
319
to
321
| if (!mappedEvent) { | ||
| /** | ||
| * To drop the resource we call `stopResource` and pass the `_dd.drop_resource` attribute in the context. | ||
| * It will be picked up by the resource mappers we implement on the native side that will drop the resource. | ||
| * This ensures we don't have any "started" resource left in memory on the native side. | ||
| */ | ||
| return bufferVoidNativeCall(() => | ||
| this.nativeRum.stopResource( | ||
| key, | ||
| statusCode, | ||
| kind, | ||
| size, | ||
| { | ||
| '_dd.resource.drop_resource': true | ||
| }, | ||
| timestampMs | ||
| ) | ||
| ); | ||
| return generateEmptyPromise(); | ||
| } |
Comment on lines
+39
to
48
| for (const mapper of this.getMappers()) { | ||
| const mappedResource = mapper(modifiedResource); | ||
| if (mappedResource === null) { | ||
| return; | ||
| } | ||
| modifiedResource = { | ||
| ...modifiedResource, | ||
| ...mappedResource | ||
| }; | ||
| } |
Comment on lines
+314
to
321
| /** | ||
| * We never drop at `stopResource`: the keep/drop decision was already | ||
| * made at `startResource`. A resource dropped there was never started, | ||
| * so this stop is a native no-op. | ||
| */ | ||
| if (!mappedEvent) { | ||
| /** | ||
| * To drop the resource we call `stopResource` and pass the `_dd.drop_resource` attribute in the context. | ||
| * It will be picked up by the resource mappers we implement on the native side that will drop the resource. | ||
| * This ensures we don't have any "started" resource left in memory on the native side. | ||
| */ | ||
| return bufferVoidNativeCall(() => | ||
| this.nativeRum.stopResource( | ||
| key, | ||
| statusCode, | ||
| kind, | ||
| size, | ||
| { | ||
| '_dd.resource.drop_resource': true | ||
| }, | ||
| timestampMs | ||
| ) | ||
| ); | ||
| return generateEmptyPromise(); | ||
| } |
Comment on lines
+39
to
48
| for (const mapper of this.getMappers()) { | ||
| const mappedResource = mapper(modifiedResource); | ||
| if (mappedResource === null) { | ||
| return; | ||
| } | ||
| modifiedResource = { | ||
| ...modifiedResource, | ||
| ...mappedResource | ||
| }; | ||
| } |
Comment on lines
34
to
37
| /** | ||
| * Filters RN symbolicate calls and Expo logs calls that happen only in dev. | ||
| * @param resource RUMResource | ||
| */ |
Comment on lines
250
to
+265
| startResource = ( | ||
| key: string, | ||
| method: string, | ||
| url: string, | ||
| context: object = {}, | ||
| timestampMs: number = this.timeProvider.now() | ||
| ): Promise<void> => { | ||
| const mappedEvent = this.resourceEventMapper.applyEventMapper({ | ||
| key, | ||
| statusCode: 0, | ||
| kind: 'xhr', | ||
| size: -1, | ||
| context, | ||
| timestampMs, | ||
| resourceContext: { responseURL: url } as XMLHttpRequest | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #1327
What does this PR do?
This PR applies
resourceEventMapperto auto-tracked XHR resources before they are startednatively, so applications can sanitize resource URLs before they are reported to Datadog.
For example, a resource URL containing PII can be rewritten before reporting:
to:
With this change, the mapper can:
null,resourceContext.responseURL,DdRum.registerResourceEventMapperorDdRum.unregisterResourceEventMapperis called after SDK initialization.The main implementation changes are:
DdSdkReactNative.initializepasses the configuredresourceEventMappertoDdRumResourceTracking.startTracking.DdRum.startResourceapplies the mapper before calling nativestartResource; if themapper returns
null, no native resource is started.DdRum.stopResourceno longer sends the_dd.resource.drop_resourcemarker when themapper returns
null, because dropped resources are now skipped before native start.DdRum.registerResourceEventMapperandDdRum.unregisterResourceEventMapperupdate theactive XHR tracking proxy.
DdRumResourceTracking.updateResourceEventMapperforwards mapper changes to the requestproxy when resource tracking is active.
XHRProxy.onTrackingUpdatesupports partial updates, so tracing sample rate and mapperupdates can be applied independently.
ResourceReporterconverts Datadog's internalRUMResourceinto amapper-compatible resource event before running mappers.
ResourceEventis exported and the native resource mapper type now includesresourceContext.Motivation
We need to sanitize resource URLs reported to Datadog to prevent leaking PII through query
parameters or other URL components.
The current automatic XHR resource tracking flow does not give
resourceEventMapperenoughcontrol at the point where the native resource is started.
Filtering decisions that depend on the request URL can happen too late: the native resource
has already been started, and dropping the event requires sending a native-side drop marker
later during
stopResource.This makes it difficult to use
resourceEventMapperas the single JavaScript-level place forresource filtering and sanitization. It also creates a gap between frontend and mobile
instrumentation: our frontend can already sanitize resource URLs before reporting them, and we
want mobile apps to support the same privacy guarantees and implementation model.
The main use cases are:
Additional Notes
The important behavior change is that the keep/drop decision now happens before native
startResource.DdRum.startResourcebuilds a mapper-compatible resource event with the original resource key,default XHR metadata, context, timestamp, and the request URL exposed as
resourceContext.responseURL:For kept resources, native tracking starts with the original resource key for lifecycle
correlation and the mapped URL/context for reporting:
If the mapper returns
nulllater duringstopResource, JavaScript returns without callingnative code. A resource dropped at start time has no native resource entry to stop:
For auto-tracked XHR resources,
ResourceReporterconverts the internalRUMResourceinto theshape expected by
ResourceEventMapperbefore running mappers:Mappers run sequentially against the latest mapped resource. Returning
nulldrops theresource; returning an object merges the mapper output back into the resource:
If the mapper sanitizes
resourceContext.responseURL, that sanitized URL is used whenreporting the resource:
Runtime mapper registration is forwarded to active resource tracking:
The chages were tested with appropriate patch being applied to production app:
@datadog+mobile-react-native+3.5.1.patch
Everything works as expected.
Review checklist (to be filled by reviewers)