Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Breaking Changes

- Throw IllegalArgumentException when calling Sentry.init on Android ([#3596](https://github.com/getsentry/sentry-java/pull/3596))
- Change OkHttp sub-spans to span attributes ([#3556](https://github.com/getsentry/sentry-java/pull/3556))
- This will reduce the number of spans created by the SDK

Expand Down
7 changes: 7 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,13 @@ public static void init(final @NotNull SentryOptions options) {
@SuppressWarnings("deprecation")
private static synchronized void init(
final @NotNull SentryOptions options, final boolean globalHubMode) {

if (!options.getClass().getName().equals("io.sentry.android.core.SentryAndroidOptions")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: Hmm not sure if this is the best way. If someone has it's own class MySentryOptions extends SentryAndroidOptions {} this will break.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the class is final, so I think we're good here. Or did you mean extends SentryOptions rather?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed the final, all good then! 👍

&& Platform.isAndroid()) {
throw new IllegalArgumentException(
"You are running Android. Please, use SentryAndroid.init. "
+ options.getClass().getName());
}
if (isEnabled()) {
options
.getLogger()
Expand Down
58 changes: 57 additions & 1 deletion sentry/src/test/java/io/sentry/SentryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import io.sentry.protocol.SentryId
import io.sentry.protocol.SentryThread
import io.sentry.test.ImmediateExecutorService
import io.sentry.test.createSentryClientMock
import io.sentry.test.injectForField
import io.sentry.util.PlatformTestManipulator
import io.sentry.util.thread.IMainThreadChecker
import io.sentry.util.thread.MainThreadChecker
Expand All @@ -42,6 +43,7 @@ import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFails
import kotlin.test.assertFalse
import kotlin.test.assertIs
import kotlin.test.assertNotEquals
Expand Down Expand Up @@ -957,12 +959,16 @@ class SentryTest {

@Test
fun `getSpan calls returns root span if globalHubMode is enabled on Android`() {
var sentryOptions: CustomAndroidOptions? = null
PlatformTestManipulator.pretendIsAndroid(true)
Sentry.init({
Sentry.init(OptionsContainer.create(CustomAndroidOptions::class.java), {
it.dsn = dsn
it.enableTracing = true
it.sampleRate = 1.0
it.mockName()
sentryOptions = it
}, true)
sentryOptions?.resetName()

val transaction = Sentry.startTransaction("name", "op-root", TransactionOptions().also { it.isBindToScope = true })
transaction.startChild("op-child")
Expand Down Expand Up @@ -1174,6 +1180,36 @@ class SentryTest {
assertTrue(appStartOption.isProfilingEnabled)
}

@Test
fun `init on Android throws when not using SentryAndroidOptions`() {
PlatformTestManipulator.pretendIsAndroid(true)
assertFails("You are running Android. Please, use SentryAndroid.init.") {
Sentry.init {
it.dsn = dsn
}
}
PlatformTestManipulator.pretendIsAndroid(false)
}

@Test
fun `init on Android works when using SentryAndroidOptions`() {
PlatformTestManipulator.pretendIsAndroid(true)
val options = CustomAndroidOptions().also {
it.dsn = dsn
it.mockName()
}
Sentry.init(options)
options.resetName()
PlatformTestManipulator.pretendIsAndroid(false)
}

@Test
fun `init on Java works when not using SentryAndroidOptions`() {
Sentry.init {
it.dsn = dsn
}
}

@Test
fun `metrics calls scopes getMetrics`() {
val scopes = mock<IScopes>()
Expand Down Expand Up @@ -1260,4 +1296,24 @@ class SentryTest {
assertFalse(tempFile.exists())
return tempFile.absolutePath
}

/**
* Custom SentryOptions for Android.
* It needs to call [mockName] to change its name in io.sentry.android.core.SentryAndroidOptions.
* The name cannot be changed right away, because Sentry.init instantiates the options through reflection.
* So the name should be changed in option configuration.
* After the test, it needs to call [resetName] to reset the name back to io.sentry.SentryTest$CustomAndroidOptions,
* since it's cached internally and would break subsequent tests otherwise.
*/
private class CustomAndroidOptions : SentryOptions() {
init {
resetName()
}
fun mockName() {
javaClass.injectForField("name", "io.sentry.android.core.SentryAndroidOptions")
}
fun resetName() {
javaClass.injectForField("name", "io.sentry.SentryTest\$CustomAndroidOptions")
}
}
}