Skip to content

Conversation

@43jay
Copy link
Collaborator

@43jay 43jay commented Nov 12, 2025

📜 Description

Continuation of MOBILE-935: [Mobile Replay] Capture Network request/response bodies, for android SDK.

💡 Motivation and Context

Related to #4796, which was the implementation.

https://docs.sentry.io/platforms/javascript/session-replay/configuration/#network-details
This PR adds the ability to enable network details extraction via SentryAndroidOptions API, either via AndroidManifest <metadata/> or via SentryAndroid#init

💚 How did you test it?

Unit Tests

ManifestMetadataReader

> ./gradlew :sentry-android-core:testDebugUnitTest --tests="*ManifestMetadataReaderTest*" --continue
BUILD SUCCESSFUL in 24s
147 actionable tasks: 15 executed, 1 from cache, 131 up-to-date

SentryReplayOptions

> ./gradlew :sentry:test --tests="*SentryReplayOptionsTest*Network*"
BUILD SUCCESSFUL in 6s
22 actionable tasks: 1 executed, 21 up-to-date

RRWebOptionsEvent

> ./gradlew :sentry:test --tests="*RRWebOptionsEventSerializationTest*when*"
BUILD SUCCESSFUL in 7s
22 actionable tasks: 1 executed, 21 up-to-date

sentry-samples test app

  • Respects configuration via AndroidManifest.xml
  • Respects configuration via SentryAndroid#init

📝 Checklist

📝
  • I added tests to verify the changes.

  • No breaking change or entry added to the changelog.

  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

  • I added GH Issue ID & Linear ID

  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
    By default network detail extraction is off - user must opt-in by adding a regex to networkDetailAllowUrls. Therefore sendDefaultPII does not apply. @markushi confirms this matches the JS behaviour.

  • When a url is on deny list the UI shows "no mobile sdk support"

Tracked in MOBILE-1131: [sentry-android][network-details] Sentry dashboard incorrectly shows "no mobile sdk support"

image
  • When networkCaptureBodies is false the UI shows "no mobile sdk support"

Tracked in MOBILE-1131: [sentry-android][network-details] Sentry dashboard incorrectly shows "no mobile sdk support"

session replay
image
image

🔮 Next steps

  • changelog changelog will be published alongside next PR for SentryOkHttpInterceptor support #skip-changelog
  • docs docs will be updated when changelog goes out
  • wizard wizard updated when changelog goes out

@linear
Copy link

linear bot commented Nov 12, 2025

@43jay 43jay marked this pull request as draft November 12, 2025 19:43
@github-actions
Copy link
Contributor

github-actions bot commented Nov 12, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 283.56 ms 353.17 ms 69.61 ms
Size 1.58 MiB 2.13 MiB 557.31 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ee747ae 357.79 ms 421.84 ms 64.05 ms
ae7fed0 293.84 ms 380.22 ms 86.38 ms
9fbb112 404.51 ms 475.65 ms 71.14 ms
27d7cf8 397.90 ms 498.65 ms 100.75 ms
a416a65 333.78 ms 410.37 ms 76.59 ms
d217708 411.22 ms 430.86 ms 19.63 ms
539ca63 313.51 ms 355.43 ms 41.92 ms
23d6b12 354.10 ms 408.38 ms 54.28 ms
14ff5ee 419.75 ms 495.73 ms 75.98 ms
ce0a49e 532.00 ms 609.96 ms 77.96 ms

App size

Revision Plain With Sentry Diff
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ae7fed0 1.58 MiB 2.12 MiB 551.77 KiB
9fbb112 1.58 MiB 2.11 MiB 539.18 KiB
27d7cf8 1.58 MiB 2.12 MiB 549.42 KiB
a416a65 1.58 MiB 2.12 MiB 555.26 KiB
d217708 1.58 MiB 2.10 MiB 532.97 KiB
539ca63 1.58 MiB 2.12 MiB 551.41 KiB
23d6b12 1.58 MiB 2.10 MiB 532.31 KiB
14ff5ee 1.58 MiB 2.10 MiB 535.08 KiB
ce0a49e 1.58 MiB 2.10 MiB 532.94 KiB

Previous results on branch: 43jay/MOBILE-935

Startup times

Revision Plain With Sentry Diff
1c4ba0c 302.22 ms 367.21 ms 64.98 ms
da92356 319.12 ms 388.73 ms 69.61 ms
6687052 321.98 ms 383.35 ms 61.38 ms
1a07f53 341.15 ms 378.86 ms 37.71 ms
790a163 339.64 ms 436.57 ms 96.94 ms

App size

Revision Plain With Sentry Diff
1c4ba0c 1.58 MiB 2.13 MiB 556.34 KiB
da92356 1.58 MiB 2.13 MiB 556.24 KiB
6687052 1.58 MiB 2.13 MiB 556.25 KiB
1a07f53 1.58 MiB 2.12 MiB 553.02 KiB
790a163 1.58 MiB 2.12 MiB 553.01 KiB

Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

LGTM, nice work!

Determine whether the networkDetail options should be added as "tags"

Yes, I think we should add them to options here.

At some point we'd need to add the changelog entry -- maybe now is a good time? Or maybe after we added this to SentryOkHttpEventListener

*/
@ApiStatus.Internal
public static @NotNull String[] getNetworkDetailsDefaultHeaders() {
return DEFAULT_HEADERS.clone();
Copy link
Member

Choose a reason for hiding this comment

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

do we have to clone it actually? I see we convert it using Arrays.asList down there anyway, so not sure if it's necessary?

Copy link
Collaborator Author

@43jay 43jay Nov 17, 2025

Choose a reason for hiding this comment

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

Good call-out.

The clone was for a defensive copy to ensure the DEFAULT_HEADERS elements couldn't be mutated by clients.

But it would be simpler as a Collections.unmodifiableList

=> fixed

@43jay 43jay marked this pull request as ready for review November 17, 2025 21:18
43jay and others added 10 commits November 17, 2025 17:19
enables Network Detail extraction via SDK and removes FAKE_OPTIONS placeholder
Seems more efficient - getNetworkRequestHeaders/getNetworkResponseHeaders is invoked on every http request but setNetwork... is only invoked on start-up
getNetworkRequestHeaders / getNetworkResponseHeaders are called on every http request => move the memory operation to the setNetworkRequest|ResponseHeaders path which is called 1x on start-up
@43jay 43jay enabled auto-merge (squash) November 19, 2025 17:19
@43jay 43jay merged commit b03edbb into main Nov 19, 2025
61 of 63 checks passed
@43jay 43jay deleted the 43jay/MOBILE-935 branch November 19, 2025 17:38
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.

4 participants