From ed69adcb8b40c6220ec92063667bee5162427036 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 21 Dec 2023 17:08:50 +0100 Subject: [PATCH 1/4] 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 448e85a1156..af6bca2e71e 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 0885b9916a4..ab34b0261a6 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 3fcc9fa88c2..3254a2df94a 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 6f5a0ead33b..25e4078fae2 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/4] 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 e78d63f740f..5fe8498db36 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 fbdc76d65ee..71ea44fae8d 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 368bfc8b572..c391bf2ce08 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 af6bca2e71e..36590a8f8fd 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 00000000000..c630e80d4ff --- /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 ab34b0261a6..087f8ec01ad 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 93fb64c5b0c..2af7250d835 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 3254a2df94a..319711c9014 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 026301ee440..0f98f684584 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/4] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfb876853d8..4131f014bd0 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/4] 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 dfd51c9db7d..a43c78a5ea3 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) {