Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide automatic breadcrumbs and transactions for click/scroll events for Compose #2390

Merged
merged 31 commits into from Dec 14, 2022

Conversation

markushi
Copy link
Member

@markushi markushi commented Nov 25, 2022

📜 Description

Provide automatic breadcrumbs and transactions for click/scroll events on Composable UI widgets.

💚 How did you test it?

📝 Checklist

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2022

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 47bc669

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 330.75 ms 379.18 ms 48.43 ms
Size 1.73 MiB 2.33 MiB 614.63 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ecf9680 303.12 ms 346.35 ms 43.23 ms
ecf9680 321.55 ms 385.52 ms 63.97 ms
b85d8aa 289.35 ms 335.92 ms 46.56 ms
81a1a6c 294.04 ms 341.19 ms 47.15 ms
81a1a6c 328.73 ms 421.28 ms 92.55 ms
90e9745 314.68 ms 357.28 ms 42.60 ms
4a9c176 336.33 ms 384.73 ms 48.41 ms
38e4f11 358.20 ms 433.73 ms 75.53 ms
f809aac 301.51 ms 346.60 ms 45.09 ms
3695453 299.25 ms 360.04 ms 60.79 ms

App size

Revision Plain With Sentry Diff
ecf9680 1.73 MiB 2.32 MiB 612.39 KiB
ecf9680 1.73 MiB 2.32 MiB 612.39 KiB
b85d8aa 1.73 MiB 2.32 MiB 611.62 KiB
81a1a6c 1.73 MiB 2.32 MiB 612.47 KiB
81a1a6c 1.73 MiB 2.32 MiB 612.47 KiB
90e9745 1.73 MiB 2.32 MiB 608.63 KiB
4a9c176 1.73 MiB 2.33 MiB 612.69 KiB
38e4f11 1.73 MiB 2.32 MiB 609.82 KiB
f809aac 1.73 MiB 2.32 MiB 608.63 KiB
3695453 1.73 MiB 2.32 MiB 611.62 KiB

Previous results on branch: feat/compose-ui-transactions

Startup times

Revision Plain With Sentry Diff
f86ae53 262.90 ms 328.56 ms 65.66 ms
a236d78 346.83 ms 411.12 ms 64.30 ms
7c0ffde 324.06 ms 417.49 ms 93.43 ms
cbc0267 357.77 ms 402.40 ms 44.63 ms
8f44f14 352.34 ms 409.55 ms 57.21 ms
003b449 358.02 ms 385.31 ms 27.29 ms
2198116 353.12 ms 382.02 ms 28.90 ms

App size

Revision Plain With Sentry Diff
f86ae53 1.73 MiB 2.33 MiB 613.64 KiB
a236d78 1.73 MiB 2.32 MiB 611.71 KiB
7c0ffde 1.73 MiB 2.33 MiB 613.08 KiB
cbc0267 1.73 MiB 2.33 MiB 613.63 KiB
8f44f14 1.73 MiB 2.33 MiB 613.64 KiB
003b449 1.73 MiB 2.33 MiB 613.11 KiB
2198116 1.73 MiB 2.33 MiB 613.64 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.

as discussed internally let's try to use our SentryGestureListener for that so we still add value even for those who don't use SAGP for auto-instrumentation

@markushi markushi changed the title Add integration for Compose clickables Provide automatic breadcrumbs and transactions for click/scroll events for Compose Nov 30, 2022
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
@markushi markushi marked this pull request as ready for review December 7, 2022 16:07
@markushi
Copy link
Member Author

markushi commented Dec 7, 2022

@romtsn looking good now from my perspective. A few points I'm not happy about right now:

  • As the GestureTargetLocator needs to live within sentry to make is accessible to both sentry-compose and the android module it doesn't know anything about Android or Compose, thus the .locate() method takes the "root view" of type Object as an argument
  • The tests are very basic right now and only check the breadcrumbs, but I really tried to keeps things simple. Also it's using "Espresso" for testing the interactions, as the built-in Compose testing lib is not simulating real touch events

@markushi
Copy link
Member Author

markushi commented Dec 9, 2022

As agreed let's get rid of the reflection parts and use java instead. This requires us to use shadow due to https://youtrack.jetbrains.com/issue/KT-30878.

@romtsn
Copy link
Member

romtsn commented Dec 13, 2022

great stuff 🚀 I think now it looks more fail-safe and more performant than it was with reflection, etc. Thanks for bearing with back and forths :)

@romtsn
Copy link
Member

romtsn commented Dec 13, 2022

ah, yeah, one more thing I forgot - could you please add a README.md to the sentry-compose-helper module explaining why do we need it in the first place and why it's not published as a separate module? 🙏 thanks

@codecov-commenter
Copy link

Codecov Report

Base: 80.21% // Head: 80.00% // Decreases project coverage by -0.20% ⚠️

Coverage data is based on head (f846c6c) compared to base (d00c464).
Patch coverage: 7.31% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2390      +/-   ##
============================================
- Coverage     80.21%   80.00%   -0.21%     
  Complexity     3772     3772              
============================================
  Files           302      303       +1     
  Lines         14222    14263      +41     
  Branches       1885     1890       +5     
============================================
+ Hits          11408    11411       +3     
- Misses         2073     2111      +38     
  Partials        741      741              
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/Breadcrumb.java 86.04% <0.00%> (-1.53%) ⬇️
...in/java/io/sentry/internal/gestures/UiElement.java 0.00% <0.00%> (ø)
sentry/src/main/java/io/sentry/util/Objects.java 50.00% <0.00%> (-50.00%) ⬇️
sentry/src/main/java/io/sentry/SentryOptions.java 79.62% <23.07%> (-1.78%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@markushi markushi merged commit 87598a5 into main Dec 14, 2022
@markushi markushi deleted the feat/compose-ui-transactions branch December 14, 2022 11:21
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.

None yet

4 participants