getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
745 stars 231 forks source link

refactor: move all native calls to SentryNativeBinding #2097

Closed vaind closed 3 months ago

vaind commented 3 months ago

This finishes the transition to SentryNativeBinding. Previously, there was still some code directly using MethodChannels. Merging this PR will open way to integrating sentry-native via FFI (e.g. for desktop platforms), which is what I originally set out to do before finding out these blockers.

Basically, this PR:

I know this PR looks pretty large but although it doesn't seem like it, I did try to keep it to the minimum necessary. Hiding whitespace changes helps a little (especially in tests where a foreach loop may have been added/removed):

image

skip-changelog because these are all internal changes

github-actions[bot] commented 3 months ago

Android Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 393.11 ms 482.51 ms 89.40 ms
Size 6.35 MiB 7.34 MiB 1008.32 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8f88a49d613434f898cd280880fbd122370dfffa 364.98 ms 413.78 ms 48.80 ms
daa1b330099908feace526008680693a69587afc 366.98 ms 451.59 ms 84.61 ms
04db237c887e97fd4e5843a50e97135e2e03ce53 330.16 ms 428.38 ms 98.22 ms
afa6e2a6ded2f7dbb4530fe3974fdbb30db1f914 349.73 ms 428.48 ms 78.75 ms
e2d89fc73b84c81801b638f4be7c488d53b7960d 323.84 ms 376.23 ms 52.39 ms
86d48419a55b5e88a17d31b0574e7e2974d4c18b 286.35 ms 372.43 ms 86.08 ms
8609bd8dd7ea572e5d241a59643c7570e5621bda 359.76 ms 437.40 ms 77.64 ms
72dfc836ec6ac6176efb374c4c1c3e3871f4117e 298.62 ms 340.14 ms 41.52 ms
870f5ebb0389ee8870a702da61188f0c7826b7e1 329.45 ms 369.29 ms 39.84 ms
f79eecfc96c81e45b312c72aea32e4943329666b 341.35 ms 388.98 ms 47.63 ms

App size

Revision Plain With Sentry Diff
8f88a49d613434f898cd280880fbd122370dfffa 6.34 MiB 7.30 MiB 979.60 KiB
daa1b330099908feace526008680693a69587afc 6.27 MiB 7.20 MiB 956.36 KiB
04db237c887e97fd4e5843a50e97135e2e03ce53 5.94 MiB 6.95 MiB 1.01 MiB
afa6e2a6ded2f7dbb4530fe3974fdbb30db1f914 6.27 MiB 7.20 MiB 955.69 KiB
e2d89fc73b84c81801b638f4be7c488d53b7960d 6.06 MiB 7.03 MiB 989.37 KiB
86d48419a55b5e88a17d31b0574e7e2974d4c18b 6.15 MiB 7.13 MiB 1000.49 KiB
8609bd8dd7ea572e5d241a59643c7570e5621bda 6.27 MiB 7.20 MiB 959.07 KiB
72dfc836ec6ac6176efb374c4c1c3e3871f4117e 5.94 MiB 6.92 MiB 1001.71 KiB
870f5ebb0389ee8870a702da61188f0c7826b7e1 5.94 MiB 6.92 MiB 1005.77 KiB
f79eecfc96c81e45b312c72aea32e4943329666b 6.15 MiB 7.13 MiB 1000.07 KiB

Previous results on branch: refactor/native-bindings

Startup times

Revision Plain With Sentry Diff
d780005b632863cb6cf959e63b7b71c786b56dc9 361.80 ms 439.98 ms 78.18 ms
69d7517972e78d98fd61a6a407b32b1c78aeb004 449.02 ms 521.77 ms 72.75 ms
298d05e7207933916e1b23b535f4e3134b403a88 424.94 ms 470.20 ms 45.26 ms
7259c8207a752f9d02de5c2eb8da1f244c1459b6 412.69 ms 479.35 ms 66.67 ms
829ca2dc107358e3c22d4ff12ab7478dd799fa91 391.83 ms 468.26 ms 76.43 ms

App size

Revision Plain With Sentry Diff
d780005b632863cb6cf959e63b7b71c786b56dc9 6.35 MiB 7.34 MiB 1007.94 KiB
69d7517972e78d98fd61a6a407b32b1c78aeb004 6.35 MiB 7.34 MiB 1007.93 KiB
298d05e7207933916e1b23b535f4e3134b403a88 6.35 MiB 7.34 MiB 1007.73 KiB
7259c8207a752f9d02de5c2eb8da1f244c1459b6 6.35 MiB 7.34 MiB 1007.95 KiB
829ca2dc107358e3c22d4ff12ab7478dd799fa91 6.35 MiB 7.34 MiB 1007.95 KiB
github-actions[bot] commented 3 months ago

