Skip to content
Open
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
2 changes: 1 addition & 1 deletion sentry-android-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ dependencies {
testImplementation(projects.sentrySpotlight)
testImplementation(projects.sentryAndroidFragment)
testImplementation(projects.sentryAndroidTimber)
testImplementation(libs.timber)
testImplementation(projects.sentryAndroidReplay)
testImplementation(projects.sentryCompose)
testImplementation(projects.sentryAndroidNdk)
Expand All @@ -132,5 +133,4 @@ dependencies {
testImplementation(libs.androidx.compose.foundation.layout)
testImplementation(libs.androidx.compose.material3)
testRuntimeOnly(libs.androidx.fragment.ktx)
testRuntimeOnly(libs.timber)
}
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ static void installDefaultIntegrations(
}

if (isTimberAvailable) {
options.addIntegration(new SentryTimberIntegration(options.isEnableTimberLogs()));
options.addIntegration(new SentryTimberIntegration(options::isEnableTimberLogs));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: A method reference options::isEnableTimberLogs was introduced, violating a class-level suppression intended to prevent them due to compatibility issues with older Android Gradle Plugin versions.
Severity: MEDIUM

Suggested Fix

To maintain compatibility with older Android Gradle Plugin versions, replace the method reference with an explicit lambda expression. Change new SentryTimberIntegration(options::isEnableTimberLogs) to new SentryTimberIntegration(() -> options.isEnableTimberLogs()), which aligns with the established pattern in the file.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java#L476

Potential issue: The file `AndroidOptionsInitializer.java` contains a class-level
`@SuppressWarnings("Convert2MethodRef")` annotation, explicitly added to prevent the use
of method references due to compatibility issues with older Android Gradle Plugin (AGP)
versions. The pull request introduces a method reference `options::isEnableTimberLogs`
at line 476, which violates this established project-wide convention. This change can
lead to build or runtime failures for developers using older, supported AGP versions,
which the project appears to support based on its conservative compatibility standards.

Did we get this right? 👍 / 👎 to inform future reviews.

}
options.addIntegration(new AppComponentsBreadcrumbsIntegration(context));
options.addIntegration(new SystemEventsBreadcrumbsIntegration(context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import io.sentry.SentryEnvelope
import io.sentry.SentryLevel
import io.sentry.SentryLevel.DEBUG
import io.sentry.SentryLevel.FATAL
import io.sentry.SentryLogEvent
import io.sentry.SentryOptions
import io.sentry.SentryOptions.BeforeSendCallback
import io.sentry.Session
Expand Down Expand Up @@ -84,6 +85,7 @@ import org.robolectric.annotation.Config
import org.robolectric.shadow.api.Shadow
import org.robolectric.shadows.ShadowActivityManager
import org.robolectric.shadows.ShadowActivityManager.ApplicationExitInfoBuilder
import timber.log.Timber

@RunWith(AndroidJUnit4::class)
@Config(sdk = [Build.VERSION_CODES.N], shadows = [SentryShadowProcess::class])
Expand Down Expand Up @@ -238,6 +240,46 @@ class SentryAndroidTest {
assertNotEquals(0, AppStartMetrics.getInstance().appStartTimeSpan.durationMs)
}

@Test
fun `auto-installed Timber integration uses Logs option set in configuration callback`() {
val logs = mutableListOf<SentryLogEvent>()
fixture.initSut { options ->
options.isEnableTimberLogs = true
options.logs.beforeSend =
SentryOptions.Logs.BeforeSendLogCallback { log ->
logs.add(log)
log
}
}

Timber.i("message")

assertEquals(1, logs.size)
}

@Test
fun `auto-installed Timber integration uses configuration callback override of manifest option`() {
val metadata =
Bundle().apply {
putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123")
putBoolean(ManifestMetadataReader.ENABLE_TIMBER_LOGS, true)
}
val mockContext = ContextUtilsTestHelper.mockMetaData(metaData = metadata)
val logs = mutableListOf<SentryLogEvent>()

initForTest(mockContext) { options ->
options.isEnableTimberLogs = false
options.logs.beforeSend =
SentryOptions.Logs.BeforeSendLogCallback { log ->
logs.add(log)
log
}
}
Timber.i("message")

assertTrue(logs.isEmpty())
}

@Test
fun `deduplicates fragment, timber and system events integrations`() {
var refOptions: SentryAndroidOptions? = null
Expand Down
1 change: 1 addition & 0 deletions sentry-android-timber/api/sentry-android-timber.api
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public final class io/sentry/android/timber/SentryTimberIntegration : io/sentry/
public fun <init> (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;)V
public synthetic fun <init> (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lio/sentry/SentryLevel;Lio/sentry/SentryLevel;Lio/sentry/SentryLogLevel;Z)V
public fun <init> (Lio/sentry/util/LazyEvaluator$Evaluator;)V
public fun <init> (Z)V
public fun close ()V
public final fun getEnableLogs ()Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.sentry.SentryLogLevel
import io.sentry.SentryOptions
import io.sentry.android.timber.BuildConfig.VERSION_NAME
import io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion
import io.sentry.util.LazyEvaluator.Evaluator
import java.io.Closeable
import timber.log.Timber

Expand All @@ -18,11 +19,13 @@ public class SentryTimberIntegration(
public val minBreadcrumbLevel: SentryLevel = SentryLevel.INFO,
public val minLogsLevel: SentryLogLevel = SentryLogLevel.INFO,
) : Integration, Closeable {
public var enableLogs: Boolean = false
private set
public val enableLogs: Boolean
get() = enableLogsProvider.evaluate()

private var enableLogsProvider: Evaluator<Boolean> = Evaluator { false }

public constructor(enableLogs: Boolean) : this() {
this.enableLogs = enableLogs
enableLogsProvider = Evaluator { enableLogs }
}

public constructor(
Expand All @@ -31,7 +34,11 @@ public class SentryTimberIntegration(
minLogsLevel: SentryLogLevel,
enableLogs: Boolean,
) : this(minEventLevel, minBreadcrumbLevel, minLogsLevel) {
this.enableLogs = enableLogs
enableLogsProvider = Evaluator { enableLogs }
}

public constructor(enableLogsProvider: Evaluator<Boolean>) : this() {
this.enableLogsProvider = enableLogsProvider

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Internal type in public constructor

Medium Severity

The new public SentryTimberIntegration constructor takes LazyEvaluator.Evaluator, which is @ApiStatus.Internal. That pulls an internal type into the published Timber API surface (also reflected in the .api dump) even though this path is only needed for auto-install. Similar SDK call sites that accept Evaluator live on @ApiStatus.Internal types.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4d120ff. Configure here.

}

private lateinit var tree: SentryTimberTree
Expand All @@ -47,7 +54,14 @@ public class SentryTimberIntegration(
override fun register(scopes: IScopes, options: SentryOptions) {
logger = options.logger

tree = SentryTimberTree(scopes, minEventLevel, minBreadcrumbLevel, minLogsLevel, enableLogs)
tree =
SentryTimberTree(
scopes,
minEventLevel,
minBreadcrumbLevel,
minLogsLevel,
enableLogsProvider.evaluate(),
)
Timber.plant(tree)

logger.log(SentryLevel.DEBUG, "SentryTimberIntegration installed.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.sentry.logger.ILoggerApi
import io.sentry.logger.SentryLogParameters
import io.sentry.protocol.SdkVersion
import io.sentry.transport.ITransport
import io.sentry.util.LazyEvaluator.Evaluator
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
Expand Down Expand Up @@ -108,6 +109,18 @@ class SentryTimberIntegrationTest {
verify(fixture.logs).log(any(), any<SentryLogParameters>(), any<String>())
}

@Test
fun `Integration evaluates Logs provider when registered`() {
var enableLogs = false
val sut = SentryTimberIntegration(Evaluator { enableLogs })
enableLogs = true

sut.register(fixture.scopes, fixture.options)
Timber.i("message")

verify(fixture.logs).log(any(), any<SentryLogParameters>(), any<String>())
}

@Test
fun `Integrations removes a tree from Timber on close integration`() {
val sut = fixture.getSut()
Expand Down
Loading