fix(android): [Logs and Metrics Enable Flags 19] Read Timber option lazily - #5964
fix(android): [Logs and Metrics Enable Flags 19] Read Timber option lazily#5964adinauer wants to merge 2 commits into
Conversation
Resolve the auto-installed Timber integration's Logs opt-in lazily when the integration registers. This honors programmatic configuration and manifest overrides applied after default integrations are constructed. Co-Authored-By: Claude <noreply@anthropic.com>
d7c56cc to
6e791e5
Compare
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d120ff. Configure here.
| } | ||
|
|
||
| public constructor(enableLogsProvider: Evaluator<Boolean>) : this() { | ||
| this.enableLogsProvider = enableLogsProvider |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 4d120ff. Configure here.
|
|
||
| if (isTimberAvailable) { | ||
| options.addIntegration(new SentryTimberIntegration(options.isEnableTimberLogs())); | ||
| options.addIntegration(new SentryTimberIntegration(options::isEnableTimberLogs)); |
There was a problem hiding this comment.
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.


PR Stack (Logs and Metrics Enable Flags)
📜 Description
Makes the auto-installed Timber integration resolve
enableTimberLogslazily when the integration registers rather than snapshotting it while Android default integrations are constructed.This honors the final option value after the Android configuration callback, including both programmatic opt-in and programmatic override of manifest configuration. Existing boolean constructors continue to behave as fixed values for manually installed integrations.
💡 Motivation and Context
Android installs default integrations before invoking the user's options callback so users can remove or replace them. Passing the current boolean value into Timber at that earlier point caused later programmatic configuration to be ignored.
💚 How did you test it?
./gradlew :sentry-android-timber:testReleaseUnitTest :sentry-android-core:testReleaseUnitTest./gradlew spotlessApply apiDumpfalse -> trueand manifesttrue -> callback false📝 Checklist
sendDefaultPIIis enabled.#skip-changelog