search

getsentry / sentry-dart

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

Add breadcrumb for `GestureDetector` onTap #2067

Closed denrase closed 3 weeks ago

denrase commented 1 month ago

:scroll: Description

:bulb: Motivation and Context

Relates to #2012

:green_heart: How did you test it?

:pencil: Checklist

:crystal_ball: Next steps

github-actions[bot] commented 1 month ago
Fails
:no_entry_sign: Please consider adding a changelog entry for the next release.
Messages
:book: Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Add breadcrumb for `GestureDetector` onTap ([#2067](https://github.com/getsentry/sentry-dart/pull/2067))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by :no_entry_sign: dangerJS against 1dceadc46cac4316abfb8d7168c4c19d06ef5db7

github-actions[bot] commented 1 month ago

iOS Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1230.90 ms 1246.37 ms 15.47 ms
Size 8.32 MiB 9.52 MiB 1.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
50bdfad10c3833c388a7a32c883117f3e9b2cbce 1253.14 ms 1274.54 ms 21.40 ms
11319141655ff681e0df68bce3cf03ffe9a95f30 1277.20 ms 1300.20 ms 23.00 ms
8a10ab719072f65d029985c211471d779ab2a3cb 1226.49 ms 1250.52 ms 24.03 ms
afa6e2a6ded2f7dbb4530fe3974fdbb30db1f914 1251.04 ms 1266.65 ms 15.61 ms
1c6eb5b33483c440c18525f0e946a9031d09863c 1277.85 ms 1285.71 ms 7.86 ms
8932ece117c0cfe9eb98048e40c31da8507bd8a2 1234.31 ms 1238.90 ms 4.59 ms
8ced2dc3a0ab7c8f70f4a0f0449ce528b924b96f 1258.35 ms 1272.98 ms 14.62 ms
e5b744f308ef88a70f679a622a88ade69705613c 1250.82 ms 1284.46 ms 33.64 ms
42f6e7e6bf2882e21c1f664a1a6c8277ab07ad3b 1232.52 ms 1247.86 ms 15.34 ms
f922f8fd57e0574f28866f4b860f5d674bdb15e2 1249.53 ms 1266.51 ms 16.98 ms

App size

Revision Plain With Sentry Diff
50bdfad10c3833c388a7a32c883117f3e9b2cbce 8.32 MiB 9.43 MiB 1.10 MiB
11319141655ff681e0df68bce3cf03ffe9a95f30 8.16 MiB 9.17 MiB 1.01 MiB
8a10ab719072f65d029985c211471d779ab2a3cb 8.28 MiB 9.34 MiB 1.06 MiB
afa6e2a6ded2f7dbb4530fe3974fdbb30db1f914 8.28 MiB 9.33 MiB 1.05 MiB
1c6eb5b33483c440c18525f0e946a9031d09863c 8.15 MiB 9.12 MiB 986.27 KiB
8932ece117c0cfe9eb98048e40c31da8507bd8a2 8.29 MiB 9.36 MiB 1.07 MiB
8ced2dc3a0ab7c8f70f4a0f0449ce528b924b96f 8.10 MiB 9.16 MiB 1.07 MiB
e5b744f308ef88a70f679a622a88ade69705613c 8.09 MiB 9.07 MiB 1001.19 KiB
42f6e7e6bf2882e21c1f664a1a6c8277ab07ad3b 8.28 MiB 9.33 MiB 1.05 MiB
f922f8fd57e0574f28866f4b860f5d674bdb15e2 8.15 MiB 9.13 MiB 1003.20 KiB
denrase commented 1 month ago

@marandaneto Hello there! :) Do you know why we specifically tested that GestureRecognizer does not trigger a crumb when containing a button?

https://github.com/getsentry/sentry-dart/blob/ee0ca56b51107c6e5048fd280474f2c0a1ea5104/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart#L178

github-actions[bot] commented 1 month ago

Android Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 348.21 ms 412.83 ms 64.62 ms
Size 6.33 MiB 7.30 MiB 992.52 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
636cb61552ee37d8206f06e518009b036a877f24 366.59 ms 448.14 ms 81.55 ms
21845e2c4fd3604443616251b5d44240ea390663 345.08 ms 382.82 ms 37.74 ms
2331d89055c421463574e68889d9ee24e7c15e32 352.45 ms 417.34 ms 64.89 ms
f4cc7440a38c30b05db7edb9baef40bda0cf0fa4 349.53 ms 394.68 ms 45.15 ms
1596141c018a1029c280be2e0a0b2cd4849239ec 300.92 ms 368.94 ms 68.02 ms
35005743b3b663c45136b546c2fd11ab2e36fd39 288.69 ms 358.34 ms 69.65 ms
04db237c887e97fd4e5843a50e97135e2e03ce53 330.16 ms 428.38 ms 98.22 ms
deaeece426a8b33728624ff88b57690a5314a1c3 347.42 ms 381.10 ms 33.68 ms
61e71ec84d02fc022b00615b4d0bae341a641502 343.94 ms 410.59 ms 66.66 ms
df16b96fceb6ce836e4d52d33120d7b1d922d5a7 326.08 ms 391.82 ms 65.74 ms

App size

Revision Plain With Sentry Diff
636cb61552ee37d8206f06e518009b036a877f24 6.27 MiB 7.20 MiB 959.08 KiB
21845e2c4fd3604443616251b5d44240ea390663 5.94 MiB 6.92 MiB 1003.77 KiB
2331d89055c421463574e68889d9ee24e7c15e32 5.94 MiB 6.96 MiB 1.02 MiB
f4cc7440a38c30b05db7edb9baef40bda0cf0fa4 5.94 MiB 6.95 MiB 1.01 MiB
1596141c018a1029c280be2e0a0b2cd4849239ec 6.16 MiB 7.14 MiB 1003.98 KiB
35005743b3b663c45136b546c2fd11ab2e36fd39 6.16 MiB 7.14 MiB 1009.90 KiB
04db237c887e97fd4e5843a50e97135e2e03ce53 5.94 MiB 6.95 MiB 1.01 MiB
deaeece426a8b33728624ff88b57690a5314a1c3 5.94 MiB 6.96 MiB 1.02 MiB
61e71ec84d02fc022b00615b4d0bae341a641502 6.34 MiB 7.28 MiB 966.26 KiB
df16b96fceb6ce836e4d52d33120d7b1d922d5a7 6.06 MiB 7.03 MiB 988.94 KiB
marandaneto commented 1 month ago

@marandaneto Hello there! :) Do you know why we specifically tested that GestureRecognizer does not trigger a crumb when containing a button?

https://github.com/getsentry/sentry-dart/blob/ee0ca56b51107c6e5048fd280474f2c0a1ea5104/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart#L178

hello hello, mmm its been a while but IIRC if also adding GestureDetector support and then finding the correct widget wasn't working well because the events will bubble up, for example, finding btn4 instead of btn5 in that case, something like that, could have been an oversight as well, maybe check the PRs and comments/reviews.

denrase commented 3 weeks ago

@buenaflor Ok, in that case I think it's better if users add breadcrumb tracking for those widgets manually, as I don't think we should change/break the current behaviour. WDYT?