diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e05874449..604b95a9e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ ## Unreleased +### Performance + +- Most SDK log calls no longer allocate when `debug` is off ([#6600](https://github.com/getsentry/sentry-java/pull/6600)) + ### Fixes - Android: Count deep sleep in breadcrumb, screenshot and view hierarchy debounce windows, so the first low-memory or battery breadcrumb after a long Doze is no longer dropped ([#6590](https://github.com/getsentry/sentry-java/pull/6590)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java index 12ca130be9..bb741d0cb2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java @@ -144,7 +144,7 @@ public boolean onScroll( } else { options .getLogger() - .log(SentryLevel.DEBUG, "Scroll target found: " + target.getIdentifier()); + .log(SentryLevel.DEBUG, "Scroll target found: %s", target.getIdentifier()); } scrollState.setTarget(target); @@ -226,9 +226,8 @@ private void startTracing(final @NotNull UiElement target, final @NotNull Gestur .getLogger() .log( SentryLevel.DEBUG, - "The view with id: " - + viewIdentifier - + " already has an ongoing transaction assigned. Rescheduling finish"); + "The view with id: %s already has an ongoing transaction assigned. Rescheduling finish", + viewIdentifier); final Long idleTimeout = options.getIdleTimeout(); if (idleTimeout != null) { @@ -351,7 +350,7 @@ void applyScope(final @NotNull IScope scope, final @NotNull ITransaction transac if (activity == null) { options .getLogger() - .log(SentryLevel.DEBUG, "Activity is null in " + caller + ". No breadcrumb captured."); + .log(SentryLevel.DEBUG, "Activity is null in %s. No breadcrumb captured.", caller); return null; } @@ -359,7 +358,7 @@ void applyScope(final @NotNull IScope scope, final @NotNull ITransaction transac if (window == null) { options .getLogger() - .log(SentryLevel.DEBUG, "Window is null in " + caller + ". No breadcrumb captured."); + .log(SentryLevel.DEBUG, "Window is null in %s. No breadcrumb captured.", caller); return null; } @@ -367,7 +366,7 @@ void applyScope(final @NotNull IScope scope, final @NotNull ITransaction transac if (decorView == null) { options .getLogger() - .log(SentryLevel.DEBUG, "DecorView is null in " + caller + ". No breadcrumb captured."); + .log(SentryLevel.DEBUG, "DecorView is null in %s. No breadcrumb captured.", caller); return null; } return decorView; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt index 7db354ea64..3de3e05463 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ApplicationExitIntegrationTestBase.kt @@ -27,7 +27,6 @@ import org.junit.Rule import org.junit.rules.TemporaryFolder import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.anyVararg import org.mockito.kotlin.argThat import org.mockito.kotlin.atMost import org.mockito.kotlin.check @@ -172,7 +171,7 @@ abstract class ApplicationExitIntegrationTestBase { verify(fixture.scopes).captureEvent(any(), anyOrNull()) verify(fixture.logger, never()) - .log(any(), argThat { startsWith(config.flushLogPrefix) }, anyVararg()) + .log(any(), argThat { startsWith(config.flushLogPrefix) }, any(), any()) } @Test @@ -185,7 +184,7 @@ abstract class ApplicationExitIntegrationTestBase { verify(fixture.scopes).captureEvent(any(), anyOrNull()) verify(fixture.logger, never()) - .log(any(), argThat { startsWith(config.flushLogPrefix) }, anyVararg()) + .log(any(), argThat { startsWith(config.flushLogPrefix) }, any(), any()) } @Test @@ -317,7 +316,6 @@ abstract class ApplicationExitIntegrationTestBase { .log( any(), argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, - anyVararg(), ) } @@ -338,7 +336,6 @@ abstract class ApplicationExitIntegrationTestBase { .log( any(), argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, - anyVararg(), ) verify(fixture.scopes).captureEvent(any(), any()) } @@ -355,7 +352,6 @@ abstract class ApplicationExitIntegrationTestBase { .log( any(), argThat { startsWith("Timed out waiting to flush previous session to its own file.") }, - anyVararg(), ) assertTrue((fixture.options.envelopeDiskCache as EnvelopeCache).waitPreviousSessionFlush()) } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index dfce80a93d..240a52a43d 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -503,6 +503,9 @@ public final class io/sentry/DiagnosticLogger : io/sentry/ILogger { public fun (Lio/sentry/SentryOptions;Lio/sentry/ILogger;)V public fun getLogger ()Lio/sentry/ILogger; public fun isEnabled (Lio/sentry/SentryLevel;)Z + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Throwable;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;[Ljava/lang/Object;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V @@ -944,6 +947,9 @@ public abstract interface class io/sentry/IHub : io/sentry/IScopes { public abstract interface class io/sentry/ILogger { public abstract fun isEnabled (Lio/sentry/SentryLevel;)Z + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;)V public abstract fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Throwable;)V public abstract fun log (Lio/sentry/SentryLevel;Ljava/lang/String;[Ljava/lang/Object;)V public abstract fun log (Lio/sentry/SentryLevel;Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V @@ -1804,6 +1810,9 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public final class io/sentry/NoOpLogger : io/sentry/ILogger { public static fun getInstance ()Lio/sentry/NoOpLogger; public fun isEnabled (Lio/sentry/SentryLevel;)Z + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;)V + public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Object;Ljava/lang/Object;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;Ljava/lang/Throwable;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/String;[Ljava/lang/Object;)V public fun log (Lio/sentry/SentryLevel;Ljava/lang/Throwable;Ljava/lang/String;[Ljava/lang/Object;)V diff --git a/sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java index 2b5a386300..89373516e2 100644 --- a/sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java @@ -168,7 +168,7 @@ public void onSpanFinished(@NotNull ISpan span) { SentryLevel.DEBUG, "stop collecting performance info for transactions %s (%s)", transaction.getName(), - transaction.getSpanContext().getTraceId().toString()); + transaction.getSpanContext().getTraceId()); for (final @NotNull IPerformanceContinuousCollector collector : continuousCollectors) { collector.onSpanFinished(transaction); @@ -180,7 +180,7 @@ public void onSpanFinished(@NotNull ISpan span) { @Override public @Nullable List stop(final @NotNull String id) { final @Nullable CompositeData data = compositeDataMap.remove(id); - options.getLogger().log(SentryLevel.DEBUG, "stop collecting performance info for " + id); + options.getLogger().log(SentryLevel.DEBUG, "stop collecting performance info for %s", id); // close if there are no more running requests if (compositeDataMap.isEmpty()) { diff --git a/sentry/src/main/java/io/sentry/DiagnosticLogger.java b/sentry/src/main/java/io/sentry/DiagnosticLogger.java index 1f067cd6f1..11b12bc8bd 100644 --- a/sentry/src/main/java/io/sentry/DiagnosticLogger.java +++ b/sentry/src/main/java/io/sentry/DiagnosticLogger.java @@ -57,6 +57,32 @@ public void log( } } + @Override + public void log(final @NotNull SentryLevel level, final @NotNull String message) { + if (logger != null && isEnabled(level)) { + logger.log(level, message); + } + } + + @Override + public void log( + final @NotNull SentryLevel level, final @NotNull String message, final @Nullable Object arg) { + if (logger != null && isEnabled(level)) { + logger.log(level, message, arg); + } + } + + @Override + public void log( + final @NotNull SentryLevel level, + final @NotNull String message, + final @Nullable Object arg1, + final @Nullable Object arg2) { + if (logger != null && isEnabled(level)) { + logger.log(level, message, arg1, arg2); + } + } + /** * Logs a message with the specified level, message and throwable. * diff --git a/sentry/src/main/java/io/sentry/ILogger.java b/sentry/src/main/java/io/sentry/ILogger.java index d709db01d1..6d87eb0dfa 100644 --- a/sentry/src/main/java/io/sentry/ILogger.java +++ b/sentry/src/main/java/io/sentry/ILogger.java @@ -15,6 +15,45 @@ public interface ILogger { */ void log(@NotNull SentryLevel level, @NotNull String message, @Nullable Object... args); + /** + * Logs a message without arguments. This overload and the overloads with one and two arguments do + * not allocate a varargs array at the call site. Thus, a disabled logger costs nothing. + * + * @param level The SentryLevel. + * @param message The message. + */ + default void log(final @NotNull SentryLevel level, final @NotNull String message) { + log(level, message, new Object[0]); + } + + /** + * Logs a message with one formatting argument. + * + * @param level The SentryLevel. + * @param message The message. + * @param arg The argument to format the message. + */ + default void log( + final @NotNull SentryLevel level, final @NotNull String message, final @Nullable Object arg) { + log(level, message, new Object[] {arg}); + } + + /** + * Logs a message with two formatting arguments. + * + * @param level The SentryLevel. + * @param message The message. + * @param arg1 The first argument to format the message. + * @param arg2 The second argument to format the message. + */ + default void log( + final @NotNull SentryLevel level, + final @NotNull String message, + final @Nullable Object arg1, + final @Nullable Object arg2) { + log(level, message, new Object[] {arg1, arg2}); + } + /** * Logs a message with the specified level, message and optional arguments. * diff --git a/sentry/src/main/java/io/sentry/NoOpLogger.java b/sentry/src/main/java/io/sentry/NoOpLogger.java index b42f5c6f05..d78adbaaac 100644 --- a/sentry/src/main/java/io/sentry/NoOpLogger.java +++ b/sentry/src/main/java/io/sentry/NoOpLogger.java @@ -19,6 +19,19 @@ private NoOpLogger() {} @Override public void log(@NotNull SentryLevel level, @NotNull String message, @Nullable Object... args) {} + @Override + public void log(@NotNull SentryLevel level, @NotNull String message) {} + + @Override + public void log(@NotNull SentryLevel level, @NotNull String message, @Nullable Object arg) {} + + @Override + public void log( + @NotNull SentryLevel level, + @NotNull String message, + @Nullable Object arg1, + @Nullable Object arg2) {} + @Override public void log( @NotNull SentryLevel level, @NotNull String message, @Nullable Throwable throwable) {} diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 703f0823c0..cf4c7d3083 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -17,7 +17,6 @@ import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue import org.mockito.kotlin.any -import org.mockito.kotlin.anyVararg import org.mockito.kotlin.argWhere import org.mockito.kotlin.check import org.mockito.kotlin.eq @@ -310,7 +309,13 @@ class OutboxSenderTest { sut.processEnvelopeFile(path, hints) // Additionally make sure we have no errors logged - verify(fixture.logger).log(eq(SentryLevel.ERROR), any(), anyVararg()) + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Item %d of type %s returned null by the parser."), + any(), + any(), + ) verify(fixture.scopes, never()).captureEvent(any()) assertFalse(File(path).exists()) } @@ -328,7 +333,13 @@ class OutboxSenderTest { sut.processEnvelopeFile(path, hints) // Additionally make sure we have no errors logged - verify(fixture.logger).log(eq(SentryLevel.ERROR), any(), anyVararg()) + verify(fixture.logger) + .log( + eq(SentryLevel.ERROR), + eq("Item %d of type %s returned null by the parser."), + any(), + any(), + ) verify(fixture.scopes, never()).captureEvent(any()) assertFalse(File(path).exists()) } diff --git a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt index 4fb74690c3..4c8510facc 100644 --- a/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt +++ b/sentry/src/test/java/io/sentry/PreviousSessionFinalizerTest.kt @@ -13,7 +13,6 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import org.mockito.kotlin.any -import org.mockito.kotlin.anyVararg import org.mockito.kotlin.argThat import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -281,7 +280,6 @@ class PreviousSessionFinalizerTest { "Timed out waiting to flush previous session to its own file in session finalizer." ) }, - anyVararg(), ) verify(fixture.scopes, never()).captureEnvelope(any()) }