Skip to content

Conversation

@buenaflor
Copy link
Contributor

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1260.88 ms 1266.24 ms 5.37 ms
Size 5.53 MiB 6.02 MiB 502.02 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
819c1e7 1250.59 ms 1249.08 ms -1.51 ms
765aa8b 1259.09 ms 1269.90 ms 10.82 ms
ad121c0 1275.04 ms 1280.59 ms 5.55 ms
de377fd 1252.28 ms 1254.76 ms 2.48 ms
192b44c 1269.08 ms 1275.52 ms 6.44 ms
e1ab497 1260.92 ms 1260.22 ms -0.69 ms
6ad8fc4 1263.70 ms 1266.06 ms 2.36 ms
5b9a0da 1249.69 ms 1250.71 ms 1.03 ms
c0dde50 1268.90 ms 1275.61 ms 6.71 ms
6bcdc99 1257.16 ms 1264.96 ms 7.79 ms

App size

Revision Plain With Sentry Diff
819c1e7 5.53 MiB 6.00 MiB 479.96 KiB
765aa8b 7.86 MiB 9.44 MiB 1.58 MiB
ad121c0 5.53 MiB 6.01 MiB 488.11 KiB
de377fd 20.71 MiB 22.43 MiB 1.73 MiB
192b44c 5.53 MiB 5.96 MiB 444.33 KiB
e1ab497 5.53 MiB 6.01 MiB 487.96 KiB
6ad8fc4 5.53 MiB 6.01 MiB 487.65 KiB
5b9a0da 5.53 MiB 5.96 MiB 444.32 KiB
c0dde50 5.53 MiB 6.01 MiB 488.14 KiB
6bcdc99 5.53 MiB 6.00 MiB 479.95 KiB

Previous results on branch: enh/ffi-jni-init

Startup times

Revision Plain With Sentry Diff
1f506d7 1255.16 ms 1253.69 ms -1.48 ms
48149f1 1240.53 ms 1252.12 ms 11.59 ms
89fb6c2 1250.77 ms 1255.64 ms 4.87 ms
37f0c51 1250.10 ms 1248.25 ms -1.85 ms
0f928f6 1247.59 ms 1255.63 ms 8.04 ms
21cac57 1259.12 ms 1269.06 ms 9.94 ms
ab4c77e 1267.45 ms 1275.17 ms 7.71 ms

App size

Revision Plain With Sentry Diff
1f506d7 5.53 MiB 6.02 MiB 501.99 KiB
48149f1 5.53 MiB 6.02 MiB 502.03 KiB
89fb6c2 5.53 MiB 6.02 MiB 502.04 KiB
37f0c51 5.53 MiB 6.02 MiB 502.02 KiB
0f928f6 5.53 MiB 6.02 MiB 502.02 KiB
21cac57 5.53 MiB 6.02 MiB 501.98 KiB
ab4c77e 5.53 MiB 6.02 MiB 502.31 KiB

Comment on lines -200 to -203
if (!sentryFlutter.autoPerformanceTracingEnabled) {
return null
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing it here, we have this check in the Dart side

final ObjCObjectBase Function(Object) _defaultObjcConverter = (obj) {
return switch (obj) {
bool b => b ? 1.toNSNumber() : 0.toNSNumber(),
bool b => NSNumberCreation.numberWithBool(b),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

previous convert was wrong, it should've been using NSNumberCreation

@buenaflor buenaflor changed the title (DRAFT) enh: refactor init to use JNI/FFI enh: refactor init to use JNI/FFI Nov 11, 2025
Copy link
Collaborator

@denrase denrase left a comment

Choose a reason for hiding this comment

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

Just the SentryReplayQualit, think we should keep this as it was.

expect(names.contains('ReplayLogIntegration'), isTrue);
} else if (isMacOS) {
expect(names.contains('LoadContextsIntegration'), isTrue);
// Replay not supported on macOS by default
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice catch!

@buenaflor buenaflor marked this pull request as ready for review November 14, 2025 11:57
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Performance Tracking Ignores Disabled State

The autoPerformanceTracingEnabled check was removed from fetchNativeAppStartAsBytes, meaning app start metrics are now returned even when auto performance tracing is disabled. This changes behavior from the previous implementation where the function would return null early when autoPerformanceTracingEnabled was false, potentially causing unwanted performance tracking.

packages/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt#L185-L196

@JvmStatic
fun fetchNativeAppStartAsBytes(): ByteArray? {
val appStartMetrics = AppStartMetrics.getInstance()
if (!appStartMetrics.isAppLaunchedInForeground ||
appStartMetrics.appStartTimeSpan.durationMs > APP_START_MAX_DURATION_MS
) {
Log.w(
"Sentry",
"Invalid app start data: app not launched in foreground or app start took too long (>60s)",
)
return null

Fix in Cursor Fix in Web


} else {
options.setReplayController(null)
}
}
Copy link

Choose a reason for hiding this comment

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

Bug: Initialization Order Causes Silent Replay Failure

The setupReplay method checks if ((replayOptions.isSessionReplayEnabled || replayOptions.isSessionReplayForErrorsEnabled) && replayCallbacks != null) but silently returns without logging when applicationContext is null. This creates a subtle initialization order issue where replay setup silently fails if called before the plugin is attached to the engine, making debugging difficult.

Fix in Cursor Fix in Web

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