iOS Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1232.24 ms 1255.52 ms 23.28 ms
Size 8.33 MiB 9.58 MiB 1.25 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
3f23617e3346caa128c5f27b9c935de4974c3067 1261.93 ms 1286.10 ms 24.17 ms
ca7f531d8ef46d53d71b084884c27cae425de945 1242.51 ms 1282.61 ms 40.10 ms
d78342459141e1798a213da74ad13e89235cbe31 1265.71 ms 1277.00 ms 11.29 ms
754cdbe8e837db0c3e8d6cebf13feee66f651444 1261.67 ms 1280.88 ms 19.20 ms
a7acb24b1deda7eb424909eeb9e0f6897cba2cf0 1296.71 ms 1317.69 ms 20.98 ms
f2db4ece59961760d0017bb7b0592fc561ab79f0 1244.14 ms 1259.79 ms 15.65 ms
25e9b5900ecb2ddc682fe68375964943fe087595 1289.76 ms 1295.27 ms 5.51 ms
683fd34f35c964a36534f6df6b3d4cb325f5069c 1239.83 ms 1259.08 ms 19.25 ms
b8562d01376a7efe9af802ab0abd9d8226eb4bd7 1249.92 ms 1267.56 ms 17.64 ms
3e9fb0ef61113312be4c21d9168d4043eca6c921 1262.49 ms 1280.65 ms 18.16 ms

App size

Revision Plain With Sentry Diff
3f23617e3346caa128c5f27b9c935de4974c3067 8.16 MiB 9.17 MiB 1.01 MiB
ca7f531d8ef46d53d71b084884c27cae425de945 8.32 MiB 9.38 MiB 1.06 MiB
d78342459141e1798a213da74ad13e89235cbe31 8.29 MiB 9.38 MiB 1.09 MiB
754cdbe8e837db0c3e8d6cebf13feee66f651444 8.29 MiB 9.37 MiB 1.08 MiB
a7acb24b1deda7eb424909eeb9e0f6897cba2cf0 8.16 MiB 9.17 MiB 1.01 MiB
f2db4ece59961760d0017bb7b0592fc561ab79f0 8.10 MiB 9.16 MiB 1.07 MiB
25e9b5900ecb2ddc682fe68375964943fe087595 8.16 MiB 9.15 MiB 1021.15 KiB
683fd34f35c964a36534f6df6b3d4cb325f5069c 8.28 MiB 9.34 MiB 1.06 MiB
b8562d01376a7efe9af802ab0abd9d8226eb4bd7 8.33 MiB 9.54 MiB 1.22 MiB
3e9fb0ef61113312be4c21d9168d4043eca6c921 8.15 MiB 9.12 MiB 989.77 KiB

Previous results on branch: refactor/native-bindings

Startup times

Revision Plain With Sentry Diff
7259c8207a752f9d02de5c2eb8da1f244c1459b6 1228.08 ms 1254.48 ms 26.40 ms
829ca2dc107358e3c22d4ff12ab7478dd799fa91 1217.73 ms 1229.41 ms 11.67 ms
d780005b632863cb6cf959e63b7b71c786b56dc9 1225.86 ms 1248.30 ms 22.44 ms
69d7517972e78d98fd61a6a407b32b1c78aeb004 1225.92 ms 1246.33 ms 20.41 ms

App size

Revision Plain With Sentry Diff
7259c8207a752f9d02de5c2eb8da1f244c1459b6 8.33 MiB 9.59 MiB 1.25 MiB
829ca2dc107358e3c22d4ff12ab7478dd799fa91 8.33 MiB 9.59 MiB 1.25 MiB
d780005b632863cb6cf959e63b7b71c786b56dc9 8.33 MiB 9.59 MiB 1.25 MiB
69d7517972e78d98fd61a6a407b32b1c78aeb004 8.33 MiB 9.59 MiB 1.25 MiB
codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 97.03704% with 4 lines in your changes missing coverage. Please review.

Project coverage is 95.43%. Comparing base (c3b8a98) to head (994cdfb).

Files Patch % Lines
...tter/lib/src/native/cocoa/sentry_native_cocoa.dart 60.00% 2 Missing :warning:
...src/integrations/native_app_start_integration.dart 75.00% 1 Missing :warning:
flutter/lib/src/native/factory_real.dart 85.71% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2097 +/- ## ========================================== + Coverage 95.25% 95.43% +0.17% ========================================== Files 54 57 +3 Lines 1791 1751 -40 ========================================== - Hits 1706 1671 -35 + Misses 85 80 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

buenaflor commented 3 months ago

btw sdk metrics is failing currently, the app size needs to be adjusted again, not related to this pr though