-
-
Notifications
You must be signed in to change notification settings - Fork 444
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
Ignore broken regex for tracePropagationTarget #2288
Ignore broken regex for tracePropagationTarget #2288
Conversation
This reverts commit bb1d054.
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
…ipulation (#2282) * fix: frameMetricsAggregator calls may not get executed on main thread * fix: try/catch all FrameMetricsAggregator calls * Add FrameMetricsAggregator fixes to changelog * Add logging and main-thread tests to ActivityFramesTracker * Split up tests for FrameMetricsTracker * Update CHANGELOG.md Co-authored-by: Alexander Dinauer <adinauer@users.noreply.github.com> * Wrap main-thread / exception handling code in runSafelyOnUiThread method Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io> Co-authored-by: Markus Hintersteiner <m.hintersteiner@gmail.com>
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7300956 | 337.57 ms | 384.21 ms | 46.64 ms |
4ca1d7b | 328.46 ms | 368.22 ms | 39.76 ms |
54cebc8 | 300.86 ms | 341.43 ms | 40.57 ms |
c5ccd8a | 329.98 ms | 365.52 ms | 35.54 ms |
1e4690d | 354.69 ms | 387.88 ms | 33.19 ms |
4dd88fe | 306.88 ms | 391.58 ms | 84.70 ms |
4dd88fe | 302.12 ms | 331.17 ms | 29.04 ms |
7300956 | 324.20 ms | 353.79 ms | 29.58 ms |
3d89dea | 345.59 ms | 364.06 ms | 18.47 ms |
54cebc8 | 331.12 ms | 385.14 ms | 54.02 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
7300956 | 1.73 MiB | 2.29 MiB | 578.69 KiB |
4ca1d7b | 1.73 MiB | 2.29 MiB | 579.88 KiB |
54cebc8 | 1.73 MiB | 2.29 MiB | 579.43 KiB |
c5ccd8a | 1.74 MiB | 2.33 MiB | 607.44 KiB |
1e4690d | 1.74 MiB | 2.33 MiB | 604.92 KiB |
4dd88fe | 1.73 MiB | 2.29 MiB | 579.50 KiB |
4dd88fe | 1.73 MiB | 2.29 MiB | 579.50 KiB |
7300956 | 1.73 MiB | 2.29 MiB | 578.69 KiB |
3d89dea | 1.74 MiB | 2.33 MiB | 604.92 KiB |
54cebc8 | 1.73 MiB | 2.29 MiB | 579.43 KiB |
Previous results on branch: fix/ignore-invalid-trace-propagation-target-regex
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
821e81c | 345.06 ms | 364.14 ms | 19.08 ms |
a6cb664 | 288.92 ms | 346.40 ms | 57.48 ms |
f00e43d | 302.61 ms | 333.02 ms | 30.41 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
821e81c | 1.73 MiB | 2.29 MiB | 580.01 KiB |
a6cb664 | 1.73 MiB | 2.29 MiB | 579.88 KiB |
f00e43d | 1.73 MiB | 2.29 MiB | 580.10 KiB |
if (url.matches(origin)) { | ||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (url.matches(origin)) { | |
return true; | |
} | |
return url.matches(origin); |
could be just this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, no we can't because then we'd just return whether the first entry in the list matches but we want to check all of them and only return false
if none of them match.
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Wrap regex URL matching for
tracePropagationTargets
with try / catch and ignore exception.💡 Motivation and Context
Fixes #2286
💚 How did you test it?
Unit Test
📝 Checklist
🔮 Next steps