From ed69adcb8b40c6220ec92063667bee5162427036 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 21 Dec 2023 17:08:50 +0100 Subject: [PATCH 1/6] added TransactionContext.isForNextStartup added SentryOptions.enableStartupProfiling --- .../main/java/io/sentry/SentryOptions.java | 21 +++++++++++++++++++ .../java/io/sentry/TransactionContext.java | 17 +++++++++++++++ .../test/java/io/sentry/SentryOptionsTest.kt | 13 ++++++++++++ .../java/io/sentry/TransactionContextTest.kt | 13 ++++++++++++ 4 files changed, 64 insertions(+) diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 448e85a115..af6bca2e71 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -440,6 +440,9 @@ public class SentryOptions { /** Contains a list of monitor slugs for which check-ins should not be sent. */ @ApiStatus.Experimental private @Nullable List ignoredCheckIns = null; + /** Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. */ + private boolean enableStartupProfiling = false; + /** * Adds an event processor * @@ -2115,6 +2118,24 @@ public void setEnablePrettySerializationOutput(boolean enablePrettySerialization this.enablePrettySerializationOutput = enablePrettySerializationOutput; } + /** + * Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. + * + * @return true if startup profiling should be started. + */ + public boolean isEnableStartupProfiling() { + return enableStartupProfiling; + } + + /** + * Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. + * + * @param enableStartupProfiling true if startup profiling should be started. + */ + public void setEnableStartupProfiling(boolean enableStartupProfiling) { + this.enableStartupProfiling = enableStartupProfiling; + } + /** * Whether to send modules containing information about versions. * diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index 0885b9916a..ab34b0261a 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -18,6 +18,7 @@ public final class TransactionContext extends SpanContext { private @Nullable TracesSamplingDecision parentSamplingDecision; private @Nullable Baggage baggage; private @NotNull Instrumenter instrumenter = Instrumenter.SENTRY; + private boolean isForNextStartup = false; /** * Creates {@link TransactionContext} from sentry-trace header. @@ -200,4 +201,20 @@ public void setName(final @NotNull String name) { public void setTransactionNameSource(final @NotNull TransactionNameSource transactionNameSource) { this.transactionNameSource = transactionNameSource; } + + @ApiStatus.Internal + public void setForNextStartup(final boolean forNextStartup) { + isForNextStartup = forNextStartup; + } + + /** + * Whether this {@link TransactionContext} evaluates for the next startup. + * If this is true, it gets called only once when the SDK initializes. + * This is set only if {@link SentryOptions#isEnableStartupProfiling()} is true. + * + * @return True if this {@link TransactionContext} will be used for the next startup. + */ + public boolean isForNextStartup() { + return isForNextStartup; + } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 3fcc9fa88c..3254a2df94 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -514,6 +514,7 @@ class SentryOptionsTest { assertEquals(customProvider, options.connectionStatusProvider) } + @Test fun `when options are initialized, enabled is set to true by default`() { assertTrue(SentryOptions().isEnabled) } @@ -527,4 +528,16 @@ class SentryOptionsTest { fun `when options are initialized, sendModules is set to true by default`() { assertTrue(SentryOptions().isSendModules) } + + @Test + fun `when options are initialized, enableStartupProfiling is set to false by default`() { + assertFalse(SentryOptions().isEnableStartupProfiling) + } + + @Test + fun `when setEnableStartupProfiling is called, overrides default`() { + val options = SentryOptions() + options.isEnableStartupProfiling = true + assertTrue(options.isEnableStartupProfiling) + } } diff --git a/sentry/src/test/java/io/sentry/TransactionContextTest.kt b/sentry/src/test/java/io/sentry/TransactionContextTest.kt index 6f5a0ead33..25e4078fae 100644 --- a/sentry/src/test/java/io/sentry/TransactionContextTest.kt +++ b/sentry/src/test/java/io/sentry/TransactionContextTest.kt @@ -18,6 +18,7 @@ class TransactionContextTest { assertNull(context.parentSampled) assertEquals("name", context.name) assertEquals("op", context.op) + assertFalse(context.isForNextStartup) } @Test @@ -27,6 +28,7 @@ class TransactionContextTest { assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) + assertFalse(context.isForNextStartup) } @Test @@ -42,6 +44,7 @@ class TransactionContextTest { assertNull(context.profileSampled) assertFalse(context.parentSampled!!) assertEquals(0.3, context.parentSamplingDecision!!.sampleRate) + assertFalse(context.isForNextStartup) } @Test @@ -57,6 +60,7 @@ class TransactionContextTest { assertNull(context.profileSampled) assertFalse(context.parentSampled!!) assertNull(context.parentSamplingDecision!!.sampleRate) + assertFalse(context.isForNextStartup) } @Test @@ -72,6 +76,7 @@ class TransactionContextTest { assertNull(context.profileSampled) assertTrue(context.parentSampled!!) assertEquals(0.3, context.parentSamplingDecision!!.sampleRate) + assertFalse(context.isForNextStartup) } @Test @@ -87,5 +92,13 @@ class TransactionContextTest { assertNull(context.profileSampled) assertTrue(context.parentSampled!!) assertNull(context.parentSamplingDecision!!.sampleRate) + assertFalse(context.isForNextStartup) + } + + @Test + fun `setForNextStartup sets the isForNextStartup flag`() { + val context = TransactionContext("name", "op") + context.isForNextStartup = true + assertTrue(context.isForNextStartup) } } From 3f7d20663f2b1faccf3e110a13783b0ca0fe48c3 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 29 Dec 2023 10:57:54 +0100 Subject: [PATCH 2/6] added SentryStartupProfilingOptions class with Json ser/deser added startupProfilingConfigFile deletion and creation on init added sampling decision on SDK init with isForNextStartup flag set to true added SentryOptions.getCacheDirPathWithoutDsn for startupProfiling config file --- sentry/api/sentry.api | 4 + .../main/java/io/sentry/JsonSerializer.java | 2 + sentry/src/main/java/io/sentry/Sentry.java | 62 ++++++- .../main/java/io/sentry/SentryOptions.java | 17 +- .../sentry/SentryStartupProfilingOptions.java | 146 ++++++++++++++++ .../java/io/sentry/TransactionContext.java | 6 +- .../test/java/io/sentry/JsonSerializerTest.kt | 33 ++++ .../test/java/io/sentry/SentryOptionsTest.kt | 25 +++ sentry/src/test/java/io/sentry/SentryTest.kt | 164 ++++++++++++++++++ 9 files changed, 452 insertions(+), 7 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e78d63f740..5fe8498db3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2162,6 +2162,7 @@ public class io/sentry/SentryOptions { public fun isEnableExternalConfiguration ()Z public fun isEnablePrettySerializationOutput ()Z public fun isEnableShutdownHook ()Z + public fun isEnableStartupProfiling ()Z public fun isEnableTimeToFullDisplayTracing ()Z public fun isEnableUncaughtExceptionHandler ()Z public fun isEnableUserInteractionBreadcrumbs ()Z @@ -2197,6 +2198,7 @@ public class io/sentry/SentryOptions { public fun setEnableExternalConfiguration (Z)V public fun setEnablePrettySerializationOutput (Z)V public fun setEnableShutdownHook (Z)V + public fun setEnableStartupProfiling (Z)V public fun setEnableTimeToFullDisplayTracing (Z)V public fun setEnableTracing (Ljava/lang/Boolean;)V public fun setEnableUncaughtExceptionHandler (Z)V @@ -2702,6 +2704,8 @@ public final class io/sentry/TransactionContext : io/sentry/SpanContext { public fun getParentSampled ()Ljava/lang/Boolean; public fun getParentSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource; + public fun isForNextStartup ()Z + public fun setForNextStartup (Z)V public fun setInstrumenter (Lio/sentry/Instrumenter;)V public fun setName (Ljava/lang/String;)V public fun setParentSampled (Ljava/lang/Boolean;)V diff --git a/sentry/src/main/java/io/sentry/JsonSerializer.java b/sentry/src/main/java/io/sentry/JsonSerializer.java index fbdc76d65e..71ea44fae8 100644 --- a/sentry/src/main/java/io/sentry/JsonSerializer.java +++ b/sentry/src/main/java/io/sentry/JsonSerializer.java @@ -104,6 +104,8 @@ public JsonSerializer(@NotNull SentryOptions options) { deserializersByClass.put(SentrySpan.class, new SentrySpan.Deserializer()); deserializersByClass.put(SentryStackFrame.class, new SentryStackFrame.Deserializer()); deserializersByClass.put(SentryStackTrace.class, new SentryStackTrace.Deserializer()); + deserializersByClass.put( + SentryStartupProfilingOptions.class, new SentryStartupProfilingOptions.Deserializer()); deserializersByClass.put(SentryThread.class, new SentryThread.Deserializer()); deserializersByClass.put(SentryTransaction.class, new SentryTransaction.Deserializer()); deserializersByClass.put(Session.class, new Session.Deserializer()); diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 368bfc8b57..c391bf2ce0 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -3,6 +3,7 @@ import io.sentry.cache.EnvelopeCache; import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; +import io.sentry.instrumentation.file.SentryFileWriter; import io.sentry.internal.debugmeta.NoOpDebugMetaLoader; import io.sentry.internal.debugmeta.ResourcesDebugMetaLoader; import io.sentry.internal.modules.CompositeModulesLoader; @@ -20,6 +21,8 @@ import io.sentry.util.thread.MainThreadChecker; import io.sentry.util.thread.NoOpMainThreadChecker; import java.io.File; +import java.io.IOException; +import java.io.Writer; import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.List; @@ -46,6 +49,9 @@ private Sentry() {} /** whether to use a single (global) Hub as opposed to one per thread. */ private static volatile boolean globalHubMode = GLOBAL_HUB_DEFAULT_MODE; + private static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = + "startup_profiling_config"; + /** * Returns the current (threads) hub, if none, clones the mainHub and returns it. * @@ -224,9 +230,7 @@ private static synchronized void init( // If the executorService passed in the init is the same that was previously closed, we have to // set a new one - final ISentryExecutorService sentryExecutorService = options.getExecutorService(); - // If the passed executor service was previously called we set a new one - if (sentryExecutorService.isClosed()) { + if (options.getExecutorService().isClosed()) { options.setExecutorService(new SentryExecutorService()); } @@ -241,6 +245,58 @@ private static synchronized void init( notifyOptionsObservers(options); finalizePreviousSession(options, HubAdapter.getInstance()); + + handleStartupProfilingConfig(options, options.getExecutorService()); + } + + @SuppressWarnings("FutureReturnValueIgnored") + private static void handleStartupProfilingConfig( + final @NotNull SentryOptions options, + final @NotNull ISentryExecutorService sentryExecutorService) { + sentryExecutorService.submit( + () -> { + final String cacheDirPath = options.getCacheDirPathWithoutDsn(); + if (cacheDirPath != null) { + final @NotNull File startupProfilingConfigFile = + new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); + // We always delete the config file for startup profiling + FileUtils.deleteRecursively(startupProfilingConfigFile); + if (!options.isEnableStartupProfiling()) { + return; + } + if (!options.isTracingEnabled()) { + options + .getLogger() + .log( + SentryLevel.INFO, + "Tracing is disabled and startup profiling will not start."); + return; + } + try { + if (startupProfilingConfigFile.createNewFile()) { + final @NotNull TracesSamplingDecision startupSamplingDecision = + sampleStartupProfiling(options); + final @NotNull SentryStartupProfilingOptions startupProfilingOptions = + new SentryStartupProfilingOptions(options, startupSamplingDecision); + try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) { + options.getSerializer().serialize(startupProfilingOptions, fileWriter); + } + } + } catch (IOException e) { + options + .getLogger() + .log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); + } + } + }); + } + + private static @NotNull TracesSamplingDecision sampleStartupProfiling( + final @NotNull SentryOptions options) { + TransactionContext startupTransactionContext = new TransactionContext("ui.load", ""); + startupTransactionContext.setForNextStartup(true); + SamplingContext startupSamplingContext = new SamplingContext(startupTransactionContext, null); + return new TracesSampler(options).sample(startupSamplingContext); } @SuppressWarnings("FutureReturnValueIgnored") diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index af6bca2e71..36590a8f8f 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -726,6 +726,20 @@ public void setBeforeBreadcrumb(@Nullable BeforeBreadcrumbCallback beforeBreadcr return dsnHash != null ? new File(cacheDirPath, dsnHash).getAbsolutePath() : cacheDirPath; } + /** + * Returns the cache dir path if set, without the appended dsn hash. + * + * @return the cache dir path, without the appended dsn hash, or null if not set. + */ + @Nullable + String getCacheDirPathWithoutDsn() { + if (cacheDirPath == null || cacheDirPath.isEmpty()) { + return null; + } + + return cacheDirPath; + } + /** * Returns the outbox path if cacheDirPath is set * @@ -2120,11 +2134,12 @@ public void setEnablePrettySerializationOutput(boolean enablePrettySerialization /** * Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. + * Depends on {@link SentryOptions#isProfilingEnabled()} * * @return true if startup profiling should be started. */ public boolean isEnableStartupProfiling() { - return enableStartupProfiling; + return isProfilingEnabled() && enableStartupProfiling; } /** diff --git a/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java new file mode 100644 index 0000000000..c630e80d4f --- /dev/null +++ b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java @@ -0,0 +1,146 @@ +package io.sentry; + +import io.sentry.vendor.gson.stream.JsonToken; +import java.io.IOException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { + + boolean profileSampled; + @Nullable Double profileSampleRate; + boolean traceSampled; + @Nullable Double traceSampleRate; + @Nullable String profilingTracesDirPath; + boolean isProfilingEnabled; + + private @Nullable Map unknown; + + SentryStartupProfilingOptions() { + traceSampled = false; + traceSampleRate = null; + profileSampled = false; + profileSampleRate = null; + profilingTracesDirPath = null; + isProfilingEnabled = false; + } + + SentryStartupProfilingOptions( + final @NotNull SentryOptions options, + final @NotNull TracesSamplingDecision samplingDecision) { + traceSampled = samplingDecision.getSampled(); + traceSampleRate = samplingDecision.getSampleRate(); + profileSampled = samplingDecision.getProfileSampled(); + profileSampleRate = samplingDecision.getProfileSampleRate(); + profilingTracesDirPath = options.getProfilingTracesDirPath(); + isProfilingEnabled = options.isProfilingEnabled(); + } + + // JsonSerializable + + public static final class JsonKeys { + public static final String PROFILE_SAMPLED = "profile_sampled"; + public static final String PROFILE_SAMPLE_RATE = "profile_sample_rate"; + public static final String TRACE_SAMPLED = "trace_sampled"; + public static final String TRACE_SAMPLE_RATE = "trace_sample_rate"; + public static final String PROFILING_TRACES_DIR_PATH = "profiling_traces_dir_path"; + public static final String IS_PROFILING_ENABLED = "is_profiling_enabled"; + } + + @Override + public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) + throws IOException { + writer.beginObject(); + writer.name(JsonKeys.PROFILE_SAMPLED).value(logger, profileSampled); + writer.name(JsonKeys.PROFILE_SAMPLE_RATE).value(logger, profileSampleRate); + writer.name(JsonKeys.TRACE_SAMPLED).value(logger, traceSampled); + writer.name(JsonKeys.TRACE_SAMPLE_RATE).value(logger, traceSampleRate); + writer.name(JsonKeys.PROFILING_TRACES_DIR_PATH).value(logger, profilingTracesDirPath); + writer.name(JsonKeys.IS_PROFILING_ENABLED).value(logger, isProfilingEnabled); + + if (unknown != null) { + for (String key : unknown.keySet()) { + Object value = unknown.get(key); + writer.name(key); + writer.value(logger, value); + } + } + writer.endObject(); + } + + @Nullable + @Override + public Map getUnknown() { + return unknown; + } + + @Override + public void setUnknown(@Nullable Map unknown) { + this.unknown = unknown; + } + + public static final class Deserializer + implements JsonDeserializer { + + @Override + public @NotNull SentryStartupProfilingOptions deserialize( + @NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { + reader.beginObject(); + SentryStartupProfilingOptions options = new SentryStartupProfilingOptions(); + Map unknown = null; + + while (reader.peek() == JsonToken.NAME) { + final String nextName = reader.nextName(); + switch (nextName) { + case JsonKeys.PROFILE_SAMPLED: + Boolean profileSampled = reader.nextBooleanOrNull(); + if (profileSampled != null) { + options.profileSampled = profileSampled; + } + break; + case JsonKeys.PROFILE_SAMPLE_RATE: + Double profileSampleRate = reader.nextDoubleOrNull(); + if (profileSampleRate != null) { + options.profileSampleRate = profileSampleRate; + } + break; + case JsonKeys.TRACE_SAMPLED: + Boolean traceSampled = reader.nextBooleanOrNull(); + if (traceSampled != null) { + options.traceSampled = traceSampled; + } + break; + case JsonKeys.TRACE_SAMPLE_RATE: + Double traceSampleRate = reader.nextDoubleOrNull(); + if (traceSampleRate != null) { + options.traceSampleRate = traceSampleRate; + } + break; + case JsonKeys.PROFILING_TRACES_DIR_PATH: + String profilingTracesDirPath = reader.nextStringOrNull(); + if (profilingTracesDirPath != null) { + options.profilingTracesDirPath = profilingTracesDirPath; + } + break; + case JsonKeys.IS_PROFILING_ENABLED: + Boolean isProfilingEnabled = reader.nextBooleanOrNull(); + if (isProfilingEnabled != null) { + options.isProfilingEnabled = isProfilingEnabled; + } + break; + default: + if (unknown == null) { + unknown = new ConcurrentHashMap<>(); + } + reader.nextUnknown(logger, unknown, nextName); + break; + } + } + options.setUnknown(unknown); + reader.endObject(); + return options; + } + } +} diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index ab34b0261a..087f8ec01a 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -208,9 +208,9 @@ public void setForNextStartup(final boolean forNextStartup) { } /** - * Whether this {@link TransactionContext} evaluates for the next startup. - * If this is true, it gets called only once when the SDK initializes. - * This is set only if {@link SentryOptions#isEnableStartupProfiling()} is true. + * Whether this {@link TransactionContext} evaluates for the next startup. If this is true, it + * gets called only once when the SDK initializes. This is set only if {@link + * SentryOptions#isEnableStartupProfiling()} is true. * * @return True if this {@link TransactionContext} will be used for the next startup. */ diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 93fb64c5b0..2af7250d83 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -973,6 +973,31 @@ class JsonSerializerTest { } } + @Test + fun `serializing SentryStartupProfilingOptions`() { + val actual = serializeToString(startupProfilingOptions) + + val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\":false," + + "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + + assertEquals(expected, actual) + } + + @Test + fun `deserializing SentryStartupProfilingOptions`() { + val jsonStartupProfilingOptions = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\"" + + ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + + val actual = fixture.serializer.deserialize(StringReader(jsonStartupProfilingOptions), SentryStartupProfilingOptions::class.java) + assertNotNull(actual) + assertEquals(startupProfilingOptions.traceSampled, actual.traceSampled) + assertEquals(startupProfilingOptions.traceSampleRate, actual.traceSampleRate) + assertEquals(startupProfilingOptions.profileSampled, actual.profileSampled) + assertEquals(startupProfilingOptions.profileSampleRate, actual.profileSampleRate) + assertEquals(startupProfilingOptions.isProfilingEnabled, actual.isProfilingEnabled) + assertNull(actual.unknown) + } + @Test fun `serializes span data`() { val sentrySpan = SentrySpan(createSpan() as Span, mapOf("data1" to "value1")) @@ -1249,6 +1274,14 @@ class JsonSerializerTest { } } + private val startupProfilingOptions = SentryStartupProfilingOptions().apply { + traceSampled = false + traceSampleRate = 0.1 + profileSampled = true + profileSampleRate = 0.8 + isProfilingEnabled = false + } + private fun createSpan(): ISpan { val trace = TransactionContext("transaction-name", "http").apply { description = "some request" diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 3254a2df94..319711c901 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -8,6 +8,7 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertIs +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -460,6 +461,21 @@ class SentryOptionsTest { assertEquals("${File.separator}test${File.separator}${hash}${File.separator}profiling_traces", options.profilingTracesDirPath) } + @Test + fun `getCacheDirPathWithoutDsn does not contain dsn hash`() { + val dsn = "http://key@localhost/proj" + val hash = StringUtils.calculateStringHash(dsn, mock()) + val options = SentryOptions().apply { + setDsn(dsn) + cacheDirPath = "${File.separator}test" + } + + val cacheDirPathWithoutDsn = options.cacheDirPathWithoutDsn!! + assertNotEquals(cacheDirPathWithoutDsn, options.cacheDirPath) + assertEquals(cacheDirPathWithoutDsn, options.cacheDirPath!!.substringBeforeLast("/")) + assertFalse(cacheDirPathWithoutDsn.contains(hash.toString())) + } + @Test fun `when options are initialized, idleTimeout is 3000`() { assertEquals(3000L, SentryOptions().idleTimeout) @@ -538,6 +554,15 @@ class SentryOptionsTest { fun `when setEnableStartupProfiling is called, overrides default`() { val options = SentryOptions() options.isEnableStartupProfiling = true + options.profilesSampleRate = 1.0 assertTrue(options.isEnableStartupProfiling) } + + @Test + fun `when profiling is disabled, isEnableStartupProfiling is always false`() { + val options = SentryOptions() + options.isEnableStartupProfiling = true + options.profilesSampleRate = 0.0 + assertFalse(options.isEnableStartupProfiling) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 026301ee44..0f98f68458 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -1,5 +1,7 @@ package io.sentry +import io.sentry.SentryOptions.ProfilesSamplerCallback +import io.sentry.SentryOptions.TracesSamplerCallback import io.sentry.cache.EnvelopeCache import io.sentry.cache.IEnvelopeCache import io.sentry.internal.debugmeta.IDebugMetaLoader @@ -21,11 +23,14 @@ import org.junit.rules.TemporaryFolder import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat +import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.io.File +import java.io.FileReader import java.nio.file.Files import java.util.Properties import java.util.concurrent.CompletableFuture @@ -83,6 +88,21 @@ class SentryTest { file.deleteOnExit() } + @Test + fun `getCacheDirPathWithoutDsn should be created at initialization`() { + var sentryOptions: SentryOptions? = null + Sentry.init { + it.dsn = dsn + it.cacheDirPath = getTempPath() + sentryOptions = it + } + + val cacheDirPathWithoutDsn = sentryOptions!!.cacheDirPathWithoutDsn!! + val file = File(cacheDirPathWithoutDsn) + assertTrue(file.exists()) + file.deleteOnExit() + } + @Test fun `Init sets SystemOutLogger if logger is NoOp and debug is enabled`() { var sentryOptions: SentryOptions? = null @@ -920,6 +940,150 @@ class SentryTest { assertEquals("op-child", span.operation) } + @Test + fun `init calls samplers if isEnableStartupProfiling is true`() { + val mockSampleTracer = mock() + val mockProfilesSampler = mock() + Sentry.init { + it.dsn = dsn + it.enableTracing = true + it.isEnableStartupProfiling = true + it.profilesSampleRate = 1.0 + it.tracesSampler = mockSampleTracer + it.profilesSampler = mockProfilesSampler + it.executorService = ImmediateExecutorService() + it.cacheDirPath = getTempPath() + } + // Samplers are called with isForNextStartup flag set to true + verify(mockSampleTracer).sample( + check { + assertTrue(it.transactionContext.isForNextStartup) + } + ) + verify(mockProfilesSampler).sample( + check { + assertTrue(it.transactionContext.isForNextStartup) + } + ) + } + + @Test + fun `init calls startup profiling samplers in the background`() { + val mockSampleTracer = mock() + val mockProfilesSampler = mock() + Sentry.init { + it.dsn = dsn + it.enableTracing = true + it.isEnableStartupProfiling = true + it.profilesSampleRate = 1.0 + it.tracesSampler = mockSampleTracer + it.profilesSampler = mockProfilesSampler + it.executorService = NoOpSentryExecutorService.getInstance() + it.cacheDirPath = getTempPath() + } + // Samplers are called with isForNextStartup flag set to true + verify(mockSampleTracer, never()).sample(any()) + verify(mockProfilesSampler, never()).sample(any()) + } + + @Test + fun `init does not call startup profiling samplers if cache dir is null`() { + val mockSampleTracer = mock() + val mockProfilesSampler = mock() + Sentry.init { + it.dsn = dsn + it.enableTracing = true + it.isEnableStartupProfiling = true + it.profilesSampleRate = 1.0 + it.tracesSampler = mockSampleTracer + it.profilesSampler = mockProfilesSampler + it.executorService = NoOpSentryExecutorService.getInstance() + it.cacheDirPath = null + } + // Samplers are called with isForNextStartup flag set to true + verify(mockSampleTracer, never()).sample(any()) + verify(mockProfilesSampler, never()).sample(any()) + } + + @Test + fun `init does not call startup profiling samplers if enableTracing is false`() { + val logger = mock() + val mockTraceSampler = mock() + val mockProfilesSampler = mock() + Sentry.init { + it.dsn = dsn + it.enableTracing = false + it.isEnableStartupProfiling = true + it.profilesSampleRate = 1.0 + it.tracesSampler = mockTraceSampler + it.profilesSampler = mockProfilesSampler + it.executorService = ImmediateExecutorService() + it.cacheDirPath = getTempPath() + it.isDebug = true + it.setLogger(logger) + } + verify(logger).log(eq(SentryLevel.INFO), eq("Tracing is disabled and startup profiling will not start.")) + verify(mockTraceSampler, never()).sample(any()) + verify(mockProfilesSampler, never()).sample(any()) + } + + @Test + fun `init deletes startup profiling config`() { + val path = getTempPath() + File(path).mkdirs() + val startupProfilingConfigFile = File(path, "startup_profiling_config") + startupProfilingConfigFile.createNewFile() + assertTrue(startupProfilingConfigFile.exists()) + Sentry.init { + it.dsn = dsn + it.executorService = ImmediateExecutorService() + it.cacheDirPath = path + } + assertFalse(startupProfilingConfigFile.exists()) + } + + @Test + fun `init creates startup profiling config if isEnableStartupProfiling and enableTracing is true`() { + val path = getTempPath() + File(path).mkdirs() + val startupProfilingConfigFile = File(path, "startup_profiling_config") + startupProfilingConfigFile.createNewFile() + assertTrue(startupProfilingConfigFile.exists()) + Sentry.init { + it.dsn = dsn + it.cacheDirPath = path + it.isEnableStartupProfiling = true + it.profilesSampleRate = 1.0 + it.enableTracing = true + it.executorService = ImmediateExecutorService() + } + assertTrue(startupProfilingConfigFile.exists()) + } + + @Test + fun `init saves SentryStartupProfilingOptions to disk`() { + var options = SentryOptions() + val path = getTempPath() + Sentry.init { + it.dsn = dsn + it.cacheDirPath = path + it.enableTracing = true + it.tracesSampleRate = 0.5 + it.isEnableStartupProfiling = true + it.profilesSampleRate = 0.2 + it.executorService = ImmediateExecutorService() + options = it + } + val startupProfilingConfigFile = File(path, "startup_profiling_config") + assertTrue(startupProfilingConfigFile.exists()) + val startupOption = + JsonSerializer(options).deserialize(FileReader(startupProfilingConfigFile), SentryStartupProfilingOptions::class.java) + assertNotNull(startupOption) + assertEquals(0.5, startupOption.traceSampleRate) + assertEquals(0.2, startupOption.profileSampleRate) + assertTrue(startupOption.isProfilingEnabled) + } + private class InMemoryOptionsObserver : IOptionsObserver { var release: String? = null private set From a44b5e386cc1cb8df0e0397c71bc79642de12d8f Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 29 Dec 2023 11:14:05 +0100 Subject: [PATCH 3/6] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfb876853d..4131f014bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096)) - Startup profiling 1 - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101)) +- Startup profiling 2 - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121)) ### Fixes From c4b79d2cc34c9de38c87a4f8535352b3c47bb17b Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 11 Jan 2024 15:36:48 +0100 Subject: [PATCH 4/6] put startup config deletion inside try catch used FileOutputStream instead for FileWriter --- sentry/src/main/java/io/sentry/Sentry.java | 43 ++++++++++++++-------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index dfd51c9db7..a43c78a5ea 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -4,7 +4,6 @@ import io.sentry.cache.EnvelopeCache; import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; -import io.sentry.instrumentation.file.SentryFileWriter; import io.sentry.internal.debugmeta.NoOpDebugMetaLoader; import io.sentry.internal.debugmeta.ResourcesDebugMetaLoader; import io.sentry.internal.modules.CompositeModulesLoader; @@ -21,10 +20,15 @@ import io.sentry.util.thread.IMainThreadChecker; import io.sentry.util.thread.MainThreadChecker; import io.sentry.util.thread.NoOpMainThreadChecker; +import java.io.BufferedWriter; import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.Writer; import java.lang.reflect.InvocationTargetException; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.List; import java.util.Properties; @@ -53,6 +57,9 @@ private Sentry() {} private static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = "startup_profiling_config"; + @SuppressWarnings("CharsetObjectCanBeUsed") + private static final Charset UTF_8 = Charset.forName("UTF-8"); + /** * Returns the current (threads) hub, if none, clones the mainHub and returns it. * @@ -260,27 +267,31 @@ private static void handleStartupProfilingConfig( if (cacheDirPath != null) { final @NotNull File startupProfilingConfigFile = new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); - // We always delete the config file for startup profiling - FileUtils.deleteRecursively(startupProfilingConfigFile); - if (!options.isEnableStartupProfiling()) { - return; - } - if (!options.isTracingEnabled()) { - options - .getLogger() - .log( - SentryLevel.INFO, - "Tracing is disabled and startup profiling will not start."); - return; - } try { + // We always delete the config file for startup profiling + FileUtils.deleteRecursively(startupProfilingConfigFile); + if (!options.isEnableStartupProfiling()) { + return; + } + if (!options.isTracingEnabled()) { + options + .getLogger() + .log( + SentryLevel.INFO, + "Tracing is disabled and startup profiling will not start."); + return; + } + if (startupProfilingConfigFile.createNewFile()) { final @NotNull TracesSamplingDecision startupSamplingDecision = sampleStartupProfiling(options); final @NotNull SentryStartupProfilingOptions startupProfilingOptions = new SentryStartupProfilingOptions(options, startupSamplingDecision); - try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) { - options.getSerializer().serialize(startupProfilingOptions, fileWriter); + try (final OutputStream outputStream = + new FileOutputStream(startupProfilingConfigFile); + final Writer writer = + new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) { + options.getSerializer().serialize(startupProfilingOptions, writer); } } } catch (IOException e) { From 7d13d655af7fc9653e5bcbd1c6aeb11e36fd2b75 Mon Sep 17 00:00:00 2001 From: Stefano Date: Fri, 12 Jan 2024 17:07:17 +0100 Subject: [PATCH 5/6] Startup Profiling 3 - Add ContentProvider and start profile (#3128) * first activity transaction now inherits startup sampling decision, if available * if a startup profiler was instantiated, it will be reused in AndroidOptionsInitializer, instead of creating a new one * added ITransactionProfiler.isRunning * startup profiler and sampling decision is stored in AppStartMetrics * startup profile is bound to the startup transaction * added io.sentry.profiling.enable-startup manifest option * moved profilingTracesHz from SentryAndroidOptions to SentryOptions * added startup profiling launch in SentryPerformanceProvider * added isStartupTransaction to TransactionOptions --- CHANGELOG.md | 1 + .../api/sentry-android-core.api | 7 +- .../core/ActivityLifecycleIntegration.java | 16 +- .../core/AndroidOptionsInitializer.java | 37 ++- .../core/AndroidTransactionProfiler.java | 9 + .../android/core/ManifestMetadataReader.java | 6 + .../android/core/SentryAndroidOptions.java | 23 -- .../core/SentryPerformanceProvider.java | 131 ++++++++++- .../core/performance/AppStartMetrics.java | 28 +++ .../core/ActivityLifecycleIntegrationTest.kt | 58 ++++- .../core/AndroidOptionsInitializerTest.kt | 6 + .../core/AndroidTransactionProfilerTest.kt | 32 +++ .../core/ManifestMetadataReaderTest.kt | 27 +++ .../core/SentryPerformanceProviderTest.kt | 215 ++++++++++++++++-- .../core/performance/AppStartMetricsTest.kt | 6 +- .../src/main/AndroidManifest.xml | 2 + sentry/api/sentry.api | 46 ++++ sentry/src/main/java/io/sentry/Hub.java | 10 +- .../java/io/sentry/ITransactionProfiler.java | 2 + .../io/sentry/NoOpTransactionProfiler.java | 5 + sentry/src/main/java/io/sentry/Sentry.java | 92 ++++---- .../main/java/io/sentry/SentryOptions.java | 26 ++- .../sentry/SentryStartupProfilingOptions.java | 75 +++++- .../java/io/sentry/TransactionOptions.java | 13 ++ sentry/src/test/java/io/sentry/HubTest.kt | 42 ++++ .../test/java/io/sentry/JsonSerializerTest.kt | 7 +- .../io/sentry/NoOpTransactionProfilerTest.kt | 6 + .../test/java/io/sentry/SentryOptionsTest.kt | 12 + sentry/src/test/java/io/sentry/SentryTest.kt | 14 +- 29 files changed, 840 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4131f014bd..da767f1fc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096)) - Startup profiling 1 - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101)) - Startup profiling 2 - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121)) +- Startup Profiling 3 - Add ContentProvider and start profile ([#3128](https://github.com/getsentry/sentry-java/pull/3128)) ### Fixes diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 360364d59d..03c2014909 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -260,7 +260,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getBeforeViewHierarchyCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback; public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader; public fun getNativeSdkName ()Ljava/lang/String; - public fun getProfilingTracesHz ()I public fun getProfilingTracesIntervalMillis ()I public fun getStartupCrashDurationThresholdMillis ()J public fun isAnrEnabled ()Z @@ -305,7 +304,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableScopeSync (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V public fun setNativeSdkName (Ljava/lang/String;)V - public fun setProfilingTracesHz (I)V public fun setProfilingTracesIntervalMillis (I)V public fun setReportHistoricalAnrs (Z)V } @@ -345,6 +343,7 @@ public final class io/sentry/android/core/SentryPerformanceProvider { public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V public fun getType (Landroid/net/Uri;)Ljava/lang/String; public fun onCreate ()Z + public fun shutdown ()V } public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { @@ -426,12 +425,16 @@ public class io/sentry/android/core/performance/AppStartMetrics { public fun getContentProviderOnCreateTimeSpans ()Ljava/util/List; public static fun getInstance ()Lio/sentry/android/core/performance/AppStartMetrics; public fun getSdkInitTimeSpan ()Lio/sentry/android/core/performance/TimeSpan; + public fun getStartupProfiler ()Lio/sentry/ITransactionProfiler; + public fun getStartupSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun isAppLaunchedInForeground ()Z public static fun onApplicationCreate (Landroid/app/Application;)V public static fun onApplicationPostCreate (Landroid/app/Application;)V public static fun onContentProviderCreate (Landroid/content/ContentProvider;)V public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V + public fun setStartupProfiler (Lio/sentry/ITransactionProfiler;)V + public fun setStartupSamplingDecision (Lio/sentry/TracesSamplingDecision;)V } public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 645094ed50..9e32a847dc 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -23,6 +23,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.SpanStatus; +import io.sentry.TracesSamplingDecision; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; import io.sentry.android.core.internal.util.ClassUtil; @@ -161,7 +162,7 @@ private void startTracing(final @NotNull Activity activity) { final @Nullable SentryDate appStartTime; final @Nullable Boolean coldStart; - final TimeSpan appStartTimeSpan = + final @NotNull TimeSpan appStartTimeSpan = AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(options); // we only track app start for processes that will show an Activity (full launch). @@ -205,20 +206,31 @@ private void startTracing(final @NotNull Activity activity) { // This will be the start timestamp of the transaction, as well as the ttid/ttfd spans final @NotNull SentryDate ttidStartTime; + final @Nullable TracesSamplingDecision startupSamplingDecision; if (!(firstActivityCreated || appStartTime == null || coldStart == null)) { // The first activity ttid/ttfd spans should start at the app start time ttidStartTime = appStartTime; + // The app start transaction inherits the sampling decision from the startup profiling, + // then clears it + startupSamplingDecision = AppStartMetrics.getInstance().getStartupSamplingDecision(); + AppStartMetrics.getInstance().setStartupSamplingDecision(null); } else { // The ttid/ttfd spans should start when the previous activity called its onPause method ttidStartTime = lastPausedTime; + startupSamplingDecision = null; } transactionOptions.setStartTimestamp(ttidStartTime); + transactionOptions.setStartupTransaction(startupSamplingDecision != null); // we can only bind to the scope if there's no running transaction ITransaction transaction = hub.startTransaction( - new TransactionContext(activityName, TransactionNameSource.COMPONENT, UI_LOAD_OP), + new TransactionContext( + activityName, + TransactionNameSource.COMPONENT, + UI_LOAD_OP, + startupSamplingDecision), transactionOptions); setSpanOrigin(transaction); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 9d4fd8900f..8f672c5f2c 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -8,6 +8,7 @@ import io.sentry.DeduplicateMultithreadedEventProcessor; import io.sentry.DefaultTransactionPerformanceCollector; import io.sentry.ILogger; +import io.sentry.ITransactionProfiler; import io.sentry.NoOpConnectionStatusProvider; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; @@ -19,6 +20,7 @@ import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider; import io.sentry.android.core.internal.util.AndroidMainThreadChecker; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; +import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; import io.sentry.cache.PersistingOptionsObserver; @@ -34,6 +36,7 @@ import java.util.ArrayList; import java.util.List; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; /** @@ -101,7 +104,7 @@ static void loadDefaultAndMetadataOptions( options.setFlushTimeoutMillis(DEFAULT_FLUSH_TIMEOUT_MS); ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider); - initializeCacheDirs(context, options); + options.setCacheDirPath(getCacheDir(context).getAbsolutePath()); readDefaultOptionValues(options, context, buildInfoProvider); } @@ -145,10 +148,25 @@ static void initializeIntegrationsAndProcessors( options.addEventProcessor(new ViewHierarchyEventProcessor(options)); options.addEventProcessor(new AnrV2EventProcessor(context, options, buildInfoProvider)); options.setTransportGate(new AndroidTransportGate(options)); - final SentryFrameMetricsCollector frameMetricsCollector = - new SentryFrameMetricsCollector(context, options, buildInfoProvider); - options.setTransactionProfiler( - new AndroidTransactionProfiler(context, options, buildInfoProvider, frameMetricsCollector)); + + // Check if the profiler was already instantiated in the startup. + // We use the Android profiler, that uses a global start/stop api, so we need to preserve the + // state of the profiler, and it's only possible retaining the instance. + synchronized (AppStartMetrics.getInstance()) { + final @Nullable ITransactionProfiler startupProfiler = + AppStartMetrics.getInstance().getStartupProfiler(); + if (startupProfiler != null) { + options.setTransactionProfiler(startupProfiler); + AppStartMetrics.getInstance().setStartupProfiler(null); + } else { + final SentryFrameMetricsCollector frameMetricsCollector = + new SentryFrameMetricsCollector(context, options, buildInfoProvider); + options.setTransactionProfiler( + new AndroidTransactionProfiler( + context, options, buildInfoProvider, frameMetricsCollector)); + } + } + options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger())); options.setDebugMetaLoader(new AssetsDebugMetaLoader(context, options.getLogger())); @@ -319,14 +337,11 @@ private static void readDefaultOptionValues( } /** - * Sets the cache dirs like sentry, outbox and sessions + * Retrieve the Sentry cache dir. * * @param context the Application context - * @param options the SentryAndroidOptions */ - private static void initializeCacheDirs( - final @NotNull Context context, final @NotNull SentryAndroidOptions options) { - final File cacheDir = new File(context.getCacheDir(), "sentry"); - options.setCacheDirPath(cacheDir.getAbsolutePath()); + static @NotNull File getCacheDir(final @NotNull Context context) { + return new File(context.getCacheDir(), "sentry"); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 4fb7f94e6f..59e13ef975 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -296,6 +296,11 @@ public synchronized void bindTransaction(final @NotNull ITransaction transaction endData.measurementsMap); } + @Override + public boolean isRunning() { + return transactionsCounter != 0; + } + @Override public void close() { // we stop profiling @@ -307,6 +312,10 @@ public void close() { true, null, HubAdapter.getInstance().getOptions()); + } else if (transactionsCounter != 0) { + // in case the startup profiling is running, and it's not bound to a transaction, we still + // stop profiling, but we also have to manually update the counter. + transactionsCounter--; } // we have to first stop profiling otherwise we would lost the last profile diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index 5551a54d3c..aaf152ad4e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -98,6 +98,8 @@ final class ManifestMetadataReader { static final String ENABLE_PERFORMANCE_V2 = "io.sentry.performance-v2.enable"; + static final String ENABLE_STARTUP_PROFILING = "io.sentry.profiling.enable-startup"; + /** ManifestMetadataReader ctor */ private ManifestMetadataReader() {} @@ -365,6 +367,10 @@ static void applyMetadata( options.setEnablePerformanceV2( readBool(metadata, logger, ENABLE_PERFORMANCE_V2, options.isEnablePerformanceV2())); + + options.setEnableStartupProfiling( + readBool( + metadata, logger, ENABLE_STARTUP_PROFILING, options.isEnableStartupProfiling())); } options diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 275dfa3d98..d92a4f7121 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -93,13 +93,6 @@ public final class SentryAndroidOptions extends SentryOptions { */ private boolean enableActivityLifecycleTracingAutoFinish = true; - /** - * Profiling traces rate. 101 hz means 101 traces in 1 second. Defaults to 101 to avoid possible - * lockstep sampling. More on - * https://stackoverflow.com/questions/45470758/what-is-lockstep-sampling - */ - private int profilingTracesHz = 101; - /** Interface that loads the debug images list */ private @NotNull IDebugImagesLoader debugImagesLoader = NoOpDebugImagesLoader.getInstance(); @@ -362,22 +355,6 @@ public int getProfilingTracesIntervalMillis() { @Deprecated public void setProfilingTracesIntervalMillis(final int profilingTracesIntervalMillis) {} - /** - * Returns the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. - * - * @return Rate the profiler will sample rates at. - */ - @ApiStatus.Internal - public int getProfilingTracesHz() { - return profilingTracesHz; - } - - /** Sets the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. */ - @ApiStatus.Internal - public void setProfilingTracesHz(final int profilingTracesHz) { - this.profilingTracesHz = profilingTracesHz; - } - /** * Returns the Debug image loader * diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java index 0cdc076037..89e2344ae5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java @@ -1,20 +1,39 @@ package io.sentry.android.core; +import static io.sentry.Sentry.STARTUP_PROFILING_CONFIG_FILE_NAME; + +import android.annotation.SuppressLint; import android.app.Activity; import android.app.Application; import android.content.Context; import android.content.pm.ProviderInfo; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.Process; import android.os.SystemClock; import androidx.annotation.NonNull; +import io.sentry.ILogger; +import io.sentry.ITransactionProfiler; +import io.sentry.JsonSerializer; import io.sentry.NoOpLogger; +import io.sentry.SentryExecutorService; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import io.sentry.SentryStartupProfilingOptions; +import io.sentry.TracesSamplingDecision; import io.sentry.android.core.internal.util.FirstDrawDoneListener; +import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.android.core.performance.ActivityLifecycleCallbacksAdapter; import io.sentry.android.core.performance.ActivityLifecycleTimeSpan; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import java.io.BufferedReader; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.InputStreamReader; +import java.io.Reader; import java.util.WeakHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; @@ -32,9 +51,26 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider private @Nullable Application app; private @Nullable Application.ActivityLifecycleCallbacks activityCallback; + private final @NotNull ILogger logger; + private final @NotNull BuildInfoProvider buildInfoProvider; + + @TestOnly + SentryPerformanceProvider( + final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider) { + this.logger = logger; + this.buildInfoProvider = buildInfoProvider; + } + + public SentryPerformanceProvider() { + logger = new AndroidLogger(); + buildInfoProvider = new BuildInfoProvider(logger); + } + @Override public boolean onCreate() { - onAppLaunched(getContext()); + final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); + onAppLaunched(getContext(), appStartMetrics); + launchStartupProfiler(appStartMetrics); return true; } @@ -54,8 +90,95 @@ public String getType(@NotNull Uri uri) { return null; } - private void onAppLaunched(final @Nullable Context context) { - final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance(); + @Override + public void shutdown() { + synchronized (AppStartMetrics.getInstance()) { + final @Nullable ITransactionProfiler startupProfiler = + AppStartMetrics.getInstance().getStartupProfiler(); + if (startupProfiler != null) { + startupProfiler.close(); + } + } + } + + private void launchStartupProfiler(final @NotNull AppStartMetrics appStartMetrics) { + final @Nullable Context context = getContext(); + + if (context == null) { + logger.log(SentryLevel.FATAL, "App. Context from ContentProvider is null"); + return; + } + + // Debug.startMethodTracingSampling() is only available since Lollipop + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) { + return; + } + + final @NotNull File cacheDir = AndroidOptionsInitializer.getCacheDir(context); + final @NotNull File configFile = new File(cacheDir, STARTUP_PROFILING_CONFIG_FILE_NAME); + + // No config exists: startup profiling is not enabled + if (!configFile.exists() || !configFile.canRead()) { + return; + } + + try (final @NotNull Reader reader = + new BufferedReader(new InputStreamReader(new FileInputStream(configFile)))) { + final @Nullable SentryStartupProfilingOptions profilingOptions = + new JsonSerializer(SentryOptions.empty()) + .deserialize(reader, SentryStartupProfilingOptions.class); + + if (profilingOptions == null) { + logger.log( + SentryLevel.WARNING, + "Unable to deserialize the SentryStartupProfilingOptions. Startup profiling will not start."); + return; + } + + if (!profilingOptions.isProfilingEnabled()) { + logger.log(SentryLevel.INFO, "Profiling is not enabled. Startup profiling will not start."); + return; + } + + final @NotNull TracesSamplingDecision startupSamplingDecision = + new TracesSamplingDecision( + profilingOptions.isTraceSampled(), + profilingOptions.getTraceSampleRate(), + profilingOptions.isProfileSampled(), + profilingOptions.getProfileSampleRate()); + // We store any sampling decision, so we can respect it when the first transaction starts + appStartMetrics.setStartupSamplingDecision(startupSamplingDecision); + + if (!(startupSamplingDecision.getProfileSampled() && startupSamplingDecision.getSampled())) { + logger.log(SentryLevel.DEBUG, "Startup profiling was not sampled. It will not start."); + return; + } + logger.log(SentryLevel.DEBUG, "Startup profiling started."); + + final @NotNull ITransactionProfiler startupProfiler = + new AndroidTransactionProfiler( + context.getApplicationContext(), + buildInfoProvider, + new SentryFrameMetricsCollector( + context.getApplicationContext(), logger, buildInfoProvider), + logger, + profilingOptions.getProfilingTracesDirPath(), + profilingOptions.isProfilingEnabled(), + profilingOptions.getProfilingTracesHz(), + new SentryExecutorService()); + appStartMetrics.setStartupProfiler(startupProfiler); + startupProfiler.start(); + + } catch (FileNotFoundException e) { + logger.log(SentryLevel.ERROR, "Startup profiling config file not found. ", e); + } catch (Throwable e) { + logger.log(SentryLevel.ERROR, "Error reading startup profiling config file. ", e); + } + } + + @SuppressLint("NewApi") + private void onAppLaunched( + final @Nullable Context context, final @NotNull AppStartMetrics appStartMetrics) { // sdk-init uses static field init as start time final @NotNull TimeSpan sdkInitTimeSpan = appStartMetrics.getSdkInitTimeSpan(); @@ -63,7 +186,7 @@ private void onAppLaunched(final @Nullable Context context) { // performance v2: Uses Process.getStartUptimeMillis() // requires API level 24+ - if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.N) { + if (buildInfoProvider.getSdkInfoVersion() < android.os.Build.VERSION_CODES.N) { return; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java index 41f9feecd7..007c3c62ab 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/performance/AppStartMetrics.java @@ -4,6 +4,8 @@ import android.content.ContentProvider; import android.os.SystemClock; import androidx.annotation.Nullable; +import io.sentry.ITransactionProfiler; +import io.sentry.TracesSamplingDecision; import io.sentry.android.core.ContextUtils; import io.sentry.android.core.SentryAndroidOptions; import java.util.ArrayList; @@ -13,6 +15,7 @@ import java.util.Map; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; /** * An in-memory representation for app-metrics during app start. As the SDK can't be initialized @@ -38,6 +41,8 @@ public enum AppStartType { private final @NotNull TimeSpan applicationOnCreate; private final @NotNull Map contentProviderOnCreates; private final @NotNull List activityLifecycles; + private @Nullable ITransactionProfiler startupProfiler = null; + private @Nullable TracesSamplingDecision startupSamplingDecision = null; public static @NotNull AppStartMetrics getInstance() { @@ -134,6 +139,7 @@ public void addActivityLifecycleTimeSpans(final @NotNull ActivityLifecycleTimeSp return getSdkInitTimeSpan(); } + @TestOnly public void clear() { appStartType = AppStartType.UNKNOWN; appStartSpan.reset(); @@ -141,6 +147,28 @@ public void clear() { applicationOnCreate.reset(); contentProviderOnCreates.clear(); activityLifecycles.clear(); + if (startupProfiler != null) { + startupProfiler.close(); + } + startupProfiler = null; + startupSamplingDecision = null; + } + + public @Nullable ITransactionProfiler getStartupProfiler() { + return startupProfiler; + } + + public void setStartupProfiler(final @Nullable ITransactionProfiler startupProfiler) { + this.startupProfiler = startupProfiler; + } + + public void setStartupSamplingDecision( + final @Nullable TracesSamplingDecision startupSamplingDecision) { + this.startupSamplingDecision = startupSamplingDecision; + } + + public @Nullable TracesSamplingDecision getStartupSamplingDecision() { + return startupSamplingDecision; } /** diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 81979027eb..7e1410be5a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -682,6 +682,59 @@ class ActivityLifecycleIntegrationTest { ) } + @Test + fun `When firstActivityCreated is true and startup sampling decision is set, start transaction with isStartup true`() { + AppStartMetrics.getInstance().startupSamplingDecision = mock() + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val date = SentryNanotimeDate(Date(1), 0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction( + any(), + check { + assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp()) + assertTrue(it.isStartupTransaction) + } + ) + } + + @Test + fun `When firstActivityCreated is true and startup sampling decision is not set, start transaction with isStartup false`() { + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val date = SentryNanotimeDate(Date(1), 0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction( + any(), + check { + assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp()) + assertFalse(it.isStartupTransaction) + } + ) + } + + @Test + fun `When firstActivityCreated is false and startup sampling decision is set, start transaction with isStartup false`() { + AppStartMetrics.getInstance().startupSamplingDecision = mock() + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction(any(), check { assertFalse(it.isStartupTransaction) }) + } + @Test fun `When firstActivityCreated is true, do not create app start span if not foregroundImportance`() { val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_BACKGROUND) @@ -697,7 +750,10 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) // call only once - verify(fixture.hub).startTransaction(any(), check { assertNotEquals(date, it.startTimestamp) }) + verify(fixture.hub).startTransaction( + any(), + check { assertNotEquals(date, it.startTimestamp) } + ) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 2f34b2a066..dd79cfe919 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -247,6 +247,12 @@ class AndroidOptionsInitializerTest { ) } + @Test + fun `getCacheDir returns sentry subfolder`() { + fixture.initSut() + assertTrue(AndroidOptionsInitializer.getCacheDir(fixture.context).path.endsWith("${File.separator}cache${File.separator}sentry")) + } + @Test fun `profilingTracesDirPath should be set at initialization`() { fixture.initSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 57410d49d0..405aa6dc98 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -170,6 +170,18 @@ class AndroidTransactionProfilerTest { assertEquals(1, profiler.transactionsCounter) } + @Test + fun `isStarted reflects inner counter`() { + val profiler = fixture.getSut(context) + profiler.start() + profiler.bindTransaction(fixture.transaction1) + assertEquals(1, profiler.transactionsCounter) + assertTrue(profiler.isRunning) + profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) + assertEquals(0, profiler.transactionsCounter) + assertFalse(profiler.isRunning) + } + @Test fun `profiler multiple starts are ignored`() { val profiler = fixture.getSut(context) @@ -491,4 +503,24 @@ class AndroidTransactionProfilerTest { val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) assertNull(profilingTraceData) } + + @Test + fun `profiler stops profiling on close, even if not bound to a transaction`() { + val profiler = fixture.getSut(context) + profiler.start() + assertEquals(1, profiler.transactionsCounter) + + profiler.close() + assertEquals(0, profiler.transactionsCounter) + + // The timeout scheduled job should be cleared + val androidProfiler = profiler.getProperty("profiler") + val scheduledJob = androidProfiler?.getProperty?>("scheduledFinish") + assertNull(scheduledJob) + + // Binding and calling transaction finish returns null, as the profiler was stopped + profiler.bindTransaction(fixture.transaction1) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) + assertNull(profilingTraceData) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index acc62be070..beb273e8fc 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -1343,4 +1343,31 @@ class ManifestMetadataReaderTest { // Assert assertFalse(fixture.options.isEnablePerformanceV2) } + + @Test + fun `applyMetadata reads startupProfiling flag to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.ENABLE_STARTUP_PROFILING to true) + val context = fixture.getContext(metaData = bundle) + fixture.options.profilesSampleRate = 1.0 + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertTrue(fixture.options.isEnableStartupProfiling) + } + + @Test + fun `applyMetadata reads startupProfiling flag to options and keeps default if not found`() { + // Arrange + val context = fixture.getContext() + fixture.options.profilesSampleRate = 1.0 + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertFalse(fixture.options.isEnableStartupProfiling) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt index 5ad892e454..dcfd36c41b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryPerformanceProviderTest.kt @@ -1,21 +1,37 @@ package io.sentry.android.core import android.app.Application -import android.content.Context import android.content.pm.ProviderInfo import android.os.Build import android.os.Bundle import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.ILogger +import io.sentry.JsonSerializer +import io.sentry.Sentry +import io.sentry.SentryLevel +import io.sentry.SentryOptions +import io.sentry.SentryStartupProfilingOptions import io.sentry.android.core.performance.AppStartMetrics import io.sentry.android.core.performance.AppStartMetrics.AppStartType import org.junit.runner.RunWith import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import org.robolectric.annotation.Config +import java.io.File +import java.io.FileWriter +import java.nio.file.Files +import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @@ -25,24 +41,71 @@ import kotlin.test.assertTrue ) class SentryPerformanceProviderTest { + private lateinit var cache: File + private lateinit var sentryCache: File + private lateinit var traceDir: File + + private inner class Fixture { + val mockContext = mock() + val providerInfo = ProviderInfo() + val logger = mock() + lateinit var configFile: File + + fun getSut(sdkVersion: Int = Build.VERSION_CODES.S, authority: String = AUTHORITY, handleFile: ((config: File) -> Unit)? = null): SentryPerformanceProvider { + val buildInfoProvider: BuildInfoProvider = mock() + whenever(buildInfoProvider.sdkInfoVersion).thenReturn(sdkVersion) + whenever(mockContext.cacheDir).thenReturn(cache) + whenever(mockContext.applicationContext).thenReturn(mockContext) + configFile = File(sentryCache, Sentry.STARTUP_PROFILING_CONFIG_FILE_NAME) + handleFile?.invoke(configFile) + + providerInfo.authority = authority + return SentryPerformanceProvider(logger, buildInfoProvider).apply { + attachInfo(mockContext, providerInfo) + } + } + } + + private val fixture = Fixture() + @BeforeTest fun `set up`() { AppStartMetrics.getInstance().clear() SentryShadowProcess.setStartUptimeMillis(1234) + cache = Files.createTempDirectory("sentry-disk-cache-test").toAbsolutePath().toFile() + sentryCache = File(cache, "sentry") + traceDir = File(sentryCache, "traces") + cache.mkdir() + sentryCache.mkdir() + traceDir.mkdir() + } + + @AfterTest + fun cleanup() { + AppStartMetrics.getInstance().clear() + cache.deleteRecursively() + Sentry.close() + } + + @Test + fun `when missing applicationId, SentryPerformanceProvider throws`() { + assertFailsWith { + fixture.getSut(authority = SentryPerformanceProvider::class.java.name) + } } @Test fun `provider starts appStartTimeSpan`() { assertTrue(AppStartMetrics.getInstance().sdkInitTimeSpan.hasNotStarted()) assertTrue(AppStartMetrics.getInstance().appStartTimeSpan.hasNotStarted()) - setupProvider() + fixture.getSut() assertTrue(AppStartMetrics.getInstance().sdkInitTimeSpan.hasStarted()) assertTrue(AppStartMetrics.getInstance().appStartTimeSpan.hasStarted()) } @Test fun `provider sets cold start based on first activity`() { - val provider = setupProvider() + val provider = fixture.getSut() // up until this point app start is not known assertEquals(AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) @@ -55,7 +118,7 @@ class SentryPerformanceProviderTest { @Test fun `provider sets warm start based on first activity`() { - val provider = setupProvider() + val provider = fixture.getSut() // up until this point app start is not known assertEquals(AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType) @@ -69,7 +132,7 @@ class SentryPerformanceProviderTest { @Test fun `provider keeps startup state even if multiple activities are launched`() { - val provider = setupProvider() + val provider = fixture.getSut() // when there's a saved state provider.activityCallback!!.onActivityCreated(mock(), Bundle()) @@ -86,7 +149,7 @@ class SentryPerformanceProviderTest { @Test fun `provider sets both appstart and sdk init start + end times`() { - val provider = setupProvider() + val provider = fixture.getSut() provider.onAppStartDone() val metrics = AppStartMetrics.getInstance() @@ -99,23 +162,141 @@ class SentryPerformanceProviderTest { @Test fun `provider properly registers and unregisters ActivityLifecycleCallbacks`() { - val context = mock() - val provider = setupProvider(context) + val provider = fixture.getSut() - verify(context).registerActivityLifecycleCallbacks(any()) + verify(fixture.mockContext).registerActivityLifecycleCallbacks(any()) provider.onAppStartDone() - verify(context).unregisterActivityLifecycleCallbacks(any()) + verify(fixture.mockContext).unregisterActivityLifecycleCallbacks(any()) } - private fun setupProvider(context: Context = mock()): SentryPerformanceProvider { - val providerInfo = ProviderInfo() - providerInfo.authority = AUTHORITY + //region startup profiling + @Test + fun `when config file does not exists, nothing happens`() { + fixture.getSut() + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when config file is not readable, nothing happens`() { + fixture.getSut { config -> + writeConfig(config) + config.setReadable(false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when SDK is lower than 21, nothing happens`() { + fixture.getSut(sdkVersion = Build.VERSION_CODES.KITKAT) { config -> + writeConfig(config) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when config file is empty, profiler is not started`() { + fixture.getSut { config -> + config.createNewFile() + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger).log( + eq(SentryLevel.WARNING), + eq("Unable to deserialize the SentryStartupProfilingOptions. Startup profiling will not start.") + ) + } + + @Test + fun `when profiling is disabled, profiler is not started`() { + fixture.getSut { config -> + writeConfig(config, profilingEnabled = false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger).log( + eq(SentryLevel.INFO), + eq("Profiling is not enabled. Startup profiling will not start.") + ) + } + + @Test + fun `when trace is not sampled, profiler is not started and sample decision is stored`() { + fixture.getSut { config -> + writeConfig(config, traceSampled = false, profileSampled = true) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + // If trace is not sampled, profile is not sample, either + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Startup profiling was not sampled. It will not start.") + ) + } + + @Test + fun `when profile is not sampled, profiler is not started and sample decision is stored`() { + fixture.getSut { config -> + writeConfig(config, traceSampled = true, profileSampled = false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Startup profiling was not sampled. It will not start.") + ) + } + + @Test + fun `when profiler starts, it is set in AppStartMetrics`() { + fixture.getSut { config -> + writeConfig(config) + } + assertNotNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertTrue(AppStartMetrics.getInstance().startupProfiler!!.isRunning) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Startup profiling started.") + ) + } + + @Test + fun `when provider is closed, profiler is stopped`() { + val provider = fixture.getSut { config -> + writeConfig(config) + } + provider.shutdown() + assertNotNull(AppStartMetrics.getInstance().startupProfiler) + assertFalse(AppStartMetrics.getInstance().startupProfiler!!.isRunning) + } - // calls onCreate under the hood - val provider = SentryPerformanceProvider() - provider.attachInfo(context, providerInfo) - return provider + private fun writeConfig( + configFile: File, + profilingEnabled: Boolean = true, + traceSampled: Boolean = true, + traceSampleRate: Double = 1.0, + profileSampled: Boolean = true, + profileSampleRate: Double = 1.0, + profilingTracesDirPath: String = traceDir.absolutePath + ) { + val startupProfilingOptions = SentryStartupProfilingOptions() + startupProfilingOptions.isProfilingEnabled = profilingEnabled + startupProfilingOptions.isTraceSampled = traceSampled + startupProfilingOptions.traceSampleRate = traceSampleRate + startupProfilingOptions.isProfileSampled = profileSampled + startupProfilingOptions.profileSampleRate = profileSampleRate + startupProfilingOptions.profilingTracesDirPath = profilingTracesDirPath + startupProfilingOptions.profilingTracesHz = 101 + JsonSerializer(SentryOptions.empty()).serialize(startupProfilingOptions, FileWriter(configFile)) } + //endregion companion object { private const val AUTHORITY = "io.sentry.sample.SentryPerformanceProvider" diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index bec98f04fc..c66293c55d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -12,6 +12,7 @@ import org.mockito.kotlin.mock import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -43,6 +44,8 @@ class AppStartMetricsTest { metrics.addActivityLifecycleTimeSpans(ActivityLifecycleTimeSpan()) AppStartMetrics.onApplicationCreate(mock()) AppStartMetrics.onContentProviderCreate(mock()) + metrics.setStartupProfiler(mock()) + metrics.startupSamplingDecision = mock() metrics.clear() @@ -50,10 +53,11 @@ class AppStartMetricsTest { assertTrue(metrics.sdkInitTimeSpan.hasNotStarted()) assertTrue(metrics.applicationOnCreateTimeSpan.hasNotStarted()) assertEquals(AppStartMetrics.AppStartType.UNKNOWN, metrics.appStartType) - assertTrue(metrics.applicationOnCreateTimeSpan.hasNotStarted()) assertTrue(metrics.activityLifecycleTimeSpans.isEmpty()) assertTrue(metrics.contentProviderOnCreateTimeSpans.isEmpty()) + assertNull(metrics.startupProfiler) + assertNull(metrics.startupSamplingDecision) } @Test diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index ad6889e253..1df500b3f6 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -109,6 +109,8 @@ + + diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index eb3d91b37b..6516f0bfa8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -800,6 +800,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { public abstract interface class io/sentry/ITransactionProfiler { public abstract fun bindTransaction (Lio/sentry/ITransaction;)V public abstract fun close ()V + public abstract fun isRunning ()Z public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;Lio/sentry/SentryOptions;)Lio/sentry/ProfilingTraceData; public abstract fun start ()V } @@ -1307,6 +1308,7 @@ public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionPro public fun bindTransaction (Lio/sentry/ITransaction;)V public fun close ()V public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; + public fun isRunning ()Z public fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;Lio/sentry/SentryOptions;)Lio/sentry/ProfilingTraceData; public fun start ()V } @@ -1635,6 +1637,7 @@ public final class io/sentry/SendFireAndForgetOutboxSender : io/sentry/SendCache } public final class io/sentry/Sentry { + public static final field STARTUP_PROFILING_CONFIG_FILE_NAME Ljava/lang/String; public static fun addBreadcrumb (Lio/sentry/Breadcrumb;)V public static fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V public static fun addBreadcrumb (Ljava/lang/String;)V @@ -2090,6 +2093,7 @@ public class io/sentry/SentryOptions { public fun addOptionsObserver (Lio/sentry/IOptionsObserver;)V public fun addScopeObserver (Lio/sentry/IScopeObserver;)V public fun addTracingOrigin (Ljava/lang/String;)V + public static fun empty ()Lio/sentry/SentryOptions; public fun getBackpressureMonitor ()Lio/sentry/backpressure/IBackpressureMonitor; public fun getBeforeBreadcrumb ()Lio/sentry/SentryOptions$BeforeBreadcrumbCallback; public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback; @@ -2140,6 +2144,7 @@ public class io/sentry/SentryOptions { public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; public fun getProfilingTracesDirPath ()Ljava/lang/String; + public fun getProfilingTracesHz ()I public fun getProguardUuid ()Ljava/lang/String; public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I @@ -2244,6 +2249,7 @@ public class io/sentry/SentryOptions { public fun setProfilesSampleRate (Ljava/lang/Double;)V public fun setProfilesSampler (Lio/sentry/SentryOptions$ProfilesSamplerCallback;)V public fun setProfilingEnabled (Z)V + public fun setProfilingTracesHz (I)V public fun setProguardUuid (Ljava/lang/String;)V public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V @@ -2331,6 +2337,44 @@ public final class io/sentry/SentryStackTraceFactory { public fun isInApp (Ljava/lang/String;)Ljava/lang/Boolean; } +public final class io/sentry/SentryStartupProfilingOptions : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public fun ()V + public fun getProfileSampleRate ()Ljava/lang/Double; + public fun getProfilingTracesDirPath ()Ljava/lang/String; + public fun getProfilingTracesHz ()I + public fun getTraceSampleRate ()Ljava/lang/Double; + public fun getUnknown ()Ljava/util/Map; + public fun isProfileSampled ()Z + public fun isProfilingEnabled ()Z + public fun isTraceSampled ()Z + public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V + public fun setProfileSampleRate (Ljava/lang/Double;)V + public fun setProfileSampled (Z)V + public fun setProfilingEnabled (Z)V + public fun setProfilingTracesDirPath (Ljava/lang/String;)V + public fun setProfilingTracesHz (I)V + public fun setTraceSampleRate (Ljava/lang/Double;)V + public fun setTraceSampled (Z)V + public fun setUnknown (Ljava/util/Map;)V +} + +public final class io/sentry/SentryStartupProfilingOptions$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/SentryStartupProfilingOptions; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/SentryStartupProfilingOptions$JsonKeys { + public static final field IS_PROFILING_ENABLED Ljava/lang/String; + public static final field PROFILE_SAMPLED Ljava/lang/String; + public static final field PROFILE_SAMPLE_RATE Ljava/lang/String; + public static final field PROFILING_TRACES_DIR_PATH Ljava/lang/String; + public static final field PROFILING_TRACES_HZ Ljava/lang/String; + public static final field TRACE_SAMPLED Ljava/lang/String; + public static final field TRACE_SAMPLE_RATE Ljava/lang/String; + public fun ()V +} + public final class io/sentry/SentryThreadFactory { public fun (Lio/sentry/SentryStackTraceFactory;Lio/sentry/SentryOptions;)V } @@ -2740,12 +2784,14 @@ public final class io/sentry/TransactionOptions : io/sentry/SpanOptions { public fun getStartTimestamp ()Lio/sentry/SentryDate; public fun getTransactionFinishedCallback ()Lio/sentry/TransactionFinishedCallback; public fun isBindToScope ()Z + public fun isStartupTransaction ()Z public fun isWaitForChildren ()Z public fun setBindToScope (Z)V public fun setCustomSamplingContext (Lio/sentry/CustomSamplingContext;)V public fun setDeadlineTimeout (Ljava/lang/Long;)V public fun setIdleTimeout (Ljava/lang/Long;)V public fun setStartTimestamp (Lio/sentry/SentryDate;)V + public fun setStartupTransaction (Z)V public fun setTransactionFinishedCallback (Lio/sentry/TransactionFinishedCallback;)V public fun setWaitForChildren (Z)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 60857c20ef..bfb6d90da8 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -745,8 +745,14 @@ public void flush(long timeoutMillis) { // stop it if (samplingDecision.getSampled() && samplingDecision.getProfileSampled()) { final ITransactionProfiler transactionProfiler = options.getTransactionProfiler(); - transactionProfiler.start(); - transactionProfiler.bindTransaction(transaction); + // If the profiler is not running, we start and bind it here. + if (!transactionProfiler.isRunning()) { + transactionProfiler.start(); + transactionProfiler.bindTransaction(transaction); + } else if (transactionOptions.isStartupTransaction()) { + // If the profiler is running and the current transaction is the app startup, we bind it. + transactionProfiler.bindTransaction(transaction); + } } } if (transactionOptions.isBindToScope()) { diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index 2eaec510b5..8b283fe47e 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -8,6 +8,8 @@ /** Used for performing operations when a transaction is started or ended. */ @ApiStatus.Internal public interface ITransactionProfiler { + boolean isRunning(); + void start(); void bindTransaction(@NotNull ITransaction transaction); diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index 79d5832c1e..b0aa1fdc2e 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -17,6 +17,11 @@ public static NoOpTransactionProfiler getInstance() { @Override public void start() {} + @Override + public boolean isRunning() { + return false; + } + @Override public void bindTransaction(@NotNull ITransaction transaction) {} diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index a43c78a5ea..d12f8c57be 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -23,7 +23,6 @@ import java.io.BufferedWriter; import java.io.File; import java.io.FileOutputStream; -import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Writer; @@ -54,12 +53,16 @@ private Sentry() {} /** whether to use a single (global) Hub as opposed to one per thread. */ private static volatile boolean globalHubMode = GLOBAL_HUB_DEFAULT_MODE; - private static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = + @ApiStatus.Internal + public static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = "startup_profiling_config"; @SuppressWarnings("CharsetObjectCanBeUsed") private static final Charset UTF_8 = Charset.forName("UTF-8"); + /** Timestamp used to check old profiles to delete. */ + private static final long classCreationTimestamp = System.currentTimeMillis(); + /** * Returns the current (threads) hub, if none, clones the mainHub and returns it. * @@ -261,51 +264,59 @@ private static synchronized void init( private static void handleStartupProfilingConfig( final @NotNull SentryOptions options, final @NotNull ISentryExecutorService sentryExecutorService) { - sentryExecutorService.submit( - () -> { - final String cacheDirPath = options.getCacheDirPathWithoutDsn(); - if (cacheDirPath != null) { - final @NotNull File startupProfilingConfigFile = - new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); - try { - // We always delete the config file for startup profiling - FileUtils.deleteRecursively(startupProfilingConfigFile); - if (!options.isEnableStartupProfiling()) { - return; - } - if (!options.isTracingEnabled()) { + try { + sentryExecutorService.submit( + () -> { + final String cacheDirPath = options.getCacheDirPathWithoutDsn(); + if (cacheDirPath != null) { + final @NotNull File startupProfilingConfigFile = + new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); + try { + // We always delete the config file for startup profiling + FileUtils.deleteRecursively(startupProfilingConfigFile); + if (!options.isEnableStartupProfiling()) { + return; + } + if (!options.isTracingEnabled()) { + options + .getLogger() + .log( + SentryLevel.INFO, + "Tracing is disabled and startup profiling will not start."); + return; + } + if (startupProfilingConfigFile.createNewFile()) { + final @NotNull TracesSamplingDecision startupSamplingDecision = + sampleStartupProfiling(options); + final @NotNull SentryStartupProfilingOptions startupProfilingOptions = + new SentryStartupProfilingOptions(options, startupSamplingDecision); + try (final OutputStream outputStream = + new FileOutputStream(startupProfilingConfigFile); + final Writer writer = + new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) { + options.getSerializer().serialize(startupProfilingOptions, writer); + } + } + } catch (Throwable e) { options .getLogger() - .log( - SentryLevel.INFO, - "Tracing is disabled and startup profiling will not start."); - return; + .log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); } - - if (startupProfilingConfigFile.createNewFile()) { - final @NotNull TracesSamplingDecision startupSamplingDecision = - sampleStartupProfiling(options); - final @NotNull SentryStartupProfilingOptions startupProfilingOptions = - new SentryStartupProfilingOptions(options, startupSamplingDecision); - try (final OutputStream outputStream = - new FileOutputStream(startupProfilingConfigFile); - final Writer writer = - new BufferedWriter(new OutputStreamWriter(outputStream, UTF_8))) { - options.getSerializer().serialize(startupProfilingOptions, writer); - } - } - } catch (IOException e) { - options - .getLogger() - .log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); } - } - }); + }); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Startup profiling config will not be changed. Did you call Sentry.close()?", + e); + } } private static @NotNull TracesSamplingDecision sampleStartupProfiling( final @NotNull SentryOptions options) { - TransactionContext startupTransactionContext = new TransactionContext("ui.load", ""); + TransactionContext startupTransactionContext = new TransactionContext("app.launch", "profile"); startupTransactionContext.setForNextStartup(true); SamplingContext startupSamplingContext = new SamplingContext(startupTransactionContext, null); return new TracesSampler(options).sample(startupSamplingContext); @@ -404,7 +415,6 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) final File profilingTracesDir = new File(profilingTracesDirPath); profilingTracesDir.mkdirs(); - final long timestamp = System.currentTimeMillis(); try { options @@ -416,7 +426,7 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) // Method trace files are normally deleted at the end of traces, but if that fails // for some reason we try to clear any old files here. for (File f : oldTracesDirContent) { - if (f.lastModified() < timestamp) { + if (f.lastModified() < classCreationTimestamp) { FileUtils.deleteRecursively(f); } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 3486d7f9ef..487f2e7e0e 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -450,6 +450,13 @@ public class SentryOptions { /** Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. */ private boolean enableStartupProfiling = false; + /** + * Profiling traces rate. 101 hz means 101 traces in 1 second. Defaults to 101 to avoid possible + * lockstep sampling. More on + * https://stackoverflow.com/questions/45470758/what-is-lockstep-sampling + */ + private int profilingTracesHz = 101; + /** * Adds an event processor * @@ -2251,6 +2258,22 @@ public void setEnableBackpressureHandling(final boolean enableBackpressureHandli this.enableBackpressureHandling = enableBackpressureHandling; } + /** + * Returns the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. + * + * @return Rate the profiler will sample rates at. + */ + @ApiStatus.Internal + public int getProfilingTracesHz() { + return profilingTracesHz; + } + + /** Sets the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. */ + @ApiStatus.Internal + public void setProfilingTracesHz(final int profilingTracesHz) { + this.profilingTracesHz = profilingTracesHz; + } + @ApiStatus.Experimental public boolean isEnableBackpressureHandling() { return enableBackpressureHandling; @@ -2334,7 +2357,8 @@ public interface ProfilesSamplerCallback { * * @return SentryOptions */ - static @NotNull SentryOptions empty() { + @ApiStatus.Internal + public static @NotNull SentryOptions empty() { return new SentryOptions(true); } diff --git a/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java index c630e80d4f..08e405c500 100644 --- a/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java @@ -4,10 +4,13 @@ import java.io.IOException; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; -final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { +@ApiStatus.Internal +public final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { boolean profileSampled; @Nullable Double profileSampleRate; @@ -15,16 +18,19 @@ final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializab @Nullable Double traceSampleRate; @Nullable String profilingTracesDirPath; boolean isProfilingEnabled; + int profilingTracesHz; private @Nullable Map unknown; - SentryStartupProfilingOptions() { + @VisibleForTesting + public SentryStartupProfilingOptions() { traceSampled = false; traceSampleRate = null; profileSampled = false; profileSampleRate = null; profilingTracesDirPath = null; isProfilingEnabled = false; + profilingTracesHz = 0; } SentryStartupProfilingOptions( @@ -36,6 +42,63 @@ final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializab profileSampleRate = samplingDecision.getProfileSampleRate(); profilingTracesDirPath = options.getProfilingTracesDirPath(); isProfilingEnabled = options.isProfilingEnabled(); + profilingTracesHz = options.getProfilingTracesHz(); + } + + public void setProfileSampled(final boolean profileSampled) { + this.profileSampled = profileSampled; + } + + public boolean isProfileSampled() { + return profileSampled; + } + + public void setProfileSampleRate(final @Nullable Double profileSampleRate) { + this.profileSampleRate = profileSampleRate; + } + + public @Nullable Double getProfileSampleRate() { + return profileSampleRate; + } + + public void setTraceSampled(final boolean traceSampled) { + this.traceSampled = traceSampled; + } + + public boolean isTraceSampled() { + return traceSampled; + } + + public void setTraceSampleRate(final @Nullable Double traceSampleRate) { + this.traceSampleRate = traceSampleRate; + } + + public @Nullable Double getTraceSampleRate() { + return traceSampleRate; + } + + public void setProfilingTracesDirPath(final @Nullable String profilingTracesDirPath) { + this.profilingTracesDirPath = profilingTracesDirPath; + } + + public @Nullable String getProfilingTracesDirPath() { + return profilingTracesDirPath; + } + + public void setProfilingEnabled(final boolean profilingEnabled) { + isProfilingEnabled = profilingEnabled; + } + + public boolean isProfilingEnabled() { + return isProfilingEnabled; + } + + public void setProfilingTracesHz(final int profilingTracesHz) { + this.profilingTracesHz = profilingTracesHz; + } + + public int getProfilingTracesHz() { + return profilingTracesHz; } // JsonSerializable @@ -47,6 +110,7 @@ public static final class JsonKeys { public static final String TRACE_SAMPLE_RATE = "trace_sample_rate"; public static final String PROFILING_TRACES_DIR_PATH = "profiling_traces_dir_path"; public static final String IS_PROFILING_ENABLED = "is_profiling_enabled"; + public static final String PROFILING_TRACES_HZ = "profiling_traces_hz"; } @Override @@ -59,6 +123,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger writer.name(JsonKeys.TRACE_SAMPLE_RATE).value(logger, traceSampleRate); writer.name(JsonKeys.PROFILING_TRACES_DIR_PATH).value(logger, profilingTracesDirPath); writer.name(JsonKeys.IS_PROFILING_ENABLED).value(logger, isProfilingEnabled); + writer.name(JsonKeys.PROFILING_TRACES_HZ).value(logger, profilingTracesHz); if (unknown != null) { for (String key : unknown.keySet()) { @@ -130,6 +195,12 @@ public static final class Deserializer options.isProfilingEnabled = isProfilingEnabled; } break; + case JsonKeys.PROFILING_TRACES_HZ: + Integer profilingTracesHz = reader.nextIntegerOrNull(); + if (profilingTracesHz != null) { + options.profilingTracesHz = profilingTracesHz; + } + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/main/java/io/sentry/TransactionOptions.java b/sentry/src/main/java/io/sentry/TransactionOptions.java index 0ae4b94ace..9cff444913 100644 --- a/sentry/src/main/java/io/sentry/TransactionOptions.java +++ b/sentry/src/main/java/io/sentry/TransactionOptions.java @@ -20,6 +20,9 @@ public final class TransactionOptions extends SpanOptions { /** The start timestamp of the transaction */ private @Nullable SentryDate startTimestamp = null; + /** Defines if transaction refers to the startup process */ + private boolean isStartupTransaction = false; + /** * When `waitForChildren` is set to `true`, tracer will finish only when both conditions are met * (the order of meeting condition does not matter): - tracer itself is finished - all child spans @@ -183,4 +186,14 @@ public void setTransactionFinishedCallback( @Nullable TransactionFinishedCallback transactionFinishedCallback) { this.transactionFinishedCallback = transactionFinishedCallback; } + + @ApiStatus.Internal + public void setStartupTransaction(final boolean startupTransaction) { + isStartupTransaction = startupTransaction; + } + + @ApiStatus.Internal + public boolean isStartupTransaction() { + return isStartupTransaction; + } } diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 067274ec9a..4195e5b42b 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1503,6 +1503,48 @@ class HubTest { transaction.finish() verify(mockClient, never()).captureEnvelope(any()) } + + @Test + fun `when profiler is running and isStartupTransaction is false, startTransaction does not interact with profiler`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(true) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + hub.startTransaction(context, TransactionOptions().apply { isStartupTransaction = false }) + verify(mockTransactionProfiler, never()).start() + verify(mockTransactionProfiler, never()).bindTransaction(any()) + } + + @Test + fun `when profiler is running and isStartupTransaction is true, startTransaction binds current profile`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(true) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + val transaction = hub.startTransaction(context, TransactionOptions().apply { isStartupTransaction = true }) + verify(mockTransactionProfiler, never()).start() + verify(mockTransactionProfiler).bindTransaction(eq(transaction)) + } + + @Test + fun `when profiler is not running, startTransaction starts and binds current profile`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(false) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + val transaction = hub.startTransaction(context, TransactionOptions().apply { isStartupTransaction = false }) + verify(mockTransactionProfiler).start() + verify(mockTransactionProfiler).bindTransaction(eq(transaction)) + } //endregion //region startTransaction tests diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 2af7250d83..8ff50e94d8 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -978,7 +978,7 @@ class JsonSerializerTest { val actual = serializeToString(startupProfilingOptions) val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\":false," + - "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false,\"profiling_traces_hz\":65}" assertEquals(expected, actual) } @@ -986,7 +986,7 @@ class JsonSerializerTest { @Test fun `deserializing SentryStartupProfilingOptions`() { val jsonStartupProfilingOptions = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\"" + - ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false,\"profiling_traces_hz\":65}" val actual = fixture.serializer.deserialize(StringReader(jsonStartupProfilingOptions), SentryStartupProfilingOptions::class.java) assertNotNull(actual) @@ -995,6 +995,8 @@ class JsonSerializerTest { assertEquals(startupProfilingOptions.profileSampled, actual.profileSampled) assertEquals(startupProfilingOptions.profileSampleRate, actual.profileSampleRate) assertEquals(startupProfilingOptions.isProfilingEnabled, actual.isProfilingEnabled) + assertEquals(startupProfilingOptions.profilingTracesHz, actual.profilingTracesHz) + assertEquals(startupProfilingOptions.profilingTracesDirPath, actual.profilingTracesDirPath) assertNull(actual.unknown) } @@ -1280,6 +1282,7 @@ class JsonSerializerTest { profileSampled = true profileSampleRate = 0.8 isProfilingEnabled = false + profilingTracesHz = 65 } private fun createSpan(): ISpan { diff --git a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt index 186ee39f2a..372ccc747d 100644 --- a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt @@ -2,6 +2,7 @@ package io.sentry import org.mockito.kotlin.mock import kotlin.test.Test +import kotlin.test.assertFalse import kotlin.test.assertNull class NoOpTransactionProfilerTest { @@ -15,6 +16,11 @@ class NoOpTransactionProfilerTest { fun `bindTransaction does not throw`() = profiler.bindTransaction(mock()) + @Test + fun `isRunning returns false`() { + assertFalse(profiler.isRunning) + } + @Test fun `onTransactionFinish does returns null`() { assertNull(profiler.onTransactionFinish(NoOpTransaction.getInstance(), null, mock())) diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index dd02b29dea..634e713306 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -574,4 +574,16 @@ class SentryOptionsTest { options.profilesSampleRate = 0.0 assertFalse(options.isEnableStartupProfiling) } + + @Test + fun `when options are initialized, profilingTracesHz is set to 101 by default`() { + assertEquals(101, SentryOptions().profilingTracesHz) + } + + @Test + fun `when setProfilingTracesHz is called, overrides default`() { + val options = SentryOptions() + options.profilingTracesHz = 13 + assertEquals(13, options.profilingTracesHz) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 7fa8fbafb7..306406408c 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -310,7 +310,7 @@ class SentryTest { oldProfile.createNewFile() newProfile.createNewFile() // Make the old profile look like it's created earlier - oldProfile.setLastModified(System.currentTimeMillis() - 10000) + oldProfile.setLastModified(10000) // Make the new profile look like it's created later newProfile.setLastModified(System.currentTimeMillis() + 10000) @@ -945,11 +945,11 @@ class SentryTest { @Test fun `backpressure monitor is a NoOp if handling is disabled`() { var sentryOptions: SentryOptions? = null - Sentry.init({ + Sentry.init { it.dsn = dsn it.isEnableBackpressureHandling = false sentryOptions = it - }) + } assertIs(sentryOptions?.backpressureMonitor) } @@ -957,11 +957,11 @@ class SentryTest { fun `backpressure monitor is set if handling is enabled`() { var sentryOptions: SentryOptions? = null - Sentry.init({ + Sentry.init { it.dsn = dsn it.isEnableBackpressureHandling = true sentryOptions = it - }) + } assertIs(sentryOptions?.backpressureMonitor) } @@ -982,11 +982,15 @@ class SentryTest { // Samplers are called with isForNextStartup flag set to true verify(mockSampleTracer).sample( check { + assertEquals("app.launch", it.transactionContext.name) + assertEquals("profile", it.transactionContext.operation) assertTrue(it.transactionContext.isForNextStartup) } ) verify(mockProfilesSampler).sample( check { + assertEquals("app.launch", it.transactionContext.name) + assertEquals("profile", it.transactionContext.operation) assertTrue(it.transactionContext.isForNextStartup) } ) From 4aa19d247cf7aad56869f31cae6ffcb6681fdf47 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 12 Jan 2024 17:12:47 +0100 Subject: [PATCH 6/6] updated changelog --- CHANGELOG.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da767f1fc0..c8e4cc214f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,13 @@ ### Features - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096)) -- Startup profiling 1 - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101)) -- Startup profiling 2 - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121)) -- Startup Profiling 3 - Add ContentProvider and start profile ([#3128](https://github.com/getsentry/sentry-java/pull/3128)) +- Added Startup profiling + - This depends on the new option `io.sentry.profiling.enable-startup`, other than the already existing `io.sentry.traces.profiling.sample-rate`. + - Sampler functions can check the new `isForNextStartup` flag, to adjust startup profiling sampling programmatically. + Relevant PRs: + - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101)) + - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121)) + - Add ContentProvider and start profile ([#3128](https://github.com/getsentry/sentry-java/pull/3128)) ### Fixes