throw when calling Sentry.init on Android#3596
Conversation
…ndroidOptions when using Android
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 504d056 | 417.69 ms | 492.86 ms | 75.16 ms |
| a59fca2 | 560.08 ms | 658.80 ms | 98.71 ms |
| ca82680 | 490.88 ms | 631.67 ms | 140.80 ms |
| 41e496a | 408.90 ms | 491.32 ms | 82.42 ms |
| a0f7731 | 357.02 ms | 433.40 ms | 76.38 ms |
| 2f49b9c | 416.54 ms | 479.14 ms | 62.60 ms |
| 57732e8 | 435.66 ms | 510.50 ms | 74.84 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 504d056 | 1.70 MiB | 2.29 MiB | 599.31 KiB |
| a59fca2 | 1.70 MiB | 2.29 MiB | 599.77 KiB |
| ca82680 | 1.70 MiB | 2.29 MiB | 599.31 KiB |
| 41e496a | 1.70 MiB | 2.29 MiB | 599.77 KiB |
| a0f7731 | 1.70 MiB | 2.29 MiB | 599.77 KiB |
| 2f49b9c | 1.70 MiB | 2.29 MiB | 599.31 KiB |
| 57732e8 | 1.70 MiB | 2.29 MiB | 599.77 KiB |
markushi
left a comment
There was a problem hiding this comment.
Looking good overall, left a comment which I want to discuss before merging - hence requesting changes until resolved 😅
| private static synchronized void init( | ||
| final @NotNull SentryOptions options, final boolean globalHubMode) { | ||
|
|
||
| if (!options.getClass().getName().equals("io.sentry.android.core.SentryAndroidOptions") |
There was a problem hiding this comment.
m: Hmm not sure if this is the best way. If someone has it's own class MySentryOptions extends SentryAndroidOptions {} this will break.
There was a problem hiding this comment.
the class is final, so I think we're good here. Or did you mean extends SentryOptions rather?
There was a problem hiding this comment.
I already considered it, and as roman said, SentryAndroidOptions is final.
The only other way to consistently add this check is to add another internal API Sentry.init(options, globalHubMode, isInitFromAndroid) or similar
There was a problem hiding this comment.
Oh I missed the final, all good then! 👍
Co-authored-by: Markus Hintersteiner <markus.hintersteiner@sentry.io>
📜 Description
added a check in Sentry.init to check the options against the SentryAndroidOptions when using Android
💡 Motivation and Context
We want to avoid people on Android calling Sentry.init directly, instead of SentryAndroid.init.
Closes #3440
💚 How did you test it?
Unit tests
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps