From 50505efcf17872c568634258098b6aa2ec7b9d94 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 13 Jan 2025 19:49:28 +0100 Subject: [PATCH 01/15] Set continuousProfilesSampleRate and startProfiler() and stopProfiler() as experimental --- sentry/src/main/java/io/sentry/Sentry.java | 4 +++- sentry/src/main/java/io/sentry/SentryOptions.java | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 383ba50e841..85ea2bc2247 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -1051,11 +1051,13 @@ public static void endSession() { } /** Starts the continuous profiler, if enabled. */ + @ApiStatus.Experimental public static void startProfiler() { getCurrentScopes().startProfiler(); } - /** Starts the continuous profiler, if enabled. */ + /** Stops the continuous profiler, if enabled. */ + @ApiStatus.Experimental public static void stopProfiler() { getCurrentScopes().stopProfiler(); } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 8418d4cea43..ddcf8b12b05 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1713,6 +1713,7 @@ public void setTransactionProfiler(final @Nullable ITransactionProfiler transact * * @return the continuous profiler. */ + @ApiStatus.Experimental public @NotNull IContinuousProfiler getContinuousProfiler() { return continuousProfiler; } @@ -1722,6 +1723,7 @@ public void setTransactionProfiler(final @Nullable ITransactionProfiler transact * * @param continuousProfiler - the continuous profiler */ + @ApiStatus.Experimental public void setContinuousProfiler(final @Nullable IContinuousProfiler continuousProfiler) { // We allow to set the profiler only if it was not set before, and we don't allow to unset it. if (this.continuousProfiler == NoOpContinuousProfiler.getInstance() @@ -1803,10 +1805,12 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { * * @return the sample rate */ + @ApiStatus.Experimental public double getContinuousProfilesSampleRate() { return continuousProfilesSampleRate; } + @ApiStatus.Experimental public void setContinuousProfilesSampleRate(final double continuousProfilesSampleRate) { if (!SampleRateUtils.isValidContinuousProfilesSampleRate(continuousProfilesSampleRate)) { throw new IllegalArgumentException( From def8f0c402b7084b664cd1428819e039eaba6752 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 20 Jan 2025 12:45:22 +0100 Subject: [PATCH 02/15] Added chunk start timestamp to ProfileChunk --- .../core/AndroidContinuousProfiler.java | 13 +++++++++-- sentry/api/sentry.api | 6 +++-- .../src/main/java/io/sentry/ProfileChunk.java | 23 +++++++++++++++++-- .../test/java/io/sentry/JsonSerializerTest.kt | 5 +++- .../test/java/io/sentry/SentryClientTest.kt | 2 +- 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index 99c042e015b..574adf8cef4 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -16,7 +16,9 @@ import io.sentry.PerformanceCollectionData; import io.sentry.ProfileChunk; import io.sentry.Sentry; +import io.sentry.SentryDate; import io.sentry.SentryLevel; +import io.sentry.SentryNanotimeDate; import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.protocol.SentryId; @@ -52,6 +54,7 @@ public class AndroidContinuousProfiler private @NotNull SentryId profilerId = SentryId.EMPTY_ID; private @NotNull SentryId chunkId = SentryId.EMPTY_ID; private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false); + private @NotNull SentryDate startProfileChunkTimestamp = new SentryNanotimeDate(); public AndroidContinuousProfiler( final @NotNull BuildInfoProvider buildInfoProvider, @@ -138,8 +141,10 @@ public synchronized void start() { stop(); return; } + startProfileChunkTimestamp = scopes.getOptions().getDateProvider().now(); + } else { + startProfileChunkTimestamp = new SentryNanotimeDate(); } - final AndroidProfiler.ProfileStartData startData = profiler.start(); // check if profiling started if (startData == null) { @@ -213,7 +218,11 @@ private synchronized void stop(final boolean restartProfiler) { synchronized (payloadBuilders) { payloadBuilders.add( new ProfileChunk.Builder( - profilerId, chunkId, endData.measurementsMap, endData.traceFile)); + profilerId, + chunkId, + endData.measurementsMap, + endData.traceFile, + startProfileChunkTimestamp)); } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index a097588a7c4..e8ea8ec7c0b 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1846,7 +1846,7 @@ public final class io/sentry/PerformanceCollectionData { public final class io/sentry/ProfileChunk : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/io/File;Ljava/util/Map;Lio/sentry/SentryOptions;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/io/File;Ljava/util/Map;Ljava/lang/Double;Lio/sentry/SentryOptions;)V public fun equals (Ljava/lang/Object;)Z public fun getChunkId ()Lio/sentry/protocol/SentryId; public fun getClientSdk ()Lio/sentry/protocol/SdkVersion; @@ -1857,6 +1857,7 @@ public final class io/sentry/ProfileChunk : io/sentry/JsonSerializable, io/sentr public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun getRelease ()Ljava/lang/String; public fun getSampledProfile ()Ljava/lang/String; + public fun getTimestamp ()D public fun getTraceFile ()Ljava/io/File; public fun getUnknown ()Ljava/util/Map; public fun getVersion ()Ljava/lang/String; @@ -1868,7 +1869,7 @@ public final class io/sentry/ProfileChunk : io/sentry/JsonSerializable, io/sentr } public final class io/sentry/ProfileChunk$Builder { - public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/util/Map;Ljava/io/File;)V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/protocol/SentryId;Ljava/util/Map;Ljava/io/File;Lio/sentry/SentryDate;)V public fun build (Lio/sentry/SentryOptions;)Lio/sentry/ProfileChunk; } @@ -1888,6 +1889,7 @@ public final class io/sentry/ProfileChunk$JsonKeys { public static final field PROFILER_ID Ljava/lang/String; public static final field RELEASE Ljava/lang/String; public static final field SAMPLED_PROFILE Ljava/lang/String; + public static final field TIMESTAMP Ljava/lang/String; public static final field VERSION Ljava/lang/String; public fun ()V } diff --git a/sentry/src/main/java/io/sentry/ProfileChunk.java b/sentry/src/main/java/io/sentry/ProfileChunk.java index 725c151dbd0..89d9293f5c2 100644 --- a/sentry/src/main/java/io/sentry/ProfileChunk.java +++ b/sentry/src/main/java/io/sentry/ProfileChunk.java @@ -26,6 +26,7 @@ public final class ProfileChunk implements JsonUnknown, JsonSerializable { private @NotNull String release; private @Nullable String environment; private @NotNull String version; + private double timestamp; private final @NotNull File traceFile; @@ -40,6 +41,7 @@ public ProfileChunk() { SentryId.EMPTY_ID, new File("dummy"), new HashMap<>(), + 0.0, SentryOptions.empty()); } @@ -48,6 +50,7 @@ public ProfileChunk( final @NotNull SentryId chunkId, final @NotNull File traceFile, final @NotNull Map measurements, + final @NotNull Double timestamp, final @NotNull SentryOptions options) { this.profilerId = profilerId; this.chunkId = chunkId; @@ -59,6 +62,7 @@ public ProfileChunk( this.environment = options.getEnvironment(); this.platform = "android"; this.version = "2"; + this.timestamp = timestamp; } public @NotNull Map getMeasurements() { @@ -109,6 +113,10 @@ public void setSampledProfile(final @Nullable String sampledProfile) { return traceFile; } + public double getTimestamp() { + return timestamp; + } + public @NotNull String getVersion() { return version; } @@ -152,20 +160,23 @@ public static final class Builder { private final @NotNull SentryId chunkId; private final @NotNull Map measurements; private final @NotNull File traceFile; + private final double timestamp; public Builder( final @NotNull SentryId profilerId, final @NotNull SentryId chunkId, final @NotNull Map measurements, - final @NotNull File traceFile) { + final @NotNull File traceFile, + final @NotNull SentryDate timestamp) { this.profilerId = profilerId; this.chunkId = chunkId; this.measurements = new ConcurrentHashMap<>(measurements); this.traceFile = traceFile; + this.timestamp = DateUtils.nanosToSeconds(timestamp.nanoTimestamp()); } public ProfileChunk build(SentryOptions options) { - return new ProfileChunk(profilerId, chunkId, traceFile, measurements, options); + return new ProfileChunk(profilerId, chunkId, traceFile, measurements, timestamp, options); } } @@ -182,6 +193,7 @@ public static final class JsonKeys { public static final String ENVIRONMENT = "environment"; public static final String VERSION = "version"; public static final String SAMPLED_PROFILE = "sampled_profile"; + public static final String TIMESTAMP = "timestamp"; } @Override @@ -208,6 +220,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger if (sampledProfile != null) { writer.name(JsonKeys.SAMPLED_PROFILE).value(logger, sampledProfile); } + writer.name(JsonKeys.TIMESTAMP).value(logger, timestamp); if (unknown != null) { for (String key : unknown.keySet()) { Object value = unknown.get(key); @@ -301,6 +314,12 @@ public static final class Deserializer implements JsonDeserializer data.sampledProfile = sampledProfile; } break; + case JsonKeys.TIMESTAMP: + Double timestamp = reader.nextDoubleOrNull(); + if (timestamp != null) { + data.timestamp = timestamp; + } + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 7b8fc219b93..0b63116ee64 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -885,7 +885,7 @@ class JsonSerializerTest { fixture.options.sdkVersion = SdkVersion("test", "1.2.3") fixture.options.release = "release" fixture.options.environment = "environment" - val profileChunk = ProfileChunk(profilerId, chunkId, fixture.traceFile, HashMap(), fixture.options) + val profileChunk = ProfileChunk(profilerId, chunkId, fixture.traceFile, HashMap(), 5.3, fixture.options) val measurementNow = SentryNanotimeDate() val measurementNowSeconds = BigDecimal.valueOf(DateUtils.nanosToSeconds(measurementNow.nanoTimestamp())).setScale(6, RoundingMode.DOWN) @@ -928,6 +928,7 @@ class JsonSerializerTest { assertEquals("release", element["release"] as String) assertEquals(mapOf("name" to "test", "version" to "1.2.3"), element["client_sdk"] as Map) assertEquals("2", element["version"] as String) + assertEquals(5.3, element["timestamp"] as Double) assertEquals("sampled profile in base 64", element["sampled_profile"] as String) assertEquals( mapOf( @@ -992,6 +993,7 @@ class JsonSerializerTest { "profiler_id":"$profilerId", "release":"release", "sampled_profile":"sampled profile in base 64", + "timestamp":"5.3", "version":"2", "measurements":{ "screen_frame_rates": { @@ -1035,6 +1037,7 @@ class JsonSerializerTest { assertEquals(profilerId, profileChunk.profilerId) assertEquals("release", profileChunk.release) assertEquals("sampled profile in base 64", profileChunk.sampledProfile) + assertEquals(5.3, profileChunk.timestamp) assertEquals("2", profileChunk.version) val expectedMeasurements = mapOf( ProfileMeasurement.ID_SCREEN_FRAME_RATES to ProfileMeasurement( diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index fc54dcb1bd4..563e0725f0e 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -98,7 +98,7 @@ class SentryClientTest { whenever(scopes.options).thenReturn(sentryOptions) sentryTracer = SentryTracer(TransactionContext("a-transaction", "op", TracesSamplingDecision(true)), scopes) sentryTracer.startChild("a-span", "span 1").finish() - profileChunk = ProfileChunk(SentryId(), SentryId(), profilingTraceFile, emptyMap(), sentryOptions) + profileChunk = ProfileChunk(SentryId(), SentryId(), profilingTraceFile, emptyMap(), 1.0, sentryOptions) } var attachment = Attachment("hello".toByteArray(), "hello.txt", "text/plain", true) From 6c07e338576a9ccab8a04b657c70ba3c277bd016 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 21 Jan 2025 14:03:44 +0100 Subject: [PATCH 03/15] updated changelog --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdbdbc0d7e2..3cea9e2836d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,31 @@ # Changelog +## Unreleased + +### Features + +- Add Continuous Profiling Support ([#3710](https://github.com/getsentry/sentry-java/pull/3710)) + + To enable Continuous Profiling use the `Sentry.startProfiler` and `Sentry.stopProfiler` experimental APIs. Sampling rate can be set through `options.continuousProfilesSampleRate` (defaults to 1.0). + Note: Both `options.profilesSampler` and `options.profilesSampleRate` must not be set to enable Continuous Profiling. + + ```kotlin + import io.sentry.android.core.SentryAndroid + + SentryAndroid.init(context) { options -> + + // Currently under experimental options: + options.continuousProfilesSampleRate = 1.0 + } + // Start profiling + Sentry.startProfiler() + + // After all profiling is done, stop the profiler. Profiles can last indefinitely if not stopped. + Sentry.stopProfiler() + ``` + + To learn more visit [Sentry's Continuous Profiling](https://docs.sentry.io/product/explore/profiling/transaction-vs-continuous-profiling/#continuous-profiling-mode) documentation page. + ## 8.0.0-beta.3 ### Features From 6eeed8ef9583bc63e49abfef7fe39c272b4081c7 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 24 Jan 2025 09:49:29 +0100 Subject: [PATCH 04/15] Moved setContinuousProfilesSampleRate into ExperimentalOptions --- CHANGELOG.md | 16 +++++++++++- .../android/core/ManifestMetadataReader.java | 2 +- .../core/ManifestMetadataReaderTest.kt | 2 +- sentry/api/sentry.api | 3 ++- .../java/io/sentry/ExperimentalOptions.java | 26 +++++++++++++++++++ .../main/java/io/sentry/SentryOptions.java | 26 +++---------------- sentry/src/test/java/io/sentry/ScopesTest.kt | 2 +- .../test/java/io/sentry/SentryOptionsTest.kt | 12 ++++----- sentry/src/test/java/io/sentry/SentryTest.kt | 6 ++--- .../test/java/io/sentry/TracesSamplerTest.kt | 2 +- 10 files changed, 59 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cea9e2836d..a1d30f3de0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,13 +9,27 @@ To enable Continuous Profiling use the `Sentry.startProfiler` and `Sentry.stopProfiler` experimental APIs. Sampling rate can be set through `options.continuousProfilesSampleRate` (defaults to 1.0). Note: Both `options.profilesSampler` and `options.profilesSampleRate` must not be set to enable Continuous Profiling. + ```java + import io.sentry.android.core.SentryAndroid; + + SentryAndroid.init(context) { options -> + + // Currently under experimental options: + options.getExperimental().setContinuousProfilesSampleRate(1.0); + } + // Start profiling + Sentry.startProfiler(); + + // After all profiling is done, stop the profiler. Profiles can last indefinitely if not stopped. + Sentry.stopProfiler(); + ``` ```kotlin import io.sentry.android.core.SentryAndroid SentryAndroid.init(context) { options -> // Currently under experimental options: - options.continuousProfilesSampleRate = 1.0 + options.experimental.continuousProfilesSampleRate = 1.0 } // Start profiling Sentry.startProfiler() 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 f49291340fe..70085b23563 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 @@ -322,7 +322,7 @@ static void applyMetadata( final double continuousProfilesSampleRate = readDouble(metadata, logger, CONTINUOUS_PROFILES_SAMPLE_RATE); if (continuousProfilesSampleRate != -1) { - options.setContinuousProfilesSampleRate(continuousProfilesSampleRate); + options.getExperimental().setContinuousProfilesSampleRate(continuousProfilesSampleRate); } } 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 20900ea133b..e1f08b396e4 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 @@ -818,7 +818,7 @@ class ManifestMetadataReaderTest { fun `applyMetadata does not override continuousProfilesSampleRate from options`() { // Arrange val expectedSampleRate = 0.99f - fixture.options.continuousProfilesSampleRate = expectedSampleRate.toDouble() + fixture.options.experimental.continuousProfilesSampleRate = expectedSampleRate.toDouble() val bundle = bundleOf(ManifestMetadataReader.CONTINUOUS_PROFILES_SAMPLE_RATE to 0.1f) val context = fixture.getContext(metaData = bundle) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e8ea8ec7c0b..4d93e217f81 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -443,7 +443,9 @@ public abstract interface class io/sentry/EventProcessor { public final class io/sentry/ExperimentalOptions { public fun (Z)V + public fun getContinuousProfilesSampleRate ()D public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; + public fun setContinuousProfilesSampleRate (D)V public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V } @@ -3068,7 +3070,6 @@ public class io/sentry/SentryOptions { public fun setConnectionStatusProvider (Lio/sentry/IConnectionStatusProvider;)V public fun setConnectionTimeoutMillis (I)V public fun setContinuousProfiler (Lio/sentry/IContinuousProfiler;)V - public fun setContinuousProfilesSampleRate (D)V public fun setCron (Lio/sentry/SentryOptions$Cron;)V public fun setDateProvider (Lio/sentry/SentryDateProvider;)V public fun setDebug (Z)V diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 4a0e7de78d1..1a473d7c461 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -1,5 +1,7 @@ package io.sentry; +import io.sentry.util.SampleRateUtils; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; /** @@ -11,6 +13,15 @@ public final class ExperimentalOptions { private @NotNull SentryReplayOptions sessionReplay; + /** + * Configures the continuous profiling sample rate as a percentage of profiles to be sent in the + * range of 0.0 to 1.0. if 1.0 is set it means that 100% of profiles will be sent. If set to 0.1 + * only 10% of profiles will be sent. Profiles are picked randomly. Default is 1 (100%). + * ProfilesSampleRate takes precedence over this. To enable continuous profiling, don't set + * profilesSampleRate or profilesSampler, or set them to null. + */ + private double continuousProfilesSampleRate = 1.0; + public ExperimentalOptions(final boolean empty) { this.sessionReplay = new SentryReplayOptions(empty); } @@ -23,4 +34,19 @@ public SentryReplayOptions getSessionReplay() { public void setSessionReplay(final @NotNull SentryReplayOptions sessionReplayOptions) { this.sessionReplay = sessionReplayOptions; } + + public double getContinuousProfilesSampleRate() { + return continuousProfilesSampleRate; + } + + @ApiStatus.Experimental + public void setContinuousProfilesSampleRate(final double continuousProfilesSampleRate) { + if (!SampleRateUtils.isValidContinuousProfilesSampleRate(continuousProfilesSampleRate)) { + throw new IllegalArgumentException( + "The value " + + continuousProfilesSampleRate + + " is not valid. Use values between 0.0 and 1.0."); + } + this.continuousProfilesSampleRate = continuousProfilesSampleRate; + } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index ddcf8b12b05..ff42854431e 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -357,15 +357,6 @@ public class SentryOptions { */ private @Nullable ProfilesSamplerCallback profilesSampler; - /** - * Configures the continuous profiling sample rate as a percentage of profiles to be sent in the - * range of 0.0 to 1.0. if 1.0 is set it means that 100% of profiles will be sent. If set to 0.1 - * only 10% of profiles will be sent. Profiles are picked randomly. Default is 1 (100%). - * ProfilesSampleRate takes precedence over this. To enable continuous profiling, don't set - * profilesSampleRate or profilesSampler, or set them to null. - */ - private double continuousProfilesSampleRate = 1.0; - /** Max trace file size in bytes. */ private long maxTraceFileSize = 5 * 1024 * 1024; @@ -1751,7 +1742,7 @@ public boolean isProfilingEnabled() { public boolean isContinuousProfilingEnabled() { return profilesSampleRate == null && profilesSampler == null - && continuousProfilesSampleRate > 0; + && experimental.getContinuousProfilesSampleRate() > 0; } /** @@ -1807,18 +1798,7 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { */ @ApiStatus.Experimental public double getContinuousProfilesSampleRate() { - return continuousProfilesSampleRate; - } - - @ApiStatus.Experimental - public void setContinuousProfilesSampleRate(final double continuousProfilesSampleRate) { - if (!SampleRateUtils.isValidContinuousProfilesSampleRate(continuousProfilesSampleRate)) { - throw new IllegalArgumentException( - "The value " - + continuousProfilesSampleRate - + " is not valid. Use values between 0.0 and 1.0."); - } - this.continuousProfilesSampleRate = continuousProfilesSampleRate; + return experimental.getContinuousProfilesSampleRate(); } /** @@ -2748,7 +2728,7 @@ public void merge(final @NotNull ExternalOptions options) { setProfilesSampleRate(options.getProfilesSampleRate()); } if (options.getContinuousProfilesSampleRate() != null) { - setContinuousProfilesSampleRate(options.getContinuousProfilesSampleRate()); + experimental.setContinuousProfilesSampleRate(options.getContinuousProfilesSampleRate()); } if (options.getDebug() != null) { setDebug(options.getDebug()); diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 107d9375a4a..92343bdeb47 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -2177,7 +2177,7 @@ class ScopesTest { val logger = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) - it.continuousProfilesSampleRate = 0.1 + it.experimental.continuousProfilesSampleRate = 0.1 it.setLogger(logger) it.isDebug = true } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 03146f81800..4cea67c4eda 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -239,7 +239,7 @@ class SentryOptionsTest { @Test fun `when continuousProfilesSampleRate is set to a 0, isProfilingEnabled is false and isContinuousProfilingEnabled is false`() { val options = SentryOptions().apply { - this.continuousProfilesSampleRate = 0.0 + this.experimental.continuousProfilesSampleRate = 0.0 } assertFalse(options.isProfilingEnabled) assertFalse(options.isContinuousProfilingEnabled) @@ -266,19 +266,19 @@ class SentryOptionsTest { @Test fun `when setContinuousProfilesSampleRate is set to exactly 0, value is set`() { val options = SentryOptions().apply { - this.continuousProfilesSampleRate = 0.0 + this.experimental.continuousProfilesSampleRate = 0.0 } assertEquals(0.0, options.continuousProfilesSampleRate) } @Test fun `when setContinuousProfilesSampleRate is set to higher than 1_0, setter throws`() { - assertFailsWith { SentryOptions().continuousProfilesSampleRate = 1.0000000000001 } + assertFailsWith { SentryOptions().experimental.continuousProfilesSampleRate = 1.0000000000001 } } @Test fun `when setContinuousProfilesSampleRate is set to lower than 0, setter throws`() { - assertFailsWith { SentryOptions().continuousProfilesSampleRate = -0.0000000000001 } + assertFailsWith { SentryOptions().experimental.continuousProfilesSampleRate = -0.0000000000001 } } @Test @@ -607,7 +607,7 @@ class SentryOptionsTest { fun `when profiling is disabled, isEnableAppStartProfiling is always false`() { val options = SentryOptions() options.isEnableAppStartProfiling = true - options.continuousProfilesSampleRate = 0.0 + options.experimental.continuousProfilesSampleRate = 0.0 assertFalse(options.isEnableAppStartProfiling) } @@ -615,7 +615,7 @@ class SentryOptionsTest { fun `when setEnableAppStartProfiling is called and continuous profiling is enabled, isEnableAppStartProfiling is true`() { val options = SentryOptions() options.isEnableAppStartProfiling = true - options.continuousProfilesSampleRate = 1.0 + options.experimental.continuousProfilesSampleRate = 1.0 assertTrue(options.isEnableAppStartProfiling) } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index ae0243618a0..36dda684749 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -407,7 +407,7 @@ class SentryTest { var sentryOptions: SentryOptions? = null Sentry.init { it.dsn = dsn - it.continuousProfilesSampleRate = 1.0 + it.experimental.continuousProfilesSampleRate = 1.0 it.cacheDirPath = tempPath sentryOptions = it } @@ -422,7 +422,7 @@ class SentryTest { var sentryOptions: SentryOptions? = null Sentry.init { it.dsn = dsn - it.continuousProfilesSampleRate = 0.0 + it.experimental.continuousProfilesSampleRate = 0.0 it.cacheDirPath = tempPath sentryOptions = it } @@ -1336,7 +1336,7 @@ class SentryTest { Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) - it.continuousProfilesSampleRate = 0.1 + it.experimental.continuousProfilesSampleRate = 0.1 } // We cannot set sample rate to 0, as it would not start the profiler. So we set the seed to have consistent results SentryRandom.current().setSeed(0) diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 4523a6ecd1e..2fd047c9efa 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -35,7 +35,7 @@ class TracesSamplerTest { options.profilesSampleRate = profilesSampleRate } if (continuousProfilesSampleRate != null) { - options.continuousProfilesSampleRate = continuousProfilesSampleRate + options.experimental.continuousProfilesSampleRate = continuousProfilesSampleRate } if (tracesSamplerCallback != null) { options.tracesSampler = tracesSamplerCallback From c9fbfbb99866be81ea97f006f1cd998681018cc3 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 11 Feb 2025 16:31:19 +0100 Subject: [PATCH 05/15] increased continuous profiling chunk duration to 1 minute --- .../java/io/sentry/android/core/AndroidContinuousProfiler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index 574adf8cef4..ac43ac53c32 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -36,7 +36,7 @@ @ApiStatus.Internal public class AndroidContinuousProfiler implements IContinuousProfiler, RateLimiter.IRateLimitObserver { - private static final long MAX_CHUNK_DURATION_MILLIS = 10000; + private static final long MAX_CHUNK_DURATION_MILLIS = 60000; private final @NotNull ILogger logger; private final @Nullable String profilingTracesDirPath; From dcee57a9d24e816ff2c4f64e16e9861ae7214ae4 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 18 Feb 2025 16:45:52 +0100 Subject: [PATCH 06/15] replaced continuousProfilesSampleRate with profileSessionSampleRate (Default 0.0) sample rate is now evaluated inside AndroidContinuousProfiler and every time a session finishes --- .../api/sentry-android-core.api | 3 +- .../core/AndroidContinuousProfiler.java | 28 +++++- .../android/core/ManifestMetadataReader.java | 14 +-- .../core/SentryPerformanceProvider.java | 8 +- .../core/AndroidContinuousProfilerTest.kt | 98 +++++++++++++------ .../core/ManifestMetadataReaderTest.kt | 18 ++-- sentry/api/sentry.api | 16 +-- .../java/io/sentry/ExperimentalOptions.java | 22 ++--- .../main/java/io/sentry/ExternalOptions.java | 11 --- .../java/io/sentry/IContinuousProfiler.java | 4 +- .../io/sentry/NoOpContinuousProfiler.java | 9 +- sentry/src/main/java/io/sentry/Scope.java | 4 + sentry/src/main/java/io/sentry/Scopes.java | 9 +- .../SentryAppStartProfilingOptions.java | 2 +- .../main/java/io/sentry/SentryOptions.java | 15 ++- .../main/java/io/sentry/TracesSampler.java | 4 +- .../java/io/sentry/ExternalOptionsTest.kt | 7 -- .../io/sentry/NoOpContinuousProfilerTest.kt | 8 +- sentry/src/test/java/io/sentry/ScopeTest.kt | 52 ++++++++++ sentry/src/test/java/io/sentry/ScopesTest.kt | 34 ++----- .../test/java/io/sentry/SentryOptionsTest.kt | 30 +++--- sentry/src/test/java/io/sentry/SentryTest.kt | 25 ++--- .../test/java/io/sentry/TracesSamplerTest.kt | 24 ++--- 23 files changed, 263 insertions(+), 182 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index b1b45357969..ad1fd69cf5b 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -42,7 +42,8 @@ public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IConti public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun isRunning ()Z public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V - public fun start ()V + public fun reevaluateSampling ()V + public fun start (Lio/sentry/TracesSampler;)V public fun stop ()V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index ac43ac53c32..764d29d1f94 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -20,6 +20,7 @@ import io.sentry.SentryLevel; import io.sentry.SentryNanotimeDate; import io.sentry.SentryOptions; +import io.sentry.TracesSampler; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.protocol.SentryId; import io.sentry.transport.RateLimiter; @@ -55,6 +56,8 @@ public class AndroidContinuousProfiler private @NotNull SentryId chunkId = SentryId.EMPTY_ID; private final @NotNull AtomicBoolean isClosed = new AtomicBoolean(false); private @NotNull SentryDate startProfileChunkTimestamp = new SentryNanotimeDate(); + private boolean shouldSample = true; + private boolean isSampled = false; public AndroidContinuousProfiler( final @NotNull BuildInfoProvider buildInfoProvider, @@ -100,7 +103,24 @@ private void init() { logger); } - public synchronized void start() { + public synchronized void start(final @NotNull TracesSampler tracesSampler) { + if (shouldSample) { + isSampled = tracesSampler.sampleSessionProfile(); + shouldSample = false; + } + if (!isSampled) { + logger.log(SentryLevel.DEBUG, "Profiler was not started due to sampling decision."); + return; + } + if (isRunning()) { + logger.log(SentryLevel.DEBUG, "Profiler is already running."); + return; + } + logger.log(SentryLevel.DEBUG, "Started Profiler."); + startProfile(); + } + + private synchronized void startProfile() { if ((scopes == null || scopes == NoOpScopes.getInstance()) && Sentry.getCurrentScopes() != NoOpScopes.getInstance()) { this.scopes = Sentry.getCurrentScopes(); @@ -236,7 +256,7 @@ private synchronized void stop(final boolean restartProfiler) { if (restartProfiler) { logger.log(SentryLevel.DEBUG, "Profile chunk finished. Starting a new one."); - start(); + startProfile(); } else { // When the profiler is stopped manually, we have to reset its id profilerId = SentryId.EMPTY_ID; @@ -244,6 +264,10 @@ private synchronized void stop(final boolean restartProfiler) { } } + public synchronized void reevaluateSampling() { + shouldSample = true; + } + public synchronized void close() { stop(); isClosed.set(true); 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 70085b23563..04346f43455 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 @@ -64,8 +64,8 @@ final class ManifestMetadataReader { static final String PROFILES_SAMPLE_RATE = "io.sentry.traces.profiling.sample-rate"; - static final String CONTINUOUS_PROFILES_SAMPLE_RATE = - "io.sentry.traces.profiling.continuous-sample-rate"; + static final String PROFILE_SESSION_SAMPLE_RATE = + "io.sentry.traces.profiling.session-sample-rate"; @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets"; @@ -318,11 +318,11 @@ static void applyMetadata( } } - if (options.getContinuousProfilesSampleRate() == 1.0) { - final double continuousProfilesSampleRate = - readDouble(metadata, logger, CONTINUOUS_PROFILES_SAMPLE_RATE); - if (continuousProfilesSampleRate != -1) { - options.getExperimental().setContinuousProfilesSampleRate(continuousProfilesSampleRate); + if (options.getProfileSessionSampleRate() == 0.0) { + final double profileSessionSampleRate = + readDouble(metadata, logger, PROFILE_SESSION_SAMPLE_RATE); + if (profileSessionSampleRate != -1) { + options.getExperimental().setProfileSessionSampleRate(profileSessionSampleRate); } } 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 583f984cc8a..d46f2cc542a 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 @@ -22,6 +22,7 @@ import io.sentry.SentryExecutorService; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.TracesSampler; import io.sentry.TracesSamplingDecision; import io.sentry.android.core.internal.util.FirstDrawDoneListener; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; @@ -180,7 +181,12 @@ private void createAndStartContinuousProfiler( appStartMetrics.setAppStartProfiler(null); appStartMetrics.setAppStartContinuousProfiler(appStartContinuousProfiler); logger.log(SentryLevel.DEBUG, "App start continuous profiling started."); - appStartContinuousProfiler.start(); + SentryOptions sentryOptions = SentryOptions.empty(); + // Let's fake a sampler to accept the sampling decision that was calculated on last run + sentryOptions + .getExperimental() + .setProfileSessionSampleRate(profilingOptions.isContinuousProfileSampled() ? 1 : 0); + appStartContinuousProfiler.start(new TracesSampler(sentryOptions)); } private void createAndStartTransactionProfiler( diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt index 8093b2ee8b2..79d151fe4c5 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt @@ -17,6 +17,7 @@ import io.sentry.Sentry import io.sentry.SentryLevel import io.sentry.SentryNanotimeDate import io.sentry.SentryTracer +import io.sentry.TracesSampler import io.sentry.TransactionContext import io.sentry.android.core.internal.util.SentryFrameMetricsCollector import io.sentry.profilemeasurements.ProfileMeasurement @@ -58,6 +59,7 @@ class AndroidContinuousProfilerTest { whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP_MR1) } val mockLogger = mock() + val mockTracesSampler = mock() val scopes: IScopes = mock() val frameMetricsCollector: SentryFrameMetricsCollector = mock() @@ -73,6 +75,10 @@ class AndroidContinuousProfilerTest { setLogger(mockLogger) } + init { + whenever(mockTracesSampler.sampleSessionProfile()).thenReturn(true) + } + fun getSut(buildInfoProvider: BuildInfoProvider = buildInfo, optionConfig: ((options: SentryAndroidOptions) -> Unit) = {}): AndroidContinuousProfiler { optionConfig(options) whenever(scopes.options).thenReturn(options) @@ -136,7 +142,7 @@ class AndroidContinuousProfilerTest { @Test fun `isRunning reflects profiler status`() { val profiler = fixture.getSut() - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) profiler.stop() assertFalse(profiler.isRunning) @@ -145,21 +151,57 @@ class AndroidContinuousProfilerTest { @Test fun `profiler multiple starts are ignored`() { val profiler = fixture.getSut() - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) - verify(fixture.mockLogger, never()).log(eq(SentryLevel.WARNING), eq("Profiling has already started...")) - profiler.start() - verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("Profiling has already started...")) + verify(fixture.mockLogger, never()).log(eq(SentryLevel.DEBUG), eq("Profiler is already running.")) + profiler.start(fixture.mockTracesSampler) + verify(fixture.mockLogger).log(eq(SentryLevel.DEBUG), eq("Profiler is already running.")) assertTrue(profiler.isRunning) } + @Test + fun `profiler logs a warning on start if not sampled`() { + val profiler = fixture.getSut() + whenever(fixture.mockTracesSampler.sampleSessionProfile()).thenReturn(false) + profiler.start(fixture.mockTracesSampler) + assertFalse(profiler.isRunning) + verify(fixture.mockLogger).log(eq(SentryLevel.DEBUG), eq("Profiler was not started due to sampling decision.")) + } + + @Test + fun `profiler evaluates sessionSampleRate only the first time`() { + val profiler = fixture.getSut() + verify(fixture.mockTracesSampler, never()).sampleSessionProfile() + // The first time the profiler is started, the sessionSampleRate is evaluated + profiler.start(fixture.mockTracesSampler) + verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() + // Then, the sessionSampleRate is not evaluated again + profiler.start(fixture.mockTracesSampler) + verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() + } + + @Test + fun `when reevaluateSampling, profiler evaluates sessionSampleRate on next start`() { + val profiler = fixture.getSut() + verify(fixture.mockTracesSampler, never()).sampleSessionProfile() + // The first time the profiler is started, the sessionSampleRate is evaluated + profiler.start(fixture.mockTracesSampler) + verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() + // When reevaluateSampling is called, the sessionSampleRate is not evaluated immediately + profiler.reevaluateSampling() + verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() + // Then, when the profiler starts again, the sessionSampleRate is reevaluated + profiler.start(fixture.mockTracesSampler) + verify(fixture.mockTracesSampler, times(2)).sampleSessionProfile() + } + @Test fun `profiler works only on api 22+`() { val buildInfo = mock { whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP) } val profiler = fixture.getSut(buildInfo) - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -168,7 +210,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.profilesSampleRate = 0.0 } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) } @@ -184,8 +226,8 @@ class AndroidContinuousProfilerTest { ) // Regardless of how many times the profiler is started, the option is evaluated and logged only once - profiler.start() - profiler.start() + profiler.start(fixture.mockTracesSampler) + profiler.start(fixture.mockTracesSampler) verify(fixture.mockLogger, times(1)).log( SentryLevel.WARNING, "Disabling profiling because no profiling traces dir path is defined in options." @@ -205,8 +247,8 @@ class AndroidContinuousProfilerTest { ) // Regardless of how many times the profiler is started, the option is evaluated and logged only once - profiler.start() - profiler.start() + profiler.start(fixture.mockTracesSampler) + profiler.start(fixture.mockTracesSampler) verify(fixture.mockLogger, times(1)).log( SentryLevel.WARNING, "Disabling profiling because trace rate is set to %d", @@ -219,7 +261,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.cacheDirPath = null } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -228,7 +270,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.cacheDirPath = "" } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -237,7 +279,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.profilingTracesHz = 0 } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -248,7 +290,7 @@ class AndroidContinuousProfilerTest { it.executorService = mockExecutorService } whenever(mockExecutorService.submit(any>())).thenReturn(mock()) - profiler.start() + profiler.start(fixture.mockTracesSampler) verify(mockExecutorService, never()).submit(any()) profiler.stop() verify(mockExecutorService, never()).submit(any>()) @@ -259,7 +301,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { File(it.profilingTracesDirPath!!).setWritable(false) } - profiler.start() + profiler.start(fixture.mockTracesSampler) profiler.stop() // We assert that no trace files are written assertTrue( @@ -276,7 +318,7 @@ class AndroidContinuousProfilerTest { fixture.options.compositePerformanceCollector = performanceCollector val profiler = fixture.getSut() verify(performanceCollector, never()).start(any()) - profiler.start() + profiler.start(fixture.mockTracesSampler) verify(performanceCollector).start(any()) } @@ -285,7 +327,7 @@ class AndroidContinuousProfilerTest { val performanceCollector = mock() fixture.options.compositePerformanceCollector = performanceCollector val profiler = fixture.getSut() - profiler.start() + profiler.start(fixture.mockTracesSampler) verify(performanceCollector, never()).stop(any()) profiler.stop() verify(performanceCollector).stop(any()) @@ -296,7 +338,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut() val frameMetricsCollectorId = "id" whenever(fixture.frameMetricsCollector.startCollection(any())).thenReturn(frameMetricsCollectorId) - profiler.start() + profiler.start(fixture.mockTracesSampler) verify(fixture.frameMetricsCollector, never()).stopCollection(frameMetricsCollectorId) profiler.stop() verify(fixture.frameMetricsCollector).stopCollection(frameMetricsCollectorId) @@ -305,7 +347,7 @@ class AndroidContinuousProfilerTest { @Test fun `profiler stops profiling and clear scheduled job on close`() { val profiler = fixture.getSut() - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) profiler.close() @@ -327,7 +369,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) executorService.runAll() @@ -345,7 +387,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We run the executor service to trigger the profiler restart (chunk finish) executorService.runAll() @@ -369,7 +411,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start() + profiler.start(fixture.mockTracesSampler) profiler.stop() // We run the executor service to send the profile chunk executorService.runAll() @@ -388,7 +430,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We run the executor service to trigger the profiler restart (chunk finish) executorService.runAll() @@ -406,7 +448,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We close the profiler, which should prevent sending additional chunks @@ -426,7 +468,7 @@ class AndroidContinuousProfilerTest { val rateLimiter = mock() whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true) - profiler.start() + profiler.start(fixture.mockTracesSampler) assertTrue(profiler.isRunning) // If the SDK is rate limited, the profiler should stop @@ -447,7 +489,7 @@ class AndroidContinuousProfilerTest { whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter) // If the SDK is rate limited, the profiler should never start - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) assertEquals(SentryId.EMPTY_ID, profiler.profilerId) verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler.")) @@ -464,7 +506,7 @@ class AndroidContinuousProfilerTest { } // If the device is offline, the profiler should never start - profiler.start() + profiler.start(fixture.mockTracesSampler) assertFalse(profiler.isRunning) assertEquals(SentryId.EMPTY_ID, profiler.profilerId) verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler.")) 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 e1f08b396e4..06de99258e6 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 @@ -801,36 +801,36 @@ class ManifestMetadataReaderTest { } @Test - fun `applyMetadata reads continuousProfilesSampleRate from metadata`() { + fun `applyMetadata reads profileSessionSampleRate from metadata`() { // Arrange val expectedSampleRate = 0.99f - val bundle = bundleOf(ManifestMetadataReader.CONTINUOUS_PROFILES_SAMPLE_RATE to expectedSampleRate) + val bundle = bundleOf(ManifestMetadataReader.PROFILE_SESSION_SAMPLE_RATE to expectedSampleRate) val context = fixture.getContext(metaData = bundle) // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) // Assert - assertEquals(expectedSampleRate.toDouble(), fixture.options.continuousProfilesSampleRate) + assertEquals(expectedSampleRate.toDouble(), fixture.options.profileSessionSampleRate) } @Test - fun `applyMetadata does not override continuousProfilesSampleRate from options`() { + fun `applyMetadata does not override profileSessionSampleRate from options`() { // Arrange val expectedSampleRate = 0.99f - fixture.options.experimental.continuousProfilesSampleRate = expectedSampleRate.toDouble() - val bundle = bundleOf(ManifestMetadataReader.CONTINUOUS_PROFILES_SAMPLE_RATE to 0.1f) + fixture.options.experimental.profileSessionSampleRate = expectedSampleRate.toDouble() + val bundle = bundleOf(ManifestMetadataReader.PROFILE_SESSION_SAMPLE_RATE to 0.1f) val context = fixture.getContext(metaData = bundle) // Act ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) // Assert - assertEquals(expectedSampleRate.toDouble(), fixture.options.continuousProfilesSampleRate) + assertEquals(expectedSampleRate.toDouble(), fixture.options.profileSessionSampleRate) } @Test - fun `applyMetadata without specifying continuousProfilesSampleRate, stays 1`() { + fun `applyMetadata without specifying profileSessionSampleRate, stays 0`() { // Arrange val context = fixture.getContext() @@ -838,7 +838,7 @@ class ManifestMetadataReaderTest { ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) // Assert - assertEquals(1.0, fixture.options.continuousProfilesSampleRate) + assertEquals(0.0, fixture.options.profileSessionSampleRate) } @Test diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 4d93e217f81..ad5c7600821 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -443,9 +443,9 @@ public abstract interface class io/sentry/EventProcessor { public final class io/sentry/ExperimentalOptions { public fun (Z)V - public fun getContinuousProfilesSampleRate ()D + public fun getProfileSessionSampleRate ()D public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; - public fun setContinuousProfilesSampleRate (D)V + public fun setProfileSessionSampleRate (D)V public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V } @@ -460,7 +460,6 @@ public final class io/sentry/ExternalOptions { public static fun from (Lio/sentry/config/PropertiesProvider;Lio/sentry/ILogger;)Lio/sentry/ExternalOptions; public fun getBundleIds ()Ljava/util/Set; public fun getContextTags ()Ljava/util/List; - public fun getContinuousProfilesSampleRate ()Ljava/lang/Double; public fun getCron ()Lio/sentry/SentryOptions$Cron; public fun getDebug ()Ljava/lang/Boolean; public fun getDist ()Ljava/lang/String; @@ -494,7 +493,6 @@ public final class io/sentry/ExternalOptions { public fun isGlobalHubMode ()Ljava/lang/Boolean; public fun isSendDefaultPii ()Ljava/lang/Boolean; public fun isSendModules ()Ljava/lang/Boolean; - public fun setContinuousProfilesSampleRate (Ljava/lang/Double;)V public fun setCron (Lio/sentry/SentryOptions$Cron;)V public fun setDebug (Ljava/lang/Boolean;)V public fun setDist (Ljava/lang/String;)V @@ -726,7 +724,8 @@ public abstract interface class io/sentry/IContinuousProfiler { public abstract fun close ()V public abstract fun getProfilerId ()Lio/sentry/protocol/SentryId; public abstract fun isRunning ()Z - public abstract fun start ()V + public abstract fun reevaluateSampling ()V + public abstract fun start (Lio/sentry/TracesSampler;)V public abstract fun stop ()V } @@ -1409,7 +1408,8 @@ public final class io/sentry/NoOpContinuousProfiler : io/sentry/IContinuousProfi public static fun getInstance ()Lio/sentry/NoOpContinuousProfiler; public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun isRunning ()Z - public fun start ()V + public fun reevaluateSampling ()V + public fun start (Lio/sentry/TracesSampler;)V public fun stop ()V } @@ -2953,7 +2953,6 @@ public class io/sentry/SentryOptions { public fun getConnectionTimeoutMillis ()I public fun getContextTags ()Ljava/util/List; public fun getContinuousProfiler ()Lio/sentry/IContinuousProfiler; - public fun getContinuousProfilesSampleRate ()D public fun getCron ()Lio/sentry/SentryOptions$Cron; public fun getDateProvider ()Lio/sentry/SentryDateProvider; public fun getDebugMetaLoader ()Lio/sentry/internal/debugmeta/IDebugMetaLoader; @@ -2995,6 +2994,7 @@ public class io/sentry/SentryOptions { public fun getOptionsObservers ()Ljava/util/List; public fun getOutboxPath ()Ljava/lang/String; public fun getPerformanceCollectors ()Ljava/util/List; + public fun getProfileSessionSampleRate ()D public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; public fun getProfilingTracesDirPath ()Ljava/lang/String; @@ -3769,7 +3769,7 @@ public final class io/sentry/TraceContext$JsonKeys { public final class io/sentry/TracesSampler { public fun (Lio/sentry/SentryOptions;)V public fun sample (Lio/sentry/SamplingContext;)Lio/sentry/TracesSamplingDecision; - public fun sampleContinuousProfile ()Z + public fun sampleSessionProfile ()Z } public final class io/sentry/TracesSamplingDecision { diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 1a473d7c461..5aaa68c0371 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -14,13 +14,11 @@ public final class ExperimentalOptions { private @NotNull SentryReplayOptions sessionReplay; /** - * Configures the continuous profiling sample rate as a percentage of profiles to be sent in the - * range of 0.0 to 1.0. if 1.0 is set it means that 100% of profiles will be sent. If set to 0.1 - * only 10% of profiles will be sent. Profiles are picked randomly. Default is 1 (100%). - * ProfilesSampleRate takes precedence over this. To enable continuous profiling, don't set - * profilesSampleRate or profilesSampler, or set them to null. + * Indicates the percentage in which the profiles for the session will be created. Specifying 0 + * means never, 1.0 means always. The value needs to be >= 0.0 and <= 1.0 The default is 0 + * (disabled). */ - private double continuousProfilesSampleRate = 1.0; + private double profileSessionSampleRate = 0.0; public ExperimentalOptions(final boolean empty) { this.sessionReplay = new SentryReplayOptions(empty); @@ -35,18 +33,18 @@ public void setSessionReplay(final @NotNull SentryReplayOptions sessionReplayOpt this.sessionReplay = sessionReplayOptions; } - public double getContinuousProfilesSampleRate() { - return continuousProfilesSampleRate; + public double getProfileSessionSampleRate() { + return profileSessionSampleRate; } @ApiStatus.Experimental - public void setContinuousProfilesSampleRate(final double continuousProfilesSampleRate) { - if (!SampleRateUtils.isValidContinuousProfilesSampleRate(continuousProfilesSampleRate)) { + public void setProfileSessionSampleRate(final double profileSessionSampleRate) { + if (!SampleRateUtils.isValidContinuousProfilesSampleRate(profileSessionSampleRate)) { throw new IllegalArgumentException( "The value " - + continuousProfilesSampleRate + + profileSessionSampleRate + " is not valid. Use values between 0.0 and 1.0."); } - this.continuousProfilesSampleRate = continuousProfilesSampleRate; + this.profileSessionSampleRate = profileSessionSampleRate; } } diff --git a/sentry/src/main/java/io/sentry/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index 020b7aea9f6..d9b075e1c89 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -28,7 +28,6 @@ public final class ExternalOptions { private @Nullable Boolean enableDeduplication; private @Nullable Double tracesSampleRate; private @Nullable Double profilesSampleRate; - private @Nullable Double continuousProfilesSampleRate; private @Nullable SentryOptions.RequestSize maxRequestBodySize; private final @NotNull Map tags = new ConcurrentHashMap<>(); private @Nullable SentryOptions.Proxy proxy; @@ -74,8 +73,6 @@ public final class ExternalOptions { propertiesProvider.getBooleanProperty("uncaught.handler.print-stacktrace")); options.setTracesSampleRate(propertiesProvider.getDoubleProperty("traces-sample-rate")); options.setProfilesSampleRate(propertiesProvider.getDoubleProperty("profiles-sample-rate")); - options.setContinuousProfilesSampleRate( - propertiesProvider.getDoubleProperty("continuous-profiles-sample-rate")); options.setDebug(propertiesProvider.getBooleanProperty("debug")); options.setEnableDeduplication(propertiesProvider.getBooleanProperty("enable-deduplication")); options.setSendClientReports(propertiesProvider.getBooleanProperty("send-client-reports")); @@ -287,14 +284,6 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { this.profilesSampleRate = profilesSampleRate; } - public @Nullable Double getContinuousProfilesSampleRate() { - return continuousProfilesSampleRate; - } - - public void setContinuousProfilesSampleRate(final @Nullable Double continuousProfilesSampleRate) { - this.continuousProfilesSampleRate = continuousProfilesSampleRate; - } - public @Nullable SentryOptions.RequestSize getMaxRequestBodySize() { return maxRequestBodySize; } diff --git a/sentry/src/main/java/io/sentry/IContinuousProfiler.java b/sentry/src/main/java/io/sentry/IContinuousProfiler.java index 14ce41a8156..bd37f6e14f5 100644 --- a/sentry/src/main/java/io/sentry/IContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/IContinuousProfiler.java @@ -9,13 +9,15 @@ public interface IContinuousProfiler { boolean isRunning(); - void start(); + void start(final @NotNull TracesSampler tracesSampler); void stop(); /** Cancel the profiler and stops it. Used on SDK close. */ void close(); + void reevaluateSampling(); + @NotNull SentryId getProfilerId(); } diff --git a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java index 4ccf7cc681c..597c3c5cfbb 100644 --- a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java @@ -13,9 +13,6 @@ public static NoOpContinuousProfiler getInstance() { return instance; } - @Override - public void start() {} - @Override public void stop() {} @@ -24,9 +21,15 @@ public boolean isRunning() { return false; } + @Override + public void start(final @NotNull TracesSampler tracesSampler) {} + @Override public void close() {} + @Override + public void reevaluateSampling() {} + @Override public @NotNull SentryId getProfilerId() { return SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 4dacc8e4dac..93cfad44632 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -872,6 +872,8 @@ public SessionPair startSession() { if (session != null) { // Assumes session will NOT flush itself (Not passing any scopes to it) session.end(); + // Continuous profiler sample rate is reevaluated every time a session ends + options.getContinuousProfiler().reevaluateSampling(); } previousSession = session; @@ -945,6 +947,8 @@ public Session endSession() { try (final @NotNull ISentryLifecycleToken ignored = sessionLock.acquire()) { if (session != null) { session.end(); + // Continuous profiler sample rate is reevaluated every time a session ends + options.getContinuousProfiler().reevaluateSampling(); previousSession = session.clone(); session = null; } diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 638127cc547..e2b47b2821d 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -926,14 +926,7 @@ public void flush(long timeoutMillis) { @Override public void startProfiler() { if (getOptions().isContinuousProfilingEnabled()) { - if (getOptions().getInternalTracesSampler().sampleContinuousProfile()) { - getOptions().getLogger().log(SentryLevel.DEBUG, "Started continuous Profiling."); - getOptions().getContinuousProfiler().start(); - } else { - getOptions() - .getLogger() - .log(SentryLevel.DEBUG, "Profiler was not started due to sampling decision."); - } + getOptions().getContinuousProfiler().start(getOptions().getInternalTracesSampler()); } else if (getOptions().isProfilingEnabled()) { getOptions() .getLogger() diff --git a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java index c4d99a60f25..364a71e5cad 100644 --- a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java @@ -44,7 +44,7 @@ public SentryAppStartProfilingOptions() { traceSampleRate = samplingDecision.getSampleRate(); profileSampled = samplingDecision.getProfileSampled(); profileSampleRate = samplingDecision.getProfileSampleRate(); - continuousProfileSampled = options.getInternalTracesSampler().sampleContinuousProfile(); + continuousProfileSampled = options.getInternalTracesSampler().sampleSessionProfile(); profilingTracesDirPath = options.getProfilingTracesDirPath(); isProfilingEnabled = options.isProfilingEnabled(); isContinuousProfilingEnabled = options.isContinuousProfilingEnabled(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index ff42854431e..717a039c455 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1742,7 +1742,7 @@ public boolean isProfilingEnabled() { public boolean isContinuousProfilingEnabled() { return profilesSampleRate == null && profilesSampler == null - && experimental.getContinuousProfilesSampleRate() > 0; + && experimental.getProfileSessionSampleRate() > 0; } /** @@ -1790,15 +1790,15 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { } /** - * Returns the continuous profiling sample rate. Default is 1 (100%). ProfilesSampleRate takes - * precedence over this. To enable continuous profiling, don't set profilesSampleRate or - * profilesSampler, or set them to null. + * Returns the session sample rate. Default is 0 (disabled). ProfilesSampleRate takes precedence + * over this. To enable continuous profiling, don't set profilesSampleRate or profilesSampler, or + * set them to null. * * @return the sample rate */ @ApiStatus.Experimental - public double getContinuousProfilesSampleRate() { - return experimental.getContinuousProfilesSampleRate(); + public double getProfileSessionSampleRate() { + return experimental.getProfileSessionSampleRate(); } /** @@ -2727,9 +2727,6 @@ public void merge(final @NotNull ExternalOptions options) { if (options.getProfilesSampleRate() != null) { setProfilesSampleRate(options.getProfilesSampleRate()); } - if (options.getContinuousProfilesSampleRate() != null) { - experimental.setContinuousProfilesSampleRate(options.getContinuousProfilesSampleRate()); - } if (options.getDebug() != null) { setDebug(options.getDebug()); } diff --git a/sentry/src/main/java/io/sentry/TracesSampler.java b/sentry/src/main/java/io/sentry/TracesSampler.java index fcf6e3929ae..84e2c0e0ad1 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -85,8 +85,8 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont return new TracesSamplingDecision(false, null, false, null); } - public boolean sampleContinuousProfile() { - final double sampling = options.getContinuousProfilesSampleRate(); + public boolean sampleSessionProfile() { + final double sampling = options.getProfileSessionSampleRate(); return sample(sampling); } diff --git a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt index 3a1d66b264c..b25f67405cf 100644 --- a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt @@ -120,13 +120,6 @@ class ExternalOptionsTest { } } - @Test - fun `creates options with continuousProfilesSampleRate using external properties`() { - withPropertiesFile("continuous-profiles-sample-rate=0.2") { - assertEquals(0.2, it.continuousProfilesSampleRate) - } - } - @Test fun `creates options with enableDeduplication using external properties`() { withPropertiesFile("enable-deduplication=true") { diff --git a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt index e791651aefd..30ab1d090a5 100644 --- a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt @@ -1,6 +1,7 @@ package io.sentry import io.sentry.protocol.SentryId +import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -10,7 +11,7 @@ class NoOpContinuousProfilerTest { @Test fun `start does not throw`() = - profiler.start() + profiler.start(mock()) @Test fun `stop does not throw`() = @@ -29,4 +30,9 @@ class NoOpContinuousProfilerTest { fun `getProfilerId returns Empty SentryId`() { assertEquals(profiler.profilerId, SentryId.EMPTY_ID) } + + @Test + fun `reevaluateSampling does not throw`() { + profiler.reevaluateSampling() + } } diff --git a/sentry/src/test/java/io/sentry/ScopeTest.kt b/sentry/src/test/java/io/sentry/ScopeTest.kt index b8025735e8a..cbd10a07417 100644 --- a/sentry/src/test/java/io/sentry/ScopeTest.kt +++ b/sentry/src/test/java/io/sentry/ScopeTest.kt @@ -10,6 +10,7 @@ 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.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -391,6 +392,57 @@ class ScopeTest { assertNull(session) } + @Test + fun `Starting a session multiple times reevaluates profileSessionSampleRate`() { + val profiler = mock() + val options = SentryOptions().apply { + release = "0.0.1" + setContinuousProfiler(profiler) + experimental.profileSessionSampleRate = 1.0 + } + + val scope = Scope(options) + // The first time a session is started, sample rate is not reevaluated, as there's no need + scope.startSession() + verify(profiler, never()).reevaluateSampling() + // The second time a session is started, sample rate is reevaluated + scope.startSession() + verify(profiler).reevaluateSampling() + // Every time a session is started with an already running one, sample rate is reevaluated + scope.startSession() + verify(profiler, times(2)).reevaluateSampling() + } + + @Test + fun `Scope ends a session and reevaluates profileSessionSampleRate`() { + val profiler = mock() + val options = SentryOptions().apply { + release = "0.0.1" + setContinuousProfiler(profiler) + experimental.profileSessionSampleRate = 1.0 + } + + val scope = Scope(options) + scope.startSession() + verify(profiler, never()).reevaluateSampling() + scope.endSession() + verify(profiler).reevaluateSampling() + } + + @Test + fun `Scope ends a session and does not reevaluate profileSessionSampleRate if none exist`() { + val profiler = mock() + val options = SentryOptions().apply { + release = "0.0.1" + setContinuousProfiler(profiler) + experimental.profileSessionSampleRate = 1.0 + } + + val scope = Scope(options) + scope.endSession() + verify(profiler, never()).reevaluateSampling() + } + @Test fun `withSession returns a callback with the current Session`() { val options = SentryOptions().apply { diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 92343bdeb47..e5674b38235 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -15,7 +15,6 @@ import io.sentry.test.callMethod import io.sentry.test.createSentryClientMock import io.sentry.test.createTestScopes import io.sentry.util.HintUtils -import io.sentry.util.SentryRandom import io.sentry.util.StringUtils import junit.framework.TestCase.assertSame import org.mockito.kotlin.any @@ -1805,6 +1804,7 @@ class ScopesTest { setTransactionProfiler(profiler) compositePerformanceCollector = performanceCollector setContinuousProfiler(continuousProfiler) + experimental.profileSessionSampleRate = 1.0 } val sut = createScopes(options) sut.close() @@ -2166,27 +2166,26 @@ class ScopesTest { val profiler = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 } scopes.startProfiler() - verify(profiler).start() + verify(profiler).start(any()) } @Test - fun `startProfiler logs a message if not sampled`() { + fun `startProfiler logs instructions if continuous profiling is disabled`() { val profiler = mock() val logger = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) - it.experimental.continuousProfilesSampleRate = 0.1 + it.experimental.profileSessionSampleRate = 1.0 + it.profilesSampleRate = 1.0 it.setLogger(logger) it.isDebug = true } - // We cannot set sample rate to 0, as it would not start the profiler. So we set the seed to have consistent results - SentryRandom.current().setSeed(0) scopes.startProfiler() - - verify(profiler, never()).start() - verify(logger).log(eq(SentryLevel.DEBUG), eq("Profiler was not started due to sampling decision.")) + verify(profiler, never()).start(any()) + verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) } @Test @@ -2194,32 +2193,19 @@ class ScopesTest { val profiler = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 } scopes.stopProfiler() verify(profiler).stop() } - @Test - fun `startProfiler logs instructions if continuous profiling is disabled`() { - val profiler = mock() - val logger = mock() - val scopes = generateScopes { - it.setContinuousProfiler(profiler) - it.profilesSampleRate = 1.0 - it.setLogger(logger) - it.isDebug = true - } - scopes.startProfiler() - verify(profiler, never()).start() - verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) - } - @Test fun `stopProfiler logs instructions if continuous profiling is disabled`() { val profiler = mock() val logger = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 it.profilesSampleRate = 1.0 it.setLogger(logger) it.isDebug = true diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 4cea67c4eda..c5cbf6b11f4 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -195,17 +195,17 @@ class SentryOptionsTest { @Test fun `when options is initialized, isProfilingEnabled is false and isContinuousProfilingEnabled is true`() { assertFalse(SentryOptions().isProfilingEnabled) - assertTrue(SentryOptions().isContinuousProfilingEnabled) + assertFalse(SentryOptions().isContinuousProfilingEnabled) } @Test - fun `when profilesSampleRate is null and profilesSampler is null, isProfilingEnabled is false and isContinuousProfilingEnabled is true`() { + fun `when profilesSampleRate is null and profilesSampler is null, isProfilingEnabled is false and isContinuousProfilingEnabled is false`() { val options = SentryOptions().apply { this.profilesSampleRate = null this.profilesSampler = null } assertFalse(options.isProfilingEnabled) - assertTrue(options.isContinuousProfilingEnabled) + assertFalse(options.isContinuousProfilingEnabled) } @Test @@ -237,9 +237,9 @@ class SentryOptionsTest { } @Test - fun `when continuousProfilesSampleRate is set to a 0, isProfilingEnabled is false and isContinuousProfilingEnabled is false`() { + fun `when profileSessionSampleRate is set to a 0, isProfilingEnabled is false and isContinuousProfilingEnabled is false`() { val options = SentryOptions().apply { - this.experimental.continuousProfilesSampleRate = 0.0 + this.experimental.profileSessionSampleRate = 0.0 } assertFalse(options.isProfilingEnabled) assertFalse(options.isContinuousProfilingEnabled) @@ -264,21 +264,21 @@ class SentryOptionsTest { } @Test - fun `when setContinuousProfilesSampleRate is set to exactly 0, value is set`() { + fun `when profileSessionSampleRate is set to exactly 0, value is set`() { val options = SentryOptions().apply { - this.experimental.continuousProfilesSampleRate = 0.0 + this.experimental.profileSessionSampleRate = 0.0 } - assertEquals(0.0, options.continuousProfilesSampleRate) + assertEquals(0.0, options.profileSessionSampleRate) } @Test - fun `when setContinuousProfilesSampleRate is set to higher than 1_0, setter throws`() { - assertFailsWith { SentryOptions().experimental.continuousProfilesSampleRate = 1.0000000000001 } + fun `when profileSessionSampleRate is set to higher than 1_0, setter throws`() { + assertFailsWith { SentryOptions().experimental.profileSessionSampleRate = 1.0000000000001 } } @Test - fun `when setContinuousProfilesSampleRate is set to lower than 0, setter throws`() { - assertFailsWith { SentryOptions().experimental.continuousProfilesSampleRate = -0.0000000000001 } + fun `when profileSessionSampleRate is set to lower than 0, setter throws`() { + assertFailsWith { SentryOptions().experimental.profileSessionSampleRate = -0.0000000000001 } } @Test @@ -349,7 +349,6 @@ class SentryOptionsTest { externalOptions.enableUncaughtExceptionHandler = false externalOptions.tracesSampleRate = 0.5 externalOptions.profilesSampleRate = 0.5 - externalOptions.continuousProfilesSampleRate = 0.3 externalOptions.addInAppInclude("com.app") externalOptions.addInAppExclude("io.off") externalOptions.addTracePropagationTarget("localhost") @@ -396,7 +395,6 @@ class SentryOptionsTest { assertFalse(options.isEnableUncaughtExceptionHandler) assertEquals(0.5, options.tracesSampleRate) assertEquals(0.5, options.profilesSampleRate) - assertEquals(0.3, options.continuousProfilesSampleRate) assertEquals(listOf("com.app"), options.inAppIncludes) assertEquals(listOf("io.off"), options.inAppExcludes) assertEquals(listOf("localhost", "api.foo.com"), options.tracePropagationTargets) @@ -607,7 +605,7 @@ class SentryOptionsTest { fun `when profiling is disabled, isEnableAppStartProfiling is always false`() { val options = SentryOptions() options.isEnableAppStartProfiling = true - options.experimental.continuousProfilesSampleRate = 0.0 + options.experimental.profileSessionSampleRate = 0.0 assertFalse(options.isEnableAppStartProfiling) } @@ -615,7 +613,7 @@ class SentryOptionsTest { fun `when setEnableAppStartProfiling is called and continuous profiling is enabled, isEnableAppStartProfiling is true`() { val options = SentryOptions() options.isEnableAppStartProfiling = true - options.experimental.continuousProfilesSampleRate = 1.0 + options.experimental.profileSessionSampleRate = 1.0 assertTrue(options.isEnableAppStartProfiling) } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 36dda684749..1a9b4f4f1e9 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -18,7 +18,6 @@ import io.sentry.test.ImmediateExecutorService import io.sentry.test.createSentryClientMock import io.sentry.test.injectForField import io.sentry.util.PlatformTestManipulator -import io.sentry.util.SentryRandom import io.sentry.util.thread.IThreadChecker import io.sentry.util.thread.ThreadChecker import org.awaitility.kotlin.await @@ -407,7 +406,7 @@ class SentryTest { var sentryOptions: SentryOptions? = null Sentry.init { it.dsn = dsn - it.experimental.continuousProfilesSampleRate = 1.0 + it.experimental.profileSessionSampleRate = 1.0 it.cacheDirPath = tempPath sentryOptions = it } @@ -422,7 +421,7 @@ class SentryTest { var sentryOptions: SentryOptions? = null Sentry.init { it.dsn = dsn - it.experimental.continuousProfilesSampleRate = 0.0 + it.experimental.profileSessionSampleRate = 0.0 it.cacheDirPath = tempPath sentryOptions = it } @@ -1313,9 +1312,10 @@ class SentryTest { Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 } Sentry.startProfiler() - verify(profiler).start() + verify(profiler).start(any()) } @Test @@ -1327,21 +1327,7 @@ class SentryTest { it.profilesSampleRate = 1.0 } Sentry.startProfiler() - verify(profiler, never()).start() - } - - @Test - fun `startProfiler is ignored when not sampled`() { - val profiler = mock() - Sentry.init { - it.dsn = dsn - it.setContinuousProfiler(profiler) - it.experimental.continuousProfilesSampleRate = 0.1 - } - // We cannot set sample rate to 0, as it would not start the profiler. So we set the seed to have consistent results - SentryRandom.current().setSeed(0) - Sentry.startProfiler() - verify(profiler, never()).start() + verify(profiler, never()).start(any()) } @Test @@ -1350,6 +1336,7 @@ class SentryTest { Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 } Sentry.stopProfiler() verify(profiler).stop() diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 2fd047c9efa..46bec1077e9 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -18,7 +18,7 @@ class TracesSamplerTest { randomResult: Double? = null, tracesSampleRate: Double? = null, profilesSampleRate: Double? = null, - continuousProfilesSampleRate: Double? = null, + profileSessionSampleRate: Double? = null, tracesSamplerCallback: SentryOptions.TracesSamplerCallback? = null, profilesSamplerCallback: SentryOptions.ProfilesSamplerCallback? = null, logger: ILogger? = null @@ -34,8 +34,8 @@ class TracesSamplerTest { if (profilesSampleRate != null) { options.profilesSampleRate = profilesSampleRate } - if (continuousProfilesSampleRate != null) { - options.experimental.continuousProfilesSampleRate = continuousProfilesSampleRate + if (profileSessionSampleRate != null) { + options.experimental.profileSessionSampleRate = profileSessionSampleRate } if (tracesSamplerCallback != null) { options.tracesSampler = tracesSamplerCallback @@ -155,23 +155,23 @@ class TracesSamplerTest { } @Test - fun `when continuousProfilesSampleRate is not set returns true`() { + fun `when profileSessionSampleRate is not set returns true`() { val sampler = fixture.getSut(randomResult = 1.0) - val sampled = sampler.sampleContinuousProfile() - assertTrue(sampled) + val sampled = sampler.sampleSessionProfile() + assertFalse(sampled) } @Test - fun `when continuousProfilesSampleRate is set and random returns lower number returns true`() { - val sampler = fixture.getSut(randomResult = 0.1, continuousProfilesSampleRate = 0.2) - val sampled = sampler.sampleContinuousProfile() + fun `when profileSessionSampleRate is set and random returns lower number returns true`() { + val sampler = fixture.getSut(randomResult = 0.1, profileSessionSampleRate = 0.2) + val sampled = sampler.sampleSessionProfile() assertTrue(sampled) } @Test - fun `when continuousProfilesSampleRate is set and random returns greater number returns false`() { - val sampler = fixture.getSut(randomResult = 0.9, continuousProfilesSampleRate = 0.2) - val sampled = sampler.sampleContinuousProfile() + fun `when profileSessionSampleRate is set and random returns greater number returns false`() { + val sampler = fixture.getSut(randomResult = 0.9, profileSessionSampleRate = 0.2) + val sampled = sampler.sampleSessionProfile() assertFalse(sampled) } From 04e99f937e54bb66483ec628b460354bb8bdffbf Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 19 Feb 2025 11:35:11 +0100 Subject: [PATCH 07/15] renamed Sentry.startProfiler with Sentry.startProfileSession and Sentry.stopProfiler with Sentry.stopProfileSession --- CHANGELOG.md | 10 +++++----- .../io/sentry/samples/android/MyApplication.java | 2 +- sentry/src/main/java/io/sentry/HubAdapter.java | 8 ++++---- .../main/java/io/sentry/HubScopesWrapper.java | 8 ++++---- sentry/src/main/java/io/sentry/IScopes.java | 4 ++-- sentry/src/main/java/io/sentry/NoOpHub.java | 4 ++-- sentry/src/main/java/io/sentry/NoOpScopes.java | 4 ++-- sentry/src/main/java/io/sentry/Scopes.java | 4 ++-- .../src/main/java/io/sentry/ScopesAdapter.java | 8 ++++---- sentry/src/main/java/io/sentry/Sentry.java | 8 ++++---- sentry/src/test/java/io/sentry/HubAdapterTest.kt | 12 ++++++------ sentry/src/test/java/io/sentry/NoOpHubTest.kt | 4 ++-- .../src/test/java/io/sentry/ScopesAdapterTest.kt | 12 ++++++------ sentry/src/test/java/io/sentry/ScopesTest.kt | 16 ++++++++-------- sentry/src/test/java/io/sentry/SentryTest.kt | 16 ++++++++-------- 15 files changed, 60 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79ba9f7283c..a912f1a1e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - Add Continuous Profiling Support ([#3710](https://github.com/getsentry/sentry-java/pull/3710)) - To enable Continuous Profiling use the `Sentry.startProfiler` and `Sentry.stopProfiler` experimental APIs. Sampling rate can be set through `options.continuousProfilesSampleRate` (defaults to 1.0). + To enable Continuous Profiling use the `Sentry.startProfileSession` and `Sentry.stopProfileSession` experimental APIs. Sampling rate can be set through `options.profileSessionSampleRate` (defaults to 0.0). Note: Both `options.profilesSampler` and `options.profilesSampleRate` must **not** be set to enable Continuous Profiling. ```java @@ -18,10 +18,10 @@ options.getExperimental().setProfileSessionSampleRate(1.0); } // Start profiling - Sentry.startProfiler(); + Sentry.startProfileSession(); // After all profiling is done, stop the profiler. Profiles can last indefinitely if not stopped. - Sentry.stopProfiler(); + Sentry.stopProfileSession(); ``` ```kotlin import io.sentry.android.core.SentryAndroid @@ -32,10 +32,10 @@ options.experimental.profileSessionSampleRate = 1.0 } // Start profiling - Sentry.startProfiler() + Sentry.startProfileSession() // After all profiling is done, stop the profiler. Profiles can last indefinitely if not stopped. - Sentry.stopProfiler() + Sentry.stopProfileSession() ``` To learn more visit [Sentry's Continuous Profiling](https://docs.sentry.io/product/explore/profiling/transaction-vs-continuous-profiling/#continuous-profiling-mode) documentation page. diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java index 572c4cdba72..13117e39e6c 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MyApplication.java @@ -9,7 +9,7 @@ public class MyApplication extends Application { @Override public void onCreate() { - Sentry.startProfiler(); + Sentry.startProfileSession(); strictMode(); super.onCreate(); diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 537bcdf1044..dadb40eff5b 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -278,13 +278,13 @@ public boolean isAncestorOf(final @Nullable IScopes otherScopes) { } @Override - public void startProfiler() { - Sentry.startProfiler(); + public void startProfileSession() { + Sentry.startProfileSession(); } @Override - public void stopProfiler() { - Sentry.stopProfiler(); + public void stopProfileSession() { + Sentry.stopProfileSession(); } @Override diff --git a/sentry/src/main/java/io/sentry/HubScopesWrapper.java b/sentry/src/main/java/io/sentry/HubScopesWrapper.java index d6755c2c41d..1b3aa408439 100644 --- a/sentry/src/main/java/io/sentry/HubScopesWrapper.java +++ b/sentry/src/main/java/io/sentry/HubScopesWrapper.java @@ -278,13 +278,13 @@ public boolean isAncestorOf(final @Nullable IScopes otherScopes) { } @Override - public void startProfiler() { - scopes.startProfiler(); + public void startProfileSession() { + scopes.startProfileSession(); } @Override - public void stopProfiler() { - scopes.stopProfiler(); + public void stopProfileSession() { + scopes.stopProfileSession(); } @ApiStatus.Internal diff --git a/sentry/src/main/java/io/sentry/IScopes.java b/sentry/src/main/java/io/sentry/IScopes.java index 59a577f04b3..3b2bbef34d1 100644 --- a/sentry/src/main/java/io/sentry/IScopes.java +++ b/sentry/src/main/java/io/sentry/IScopes.java @@ -592,9 +592,9 @@ ITransaction startTransaction( final @NotNull TransactionContext transactionContext, final @NotNull TransactionOptions transactionOptions); - void startProfiler(); + void startProfileSession(); - void stopProfiler(); + void stopProfileSession(); /** * Associates {@link ISpan} and the transaction name with the {@link Throwable}. Used to determine diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 925f1e64eeb..71f5d3fb783 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -244,10 +244,10 @@ public boolean isAncestorOf(@Nullable IScopes otherScopes) { } @Override - public void startProfiler() {} + public void startProfileSession() {} @Override - public void stopProfiler() {} + public void stopProfileSession() {} @Override public void setSpanContext( diff --git a/sentry/src/main/java/io/sentry/NoOpScopes.java b/sentry/src/main/java/io/sentry/NoOpScopes.java index 11bae042b0e..25d2037ccaf 100644 --- a/sentry/src/main/java/io/sentry/NoOpScopes.java +++ b/sentry/src/main/java/io/sentry/NoOpScopes.java @@ -239,10 +239,10 @@ public boolean isAncestorOf(@Nullable IScopes otherScopes) { } @Override - public void startProfiler() {} + public void startProfileSession() {} @Override - public void stopProfiler() {} + public void stopProfileSession() {} @Override public void setSpanContext( diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index e2b47b2821d..c57a86b264f 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -924,7 +924,7 @@ public void flush(long timeoutMillis) { } @Override - public void startProfiler() { + public void startProfileSession() { if (getOptions().isContinuousProfilingEnabled()) { getOptions().getContinuousProfiler().start(getOptions().getInternalTracesSampler()); } else if (getOptions().isProfilingEnabled()) { @@ -937,7 +937,7 @@ public void startProfiler() { } @Override - public void stopProfiler() { + public void stopProfileSession() { if (getOptions().isContinuousProfilingEnabled()) { getOptions().getLogger().log(SentryLevel.DEBUG, "Stopped continuous Profiling."); getOptions().getContinuousProfiler().stop(); diff --git a/sentry/src/main/java/io/sentry/ScopesAdapter.java b/sentry/src/main/java/io/sentry/ScopesAdapter.java index 9944a87a0a8..62ba91993c0 100644 --- a/sentry/src/main/java/io/sentry/ScopesAdapter.java +++ b/sentry/src/main/java/io/sentry/ScopesAdapter.java @@ -281,13 +281,13 @@ public boolean isAncestorOf(final @Nullable IScopes otherScopes) { } @Override - public void startProfiler() { - Sentry.startProfiler(); + public void startProfileSession() { + Sentry.startProfileSession(); } @Override - public void stopProfiler() { - Sentry.stopProfiler(); + public void stopProfileSession() { + Sentry.stopProfileSession(); } @ApiStatus.Internal diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 85ea2bc2247..02e7d168c79 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -1052,14 +1052,14 @@ public static void endSession() { /** Starts the continuous profiler, if enabled. */ @ApiStatus.Experimental - public static void startProfiler() { - getCurrentScopes().startProfiler(); + public static void startProfileSession() { + getCurrentScopes().startProfileSession(); } /** Stops the continuous profiler, if enabled. */ @ApiStatus.Experimental - public static void stopProfiler() { - getCurrentScopes().stopProfiler(); + public static void stopProfileSession() { + getCurrentScopes().stopProfileSession(); } /** diff --git a/sentry/src/test/java/io/sentry/HubAdapterTest.kt b/sentry/src/test/java/io/sentry/HubAdapterTest.kt index 310c1537099..06b9870a031 100644 --- a/sentry/src/test/java/io/sentry/HubAdapterTest.kt +++ b/sentry/src/test/java/io/sentry/HubAdapterTest.kt @@ -266,13 +266,13 @@ class HubAdapterTest { verify(scopes).reportFullyDisplayed() } - @Test fun `startProfiler calls Hub`() { - HubAdapter.getInstance().startProfiler() - verify(scopes).startProfiler() + @Test fun `startProfileSession calls Hub`() { + HubAdapter.getInstance().startProfileSession() + verify(scopes).startProfileSession() } - @Test fun `stopProfiler calls Hub`() { - HubAdapter.getInstance().stopProfiler() - verify(scopes).stopProfiler() + @Test fun `stopProfileSession calls Hub`() { + HubAdapter.getInstance().stopProfileSession() + verify(scopes).stopProfileSession() } } diff --git a/sentry/src/test/java/io/sentry/NoOpHubTest.kt b/sentry/src/test/java/io/sentry/NoOpHubTest.kt index e0eb08ded00..fdf61859700 100644 --- a/sentry/src/test/java/io/sentry/NoOpHubTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpHubTest.kt @@ -117,8 +117,8 @@ class NoOpHubTest { } @Test - fun `startProfiler doesnt throw`() = sut.startProfiler() + fun `startProfileSession doesnt throw`() = sut.startProfileSession() @Test - fun `stopProfiler doesnt throw`() = sut.stopProfiler() + fun `stopProfileSession doesnt throw`() = sut.stopProfileSession() } diff --git a/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt b/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt index 9c7418c6b55..35645a8986d 100644 --- a/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesAdapterTest.kt @@ -266,13 +266,13 @@ class ScopesAdapterTest { verify(scopes).reportFullyDisplayed() } - @Test fun `startProfiler calls Scopes`() { - ScopesAdapter.getInstance().startProfiler() - verify(scopes).startProfiler() + @Test fun `startProfileSession calls Scopes`() { + ScopesAdapter.getInstance().startProfileSession() + verify(scopes).startProfileSession() } - @Test fun `stopProfiler calls Scopes`() { - ScopesAdapter.getInstance().stopProfiler() - verify(scopes).stopProfiler() + @Test fun `stopProfileSession calls Scopes`() { + ScopesAdapter.getInstance().stopProfileSession() + verify(scopes).stopProfileSession() } } diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index e5674b38235..9cb8ca177af 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -2162,18 +2162,18 @@ class ScopesTest { } @Test - fun `startProfiler starts the continuous profiler`() { + fun `startProfileSession starts the continuous profiler`() { val profiler = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) it.experimental.profileSessionSampleRate = 1.0 } - scopes.startProfiler() + scopes.startProfileSession() verify(profiler).start(any()) } @Test - fun `startProfiler logs instructions if continuous profiling is disabled`() { + fun `startProfileSession logs instructions if continuous profiling is disabled`() { val profiler = mock() val logger = mock() val scopes = generateScopes { @@ -2183,24 +2183,24 @@ class ScopesTest { it.setLogger(logger) it.isDebug = true } - scopes.startProfiler() + scopes.startProfileSession() verify(profiler, never()).start(any()) verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) } @Test - fun `stopProfiler stops the continuous profiler`() { + fun `stopProfileSession stops the continuous profiler`() { val profiler = mock() val scopes = generateScopes { it.setContinuousProfiler(profiler) it.experimental.profileSessionSampleRate = 1.0 } - scopes.stopProfiler() + scopes.stopProfileSession() verify(profiler).stop() } @Test - fun `stopProfiler logs instructions if continuous profiling is disabled`() { + fun `stopProfileSession logs instructions if continuous profiling is disabled`() { val profiler = mock() val logger = mock() val scopes = generateScopes { @@ -2210,7 +2210,7 @@ class ScopesTest { it.setLogger(logger) it.isDebug = true } - scopes.stopProfiler() + scopes.stopProfileSession() verify(profiler, never()).stop() verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 1a9b4f4f1e9..ef337dff90f 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -1307,50 +1307,50 @@ class SentryTest { } @Test - fun `startProfiler starts the continuous profiler`() { + fun `startProfileSession starts the continuous profiler`() { val profiler = mock() Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) it.experimental.profileSessionSampleRate = 1.0 } - Sentry.startProfiler() + Sentry.startProfileSession() verify(profiler).start(any()) } @Test - fun `startProfiler is ignored when continuous profiling is disabled`() { + fun `startProfileSession is ignored when continuous profiling is disabled`() { val profiler = mock() Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) it.profilesSampleRate = 1.0 } - Sentry.startProfiler() + Sentry.startProfileSession() verify(profiler, never()).start(any()) } @Test - fun `stopProfiler stops the continuous profiler`() { + fun `stopProfileSession stops the continuous profiler`() { val profiler = mock() Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) it.experimental.profileSessionSampleRate = 1.0 } - Sentry.stopProfiler() + Sentry.stopProfileSession() verify(profiler).stop() } @Test - fun `stopProfiler is ignored when continuous profiling is disabled`() { + fun `stopProfileSession is ignored when continuous profiling is disabled`() { val profiler = mock() Sentry.init { it.dsn = dsn it.setContinuousProfiler(profiler) it.profilesSampleRate = 1.0 } - Sentry.stopProfiler() + Sentry.stopProfileSession() verify(profiler, never()).stop() } From c0279e2b19db11620676a9f176c69c904cf7d1f5 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 19 Feb 2025 11:58:55 +0100 Subject: [PATCH 08/15] renamed Sentry.startProfiler with Sentry.startProfileSession and Sentry.stopProfiler with Sentry.stopProfileSession --- sentry/api/sentry.api | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index ad5c7600821..1c5988ac9e6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -625,10 +625,10 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -692,10 +692,10 @@ public final class io/sentry/HubScopesWrapper : io/sentry/IHub { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -931,13 +931,13 @@ public abstract interface class io/sentry/IScopes { public abstract fun setTag (Ljava/lang/String;Ljava/lang/String;)V public abstract fun setTransaction (Ljava/lang/String;)V public abstract fun setUser (Lio/sentry/protocol/User;)V - public abstract fun startProfiler ()V + public abstract fun startProfileSession ()V public abstract fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public abstract fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ITransaction; public fun startTransaction (Ljava/lang/String;Ljava/lang/String;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public abstract fun stopProfiler ()V + public abstract fun stopProfileSession ()V public abstract fun withIsolationScope (Lio/sentry/ScopeCallback;)V public abstract fun withScope (Lio/sentry/ScopeCallback;)V } @@ -1478,10 +1478,10 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -1640,10 +1640,10 @@ public final class io/sentry/NoOpScopes : io/sentry/IScopes { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -2298,10 +2298,10 @@ public final class io/sentry/Scopes : io/sentry/IScopes { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -2365,10 +2365,10 @@ public final class io/sentry/ScopesAdapter : io/sentry/IScopes { public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V - public fun startProfiler ()V + public fun startProfileSession ()V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public fun stopProfiler ()V + public fun stopProfileSession ()V public fun withIsolationScope (Lio/sentry/ScopeCallback;)V public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -2471,14 +2471,14 @@ public final class io/sentry/Sentry { public static fun setTag (Ljava/lang/String;Ljava/lang/String;)V public static fun setTransaction (Ljava/lang/String;)V public static fun setUser (Lio/sentry/protocol/User;)V - public static fun startProfiler ()V + public static fun startProfileSession ()V public static fun startSession ()V public static fun startTransaction (Lio/sentry/TransactionContext;)Lio/sentry/ITransaction; public static fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; public static fun startTransaction (Ljava/lang/String;Ljava/lang/String;)Lio/sentry/ITransaction; public static fun startTransaction (Ljava/lang/String;Ljava/lang/String;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; public static fun startTransaction (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; - public static fun stopProfiler ()V + public static fun stopProfileSession ()V public static fun withIsolationScope (Lio/sentry/ScopeCallback;)V public static fun withScope (Lio/sentry/ScopeCallback;)V } From fbe53ebf44903d22f7fcd61a731c99a47d336865 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 24 Feb 2025 13:39:33 +0100 Subject: [PATCH 09/15] Added ProfileLifecycle Sentry.startProfileSession() will work only in MANUAL mode Tracer start will start profiler only in TRACE mode Tracer and spans now attach profilerId only when sampled --- .../api/sentry-android-core.api | 5 +- .../core/AndroidContinuousProfiler.java | 75 ++++++++++--- .../android/core/ManifestMetadataReader.java | 16 +++ .../core/SentryPerformanceProvider.java | 3 +- .../core/AndroidContinuousProfilerTest.kt | 105 +++++++++++------- .../core/ManifestMetadataReaderTest.kt | 27 +++++ sentry/api/sentry.api | 21 +++- .../java/io/sentry/ExperimentalOptions.java | 27 +++++ .../java/io/sentry/IContinuousProfiler.java | 5 +- .../io/sentry/NoOpContinuousProfiler.java | 6 +- .../main/java/io/sentry/ProfileLifecycle.java | 18 +++ sentry/src/main/java/io/sentry/Scopes.java | 54 +++++++-- .../SentryAppStartProfilingOptions.java | 25 +++++ .../main/java/io/sentry/SentryOptions.java | 11 ++ .../src/main/java/io/sentry/SentryTracer.java | 8 +- .../test/java/io/sentry/JsonSerializerTest.kt | 8 +- .../io/sentry/NoOpContinuousProfilerTest.kt | 4 +- sentry/src/test/java/io/sentry/ScopesTest.kt | 90 ++++++++++++++- .../test/java/io/sentry/SentryOptionsTest.kt | 14 +++ sentry/src/test/java/io/sentry/SentryTest.kt | 29 ++++- .../test/java/io/sentry/SentryTracerTest.kt | 53 ++++++++- 21 files changed, 510 insertions(+), 94 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/ProfileLifecycle.java diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index ad1fd69cf5b..fe65fab8243 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -40,11 +40,12 @@ public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IConti public fun (Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ILogger;Ljava/lang/String;ILio/sentry/ISentryExecutorService;)V public fun close ()V public fun getProfilerId ()Lio/sentry/protocol/SentryId; + public fun getRootSpanCounter ()I public fun isRunning ()Z public fun onRateLimitChanged (Lio/sentry/transport/RateLimiter;)V public fun reevaluateSampling ()V - public fun start (Lio/sentry/TracesSampler;)V - public fun stop ()V + public fun startProfileSession (Lio/sentry/ProfileLifecycle;Lio/sentry/TracesSampler;)V + public fun stopProfileSession (Lio/sentry/ProfileLifecycle;)V } public final class io/sentry/android/core/AndroidCpuCollector : io/sentry/IPerformanceSnapshotCollector { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java index 764d29d1f94..3f840f21793 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java @@ -4,7 +4,6 @@ import static io.sentry.IConnectionStatusProvider.ConnectionStatus.DISCONNECTED; import static java.util.concurrent.TimeUnit.SECONDS; -import android.annotation.SuppressLint; import android.os.Build; import io.sentry.CompositePerformanceCollector; import io.sentry.DataCategory; @@ -15,6 +14,7 @@ import io.sentry.NoOpScopes; import io.sentry.PerformanceCollectionData; import io.sentry.ProfileChunk; +import io.sentry.ProfileLifecycle; import io.sentry.Sentry; import io.sentry.SentryDate; import io.sentry.SentryLevel; @@ -58,6 +58,7 @@ public class AndroidContinuousProfiler private @NotNull SentryDate startProfileChunkTimestamp = new SentryNanotimeDate(); private boolean shouldSample = true; private boolean isSampled = false; + private int rootSpanCounter = 0; public AndroidContinuousProfiler( final @NotNull BuildInfoProvider buildInfoProvider, @@ -103,7 +104,10 @@ private void init() { logger); } - public synchronized void start(final @NotNull TracesSampler tracesSampler) { + @Override + public synchronized void startProfileSession( + final @NotNull ProfileLifecycle profileLifecycle, + final @NotNull TracesSampler tracesSampler) { if (shouldSample) { isSampled = tracesSampler.sampleSessionProfile(); shouldSample = false; @@ -112,15 +116,31 @@ public synchronized void start(final @NotNull TracesSampler tracesSampler) { logger.log(SentryLevel.DEBUG, "Profiler was not started due to sampling decision."); return; } - if (isRunning()) { - logger.log(SentryLevel.DEBUG, "Profiler is already running."); - return; + switch (profileLifecycle) { + case TRACE: + // rootSpanCounter should never be negative, unless the user changed profile lifecycle while + // the profiler is running or close() is called. This is just a safety check. + if (rootSpanCounter < 0) { + rootSpanCounter = 0; + } + rootSpanCounter++; + break; + case MANUAL: + // We check if the profiler is already running and log a message only in manual mode, since + // in trace mode we can have multiple concurrent traces + if (isRunning()) { + logger.log(SentryLevel.DEBUG, "Profiler is already running."); + return; + } + break; + } + if (!isRunning()) { + logger.log(SentryLevel.DEBUG, "Started Profiler."); + start(); } - logger.log(SentryLevel.DEBUG, "Started Profiler."); - startProfile(); } - private synchronized void startProfile() { + private synchronized void start() { if ((scopes == null || scopes == NoOpScopes.getInstance()) && Sentry.getCurrentScopes() != NoOpScopes.getInstance()) { this.scopes = Sentry.getCurrentScopes(); @@ -150,7 +170,7 @@ private synchronized void startProfile() { || rateLimiter.isActiveForCategory(DataCategory.ProfileChunk))) { logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler."); // Let's stop and reset profiler id, as the profile is now broken anyway - stop(); + stop(false); return; } @@ -158,7 +178,7 @@ private synchronized void startProfile() { if (scopes.getOptions().getConnectionStatusProvider().getConnectionStatus() == DISCONNECTED) { logger.log(SentryLevel.WARNING, "Device is offline. Stopping profiler."); // Let's stop and reset profiler id, as the profile is now broken anyway - stop(); + stop(false); return; } startProfileChunkTimestamp = scopes.getOptions().getDateProvider().now(); @@ -195,11 +215,28 @@ private synchronized void startProfile() { } } - public synchronized void stop() { - stop(false); + @Override + public synchronized void stopProfileSession(final @NotNull ProfileLifecycle profileLifecycle) { + switch (profileLifecycle) { + case TRACE: + rootSpanCounter--; + // If there are active spans, and profile lifecycle is trace, we don't stop the profiler + if (rootSpanCounter > 0) { + return; + } + // rootSpanCounter should never be negative, unless the user changed profile lifecycle while + // the profiler is running or close() is called. This is just a safety check. + if (rootSpanCounter < 0) { + rootSpanCounter = 0; + } + stop(false); + break; + case MANUAL: + stop(false); + break; + } } - @SuppressLint("NewApi") private synchronized void stop(final boolean restartProfiler) { if (stopFuture != null) { stopFuture.cancel(true); @@ -256,7 +293,7 @@ private synchronized void stop(final boolean restartProfiler) { if (restartProfiler) { logger.log(SentryLevel.DEBUG, "Profile chunk finished. Starting a new one."); - startProfile(); + start(); } else { // When the profiler is stopped manually, we have to reset its id profilerId = SentryId.EMPTY_ID; @@ -269,7 +306,8 @@ public synchronized void reevaluateSampling() { } public synchronized void close() { - stop(); + rootSpanCounter = 0; + stop(false); isClosed.set(true); } @@ -315,13 +353,18 @@ Future getStopFuture() { return stopFuture; } + @VisibleForTesting + public int getRootSpanCounter() { + return rootSpanCounter; + } + @Override public void onRateLimitChanged(@NotNull RateLimiter rateLimiter) { // We stop the profiler as soon as we are rate limited, to avoid the performance overhead if (rateLimiter.isActiveForCategory(All) || rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)) { logger.log(SentryLevel.WARNING, "SDK is rate limited. Stopping profiler."); - stop(); + stop(false); } // If we are not rate limited anymore, we don't do anything: the profile is broken, so it's // useless to restart it automatically 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 04346f43455..fc05de28ea1 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 @@ -6,6 +6,7 @@ import android.os.Bundle; import io.sentry.ILogger; import io.sentry.InitPriority; +import io.sentry.ProfileLifecycle; import io.sentry.SentryIntegrationPackageStorage; import io.sentry.SentryLevel; import io.sentry.protocol.SdkVersion; @@ -67,6 +68,8 @@ final class ManifestMetadataReader { static final String PROFILE_SESSION_SAMPLE_RATE = "io.sentry.traces.profiling.session-sample-rate"; + static final String PROFILE_LIFECYCLE = "io.sentry.traces.profiling.lifecycle"; + @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets"; @@ -326,6 +329,19 @@ static void applyMetadata( } } + final String profileLifecycle = + readString( + metadata, + logger, + PROFILE_LIFECYCLE, + options.getProfileLifecycle().name().toLowerCase(Locale.ROOT)); + if (profileLifecycle != null) { + options + .getExperimental() + .setProfileLifecycle( + ProfileLifecycle.valueOf(profileLifecycle.toUpperCase(Locale.ROOT))); + } + options.setEnableUserInteractionTracing( readBool(metadata, logger, TRACES_UI_ENABLE, options.isEnableUserInteractionTracing())); 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 d46f2cc542a..d11a2b48195 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 @@ -186,7 +186,8 @@ private void createAndStartContinuousProfiler( sentryOptions .getExperimental() .setProfileSessionSampleRate(profilingOptions.isContinuousProfileSampled() ? 1 : 0); - appStartContinuousProfiler.start(new TracesSampler(sentryOptions)); + appStartContinuousProfiler.startProfileSession( + profilingOptions.getProfileLifecycle(), new TracesSampler(sentryOptions)); } private void createAndStartTransactionProfiler( diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt index 79d151fe4c5..e77d82fe465 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt @@ -13,6 +13,7 @@ import io.sentry.IScopes import io.sentry.ISentryExecutorService import io.sentry.MemoryCollectionData import io.sentry.PerformanceCollectionData +import io.sentry.ProfileLifecycle import io.sentry.Sentry import io.sentry.SentryLevel import io.sentry.SentryNanotimeDate @@ -142,28 +143,54 @@ class AndroidContinuousProfilerTest { @Test fun `isRunning reflects profiler status`() { val profiler = fixture.getSut() - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) - profiler.stop() + profiler.stopProfileSession(ProfileLifecycle.MANUAL) assertFalse(profiler.isRunning) } @Test - fun `profiler multiple starts are ignored`() { + fun `profiler multiple starts are ignored in manual mode`() { val profiler = fixture.getSut() - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) verify(fixture.mockLogger, never()).log(eq(SentryLevel.DEBUG), eq("Profiler is already running.")) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockLogger).log(eq(SentryLevel.DEBUG), eq("Profiler is already running.")) assertTrue(profiler.isRunning) + assertEquals(0, profiler.rootSpanCounter) + } + + @Test + fun `profiler multiple starts are accepted in trace mode`() { + val profiler = fixture.getSut() + + // rootSpanCounter is incremented when the profiler starts in trace mode + assertEquals(0, profiler.rootSpanCounter) + profiler.startProfileSession(ProfileLifecycle.TRACE, fixture.mockTracesSampler) + assertEquals(1, profiler.rootSpanCounter) + assertTrue(profiler.isRunning) + profiler.startProfileSession(ProfileLifecycle.TRACE, fixture.mockTracesSampler) + verify(fixture.mockLogger, never()).log(eq(SentryLevel.DEBUG), eq("Profiler is already running.")) + assertTrue(profiler.isRunning) + assertEquals(2, profiler.rootSpanCounter) + + // rootSpanCounter is decremented when the profiler stops in trace mode, and keeps running until rootSpanCounter is 0 + profiler.stopProfileSession(ProfileLifecycle.TRACE) + assertEquals(1, profiler.rootSpanCounter) + assertTrue(profiler.isRunning) + + // only when rootSpanCounter is 0 the profiler stops + profiler.stopProfileSession(ProfileLifecycle.TRACE) + assertEquals(0, profiler.rootSpanCounter) + assertFalse(profiler.isRunning) } @Test fun `profiler logs a warning on start if not sampled`() { val profiler = fixture.getSut() whenever(fixture.mockTracesSampler.sampleSessionProfile()).thenReturn(false) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) verify(fixture.mockLogger).log(eq(SentryLevel.DEBUG), eq("Profiler was not started due to sampling decision.")) } @@ -173,10 +200,10 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut() verify(fixture.mockTracesSampler, never()).sampleSessionProfile() // The first time the profiler is started, the sessionSampleRate is evaluated - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() // Then, the sessionSampleRate is not evaluated again - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() } @@ -185,13 +212,13 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut() verify(fixture.mockTracesSampler, never()).sampleSessionProfile() // The first time the profiler is started, the sessionSampleRate is evaluated - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() // When reevaluateSampling is called, the sessionSampleRate is not evaluated immediately profiler.reevaluateSampling() verify(fixture.mockTracesSampler, times(1)).sampleSessionProfile() // Then, when the profiler starts again, the sessionSampleRate is reevaluated - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockTracesSampler, times(2)).sampleSessionProfile() } @@ -201,7 +228,7 @@ class AndroidContinuousProfilerTest { whenever(it.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP) } val profiler = fixture.getSut(buildInfo) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -210,7 +237,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.profilesSampleRate = 0.0 } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) } @@ -226,8 +253,8 @@ class AndroidContinuousProfilerTest { ) // Regardless of how many times the profiler is started, the option is evaluated and logged only once - profiler.start(fixture.mockTracesSampler) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockLogger, times(1)).log( SentryLevel.WARNING, "Disabling profiling because no profiling traces dir path is defined in options." @@ -247,8 +274,8 @@ class AndroidContinuousProfilerTest { ) // Regardless of how many times the profiler is started, the option is evaluated and logged only once - profiler.start(fixture.mockTracesSampler) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.mockLogger, times(1)).log( SentryLevel.WARNING, "Disabling profiling because trace rate is set to %d", @@ -261,7 +288,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.cacheDirPath = null } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -270,7 +297,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.cacheDirPath = "" } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -279,7 +306,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.profilingTracesHz = 0 } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) } @@ -290,9 +317,9 @@ class AndroidContinuousProfilerTest { it.executorService = mockExecutorService } whenever(mockExecutorService.submit(any>())).thenReturn(mock()) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(mockExecutorService, never()).submit(any()) - profiler.stop() + profiler.stopProfileSession(ProfileLifecycle.MANUAL) verify(mockExecutorService, never()).submit(any>()) } @@ -301,8 +328,8 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { File(it.profilingTracesDirPath!!).setWritable(false) } - profiler.start(fixture.mockTracesSampler) - profiler.stop() + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + profiler.stopProfileSession(ProfileLifecycle.MANUAL) // We assert that no trace files are written assertTrue( File(fixture.options.profilingTracesDirPath!!) @@ -318,7 +345,7 @@ class AndroidContinuousProfilerTest { fixture.options.compositePerformanceCollector = performanceCollector val profiler = fixture.getSut() verify(performanceCollector, never()).start(any()) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(performanceCollector).start(any()) } @@ -327,9 +354,9 @@ class AndroidContinuousProfilerTest { val performanceCollector = mock() fixture.options.compositePerformanceCollector = performanceCollector val profiler = fixture.getSut() - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(performanceCollector, never()).stop(any()) - profiler.stop() + profiler.stopProfileSession(ProfileLifecycle.MANUAL) verify(performanceCollector).stop(any()) } @@ -338,16 +365,16 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut() val frameMetricsCollectorId = "id" whenever(fixture.frameMetricsCollector.startCollection(any())).thenReturn(frameMetricsCollectorId) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) verify(fixture.frameMetricsCollector, never()).stopCollection(frameMetricsCollectorId) - profiler.stop() + profiler.stopProfileSession(ProfileLifecycle.MANUAL) verify(fixture.frameMetricsCollector).stopCollection(frameMetricsCollectorId) } @Test fun `profiler stops profiling and clear scheduled job on close`() { val profiler = fixture.getSut() - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) profiler.close() @@ -369,7 +396,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) executorService.runAll() @@ -387,7 +414,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We run the executor service to trigger the profiler restart (chunk finish) executorService.runAll() @@ -411,8 +438,8 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start(fixture.mockTracesSampler) - profiler.stop() + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) + profiler.stopProfileSession(ProfileLifecycle.MANUAL) // We run the executor service to send the profile chunk executorService.runAll() verify(fixture.scopes).captureProfileChunk( @@ -430,13 +457,13 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We run the executor service to trigger the profiler restart (chunk finish) executorService.runAll() verify(fixture.scopes, never()).captureProfileChunk(any()) // We stop the profiler, which should send an additional chunk - profiler.stop() + profiler.stopProfileSession(ProfileLifecycle.MANUAL) // Now the executor is used to send the chunk executorService.runAll() verify(fixture.scopes, times(2)).captureProfileChunk(any()) @@ -448,7 +475,7 @@ class AndroidContinuousProfilerTest { val profiler = fixture.getSut { it.executorService = executorService } - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) // We close the profiler, which should prevent sending additional chunks @@ -468,7 +495,7 @@ class AndroidContinuousProfilerTest { val rateLimiter = mock() whenever(rateLimiter.isActiveForCategory(DataCategory.ProfileChunk)).thenReturn(true) - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertTrue(profiler.isRunning) // If the SDK is rate limited, the profiler should stop @@ -489,7 +516,7 @@ class AndroidContinuousProfilerTest { whenever(fixture.scopes.rateLimiter).thenReturn(rateLimiter) // If the SDK is rate limited, the profiler should never start - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) assertEquals(SentryId.EMPTY_ID, profiler.profilerId) verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler.")) @@ -506,7 +533,7 @@ class AndroidContinuousProfilerTest { } // If the device is offline, the profiler should never start - profiler.start(fixture.mockTracesSampler) + profiler.startProfileSession(ProfileLifecycle.MANUAL, fixture.mockTracesSampler) assertFalse(profiler.isRunning) assertEquals(SentryId.EMPTY_ID, profiler.profilerId) verify(fixture.mockLogger).log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler.")) 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 06de99258e6..9823c384042 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 @@ -5,6 +5,7 @@ import android.os.Bundle import androidx.core.os.bundleOf import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ILogger +import io.sentry.ProfileLifecycle import io.sentry.SentryLevel import io.sentry.SentryReplayOptions import org.junit.runner.RunWith @@ -829,6 +830,32 @@ class ManifestMetadataReaderTest { assertEquals(expectedSampleRate.toDouble(), fixture.options.profileSessionSampleRate) } + @Test + fun `applyMetadata without specifying profileLifecycle, stays MANUAL`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertEquals(ProfileLifecycle.MANUAL, fixture.options.profileLifecycle) + } + + @Test + fun `applyMetadata reads profileLifecycle from metadata`() { + // Arrange + val expectedLifecycle = "trace" + val bundle = bundleOf(ManifestMetadataReader.PROFILE_LIFECYCLE to expectedLifecycle) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertEquals(ProfileLifecycle.TRACE, fixture.options.profileLifecycle) + } + @Test fun `applyMetadata without specifying profileSessionSampleRate, stays 0`() { // Arrange diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 1c5988ac9e6..56d6ce19cd0 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -443,8 +443,10 @@ public abstract interface class io/sentry/EventProcessor { public final class io/sentry/ExperimentalOptions { public fun (Z)V + public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; public fun getProfileSessionSampleRate ()D public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; + public fun setProfileLifecycle (Lio/sentry/ProfileLifecycle;)V public fun setProfileSessionSampleRate (D)V public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V } @@ -725,8 +727,8 @@ public abstract interface class io/sentry/IContinuousProfiler { public abstract fun getProfilerId ()Lio/sentry/protocol/SentryId; public abstract fun isRunning ()Z public abstract fun reevaluateSampling ()V - public abstract fun start (Lio/sentry/TracesSampler;)V - public abstract fun stop ()V + public abstract fun startProfileSession (Lio/sentry/ProfileLifecycle;Lio/sentry/TracesSampler;)V + public abstract fun stopProfileSession (Lio/sentry/ProfileLifecycle;)V } public abstract interface class io/sentry/IEnvelopeReader { @@ -1409,8 +1411,8 @@ public final class io/sentry/NoOpContinuousProfiler : io/sentry/IContinuousProfi public fun getProfilerId ()Lio/sentry/protocol/SentryId; public fun isRunning ()Z public fun reevaluateSampling ()V - public fun start (Lio/sentry/TracesSampler;)V - public fun stop ()V + public fun startProfileSession (Lio/sentry/ProfileLifecycle;Lio/sentry/TracesSampler;)V + public fun stopProfileSession (Lio/sentry/ProfileLifecycle;)V } public final class io/sentry/NoOpEnvelopeReader : io/sentry/IEnvelopeReader { @@ -1920,6 +1922,13 @@ public final class io/sentry/ProfileContext$JsonKeys { public fun ()V } +public final class io/sentry/ProfileLifecycle : java/lang/Enum { + public static final field MANUAL Lio/sentry/ProfileLifecycle; + public static final field TRACE Lio/sentry/ProfileLifecycle; + public static fun valueOf (Ljava/lang/String;)Lio/sentry/ProfileLifecycle; + public static fun values ()[Lio/sentry/ProfileLifecycle; +} + public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public static final field TRUNCATION_REASON_BACKGROUNDED Ljava/lang/String; public static final field TRUNCATION_REASON_NORMAL Ljava/lang/String; @@ -2489,6 +2498,7 @@ public abstract interface class io/sentry/Sentry$OptionsConfiguration { public final class io/sentry/SentryAppStartProfilingOptions : io/sentry/JsonSerializable, io/sentry/JsonUnknown { public fun ()V + public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; public fun getProfileSampleRate ()Ljava/lang/Double; public fun getProfilingTracesDirPath ()Ljava/lang/String; public fun getProfilingTracesHz ()I @@ -2502,6 +2512,7 @@ public final class io/sentry/SentryAppStartProfilingOptions : io/sentry/JsonSeri public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V public fun setContinuousProfileSampled (Z)V public fun setContinuousProfilingEnabled (Z)V + public fun setProfileLifecycle (Lio/sentry/ProfileLifecycle;)V public fun setProfileSampleRate (Ljava/lang/Double;)V public fun setProfileSampled (Z)V public fun setProfilingEnabled (Z)V @@ -2522,6 +2533,7 @@ public final class io/sentry/SentryAppStartProfilingOptions$JsonKeys { public static final field CONTINUOUS_PROFILE_SAMPLED Ljava/lang/String; public static final field IS_CONTINUOUS_PROFILING_ENABLED Ljava/lang/String; public static final field IS_PROFILING_ENABLED Ljava/lang/String; + public static final field PROFILE_LIFECYCLE 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; @@ -2994,6 +3006,7 @@ public class io/sentry/SentryOptions { public fun getOptionsObservers ()Ljava/util/List; public fun getOutboxPath ()Ljava/lang/String; public fun getPerformanceCollectors ()Ljava/util/List; + public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; public fun getProfileSessionSampleRate ()D public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 5aaa68c0371..7ca690ec1b0 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -20,6 +20,12 @@ public final class ExperimentalOptions { */ private double profileSessionSampleRate = 0.0; + /** + * Whether the profiling lifecycle is controlled manually or based on the trace lifecycle. + * Defaults to {@link ProfileLifecycle#MANUAL}. + */ + private @NotNull ProfileLifecycle profileLifecycle = ProfileLifecycle.MANUAL; + public ExperimentalOptions(final boolean empty) { this.sessionReplay = new SentryReplayOptions(empty); } @@ -33,6 +39,27 @@ public void setSessionReplay(final @NotNull SentryReplayOptions sessionReplayOpt this.sessionReplay = sessionReplayOptions; } + /** + * Returns whether the profiling cycle is controlled manually or based on the trace lifecycle. + * Defaults to {@link ProfileLifecycle#MANUAL}. + * + * @return the profile lifecycle + */ + @ApiStatus.Experimental + @NotNull + public ProfileLifecycle getProfileLifecycle() { + return profileLifecycle; + } + + /** Sets the profiling lifecycle. */ + @ApiStatus.Experimental + public void setProfileLifecycle(final @NotNull ProfileLifecycle profileLifecycle) { + // TODO (when moved to SentryOptions): we should log a message if the user sets this to TRACE + // and tracing is disabled + this.profileLifecycle = profileLifecycle; + } + + @ApiStatus.Experimental public double getProfileSessionSampleRate() { return profileSessionSampleRate; } diff --git a/sentry/src/main/java/io/sentry/IContinuousProfiler.java b/sentry/src/main/java/io/sentry/IContinuousProfiler.java index bd37f6e14f5..68cd32813c0 100644 --- a/sentry/src/main/java/io/sentry/IContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/IContinuousProfiler.java @@ -9,9 +9,10 @@ public interface IContinuousProfiler { boolean isRunning(); - void start(final @NotNull TracesSampler tracesSampler); + void startProfileSession( + final @NotNull ProfileLifecycle profileLifecycle, final @NotNull TracesSampler tracesSampler); - void stop(); + void stopProfileSession(final @NotNull ProfileLifecycle profileLifecycle); /** Cancel the profiler and stops it. Used on SDK close. */ void close(); diff --git a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java index 597c3c5cfbb..2e788ad6d0a 100644 --- a/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java @@ -14,7 +14,7 @@ public static NoOpContinuousProfiler getInstance() { } @Override - public void stop() {} + public void stopProfileSession(final @NotNull ProfileLifecycle profileLifecycle) {} @Override public boolean isRunning() { @@ -22,7 +22,9 @@ public boolean isRunning() { } @Override - public void start(final @NotNull TracesSampler tracesSampler) {} + public void startProfileSession( + final @NotNull ProfileLifecycle profileLifecycle, + final @NotNull TracesSampler tracesSampler) {} @Override public void close() {} diff --git a/sentry/src/main/java/io/sentry/ProfileLifecycle.java b/sentry/src/main/java/io/sentry/ProfileLifecycle.java new file mode 100644 index 00000000000..42c26e5344b --- /dev/null +++ b/sentry/src/main/java/io/sentry/ProfileLifecycle.java @@ -0,0 +1,18 @@ +package io.sentry; + +/** + * Determines whether the profiling lifecycle is controlled manually or based on the trace + * lifecycle. + */ +public enum ProfileLifecycle { + /** + * Profiling is controlled manually. You must use the {@link Sentry#startProfileSession()} and + * {@link Sentry#stopProfileSession()} APIs to control the lifecycle of the profiler. + */ + MANUAL, + /** + * Profiling is automatically started when there is at least 1 sampled root span, and it's + * automatically stopped when there are none. + */ + TRACE +} diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index c57a86b264f..be1b63676ee 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -905,15 +905,27 @@ public void flush(long timeoutMillis) { // The listener is called only if the transaction exists, as the transaction is needed to // stop it - if (samplingDecision.getSampled() && samplingDecision.getProfileSampled()) { - final ITransactionProfiler transactionProfiler = getOptions().getTransactionProfiler(); - // If the profiler is not running, we start and bind it here. - if (!transactionProfiler.isRunning()) { - transactionProfiler.start(); - transactionProfiler.bindTransaction(transaction); - } else if (transactionOptions.isAppStartTransaction()) { - // If the profiler is running and the current transaction is the app start, we bind it. - transactionProfiler.bindTransaction(transaction); + if (samplingDecision.getSampled()) { + // If transaction profiler is sampled, let's start it + if (samplingDecision.getProfileSampled()) { + final ITransactionProfiler transactionProfiler = getOptions().getTransactionProfiler(); + // If the profiler is not running, we start and bind it here. + if (!transactionProfiler.isRunning()) { + transactionProfiler.start(); + transactionProfiler.bindTransaction(transaction); + } else if (transactionOptions.isAppStartTransaction()) { + // If the profiler is running and the current transaction is the app start, we bind it. + transactionProfiler.bindTransaction(transaction); + } + } + + // If continuous profiling is enabled in trace mode, let's start it. Profiler will sample on + // its own. + if (getOptions().isContinuousProfilingEnabled() + && getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE) { + getOptions() + .getContinuousProfiler() + .startProfileSession(ProfileLifecycle.TRACE, getOptions().getInternalTracesSampler()); } } } @@ -926,7 +938,18 @@ public void flush(long timeoutMillis) { @Override public void startProfileSession() { if (getOptions().isContinuousProfilingEnabled()) { - getOptions().getContinuousProfiler().start(getOptions().getInternalTracesSampler()); + if (getOptions().getProfileLifecycle() != ProfileLifecycle.MANUAL) { + getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Profiling lifecycle is %s. Profiling cannot be started manually.", + getOptions().getProfileLifecycle().name()); + return; + } + getOptions() + .getContinuousProfiler() + .startProfileSession(ProfileLifecycle.MANUAL, getOptions().getInternalTracesSampler()); } else if (getOptions().isProfilingEnabled()) { getOptions() .getLogger() @@ -939,8 +962,17 @@ public void startProfileSession() { @Override public void stopProfileSession() { if (getOptions().isContinuousProfilingEnabled()) { + if (getOptions().getProfileLifecycle() != ProfileLifecycle.MANUAL) { + getOptions() + .getLogger() + .log( + SentryLevel.WARNING, + "Profiling lifecycle is %s. Profiling cannot be stopped manually.", + getOptions().getProfileLifecycle().name()); + return; + } getOptions().getLogger().log(SentryLevel.DEBUG, "Stopped continuous Profiling."); - getOptions().getContinuousProfiler().stop(); + getOptions().getContinuousProfiler().stopProfileSession(ProfileLifecycle.MANUAL); } else { getOptions() .getLogger() diff --git a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java index 364a71e5cad..e764a42218d 100644 --- a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java @@ -21,6 +21,7 @@ public final class SentryAppStartProfilingOptions implements JsonUnknown, JsonSe boolean isContinuousProfilingEnabled; int profilingTracesHz; boolean continuousProfileSampled; + @NotNull ProfileLifecycle profileLifecycle; private @Nullable Map unknown; @@ -34,6 +35,7 @@ public SentryAppStartProfilingOptions() { profilingTracesDirPath = null; isProfilingEnabled = false; isContinuousProfilingEnabled = false; + profileLifecycle = ProfileLifecycle.MANUAL; profilingTracesHz = 0; } @@ -48,6 +50,7 @@ public SentryAppStartProfilingOptions() { profilingTracesDirPath = options.getProfilingTracesDirPath(); isProfilingEnabled = options.isProfilingEnabled(); isContinuousProfilingEnabled = options.isContinuousProfilingEnabled(); + profileLifecycle = options.getProfileLifecycle(); profilingTracesHz = options.getProfilingTracesHz(); } @@ -67,6 +70,14 @@ public boolean isContinuousProfileSampled() { return continuousProfileSampled; } + public void setProfileLifecycle(final @NotNull ProfileLifecycle profileLifecycle) { + this.profileLifecycle = profileLifecycle; + } + + public @NotNull ProfileLifecycle getProfileLifecycle() { + return profileLifecycle; + } + public void setProfileSampleRate(final @Nullable Double profileSampleRate) { this.profileSampleRate = profileSampleRate; } @@ -134,6 +145,7 @@ public static final class JsonKeys { 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 IS_CONTINUOUS_PROFILING_ENABLED = "is_continuous_profiling_enabled"; + public static final String PROFILE_LIFECYCLE = "profile_lifecycle"; public static final String PROFILING_TRACES_HZ = "profiling_traces_hz"; } @@ -151,6 +163,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger writer .name(JsonKeys.IS_CONTINUOUS_PROFILING_ENABLED) .value(logger, isContinuousProfilingEnabled); + writer.name(JsonKeys.PROFILE_LIFECYCLE).value(logger, profileLifecycle.name()); writer.name(JsonKeys.PROFILING_TRACES_HZ).value(logger, profilingTracesHz); if (unknown != null) { @@ -235,6 +248,18 @@ public static final class Deserializer options.isContinuousProfilingEnabled = isContinuousProfilingEnabled; } break; + case JsonKeys.PROFILE_LIFECYCLE: + String profileLifecycle = reader.nextStringOrNull(); + if (profileLifecycle != null) { + try { + options.profileLifecycle = ProfileLifecycle.valueOf(profileLifecycle); + } catch (IllegalArgumentException e) { + logger.log( + SentryLevel.ERROR, + "Error when deserializing ProfileLifecycle: " + profileLifecycle); + } + } + break; case JsonKeys.PROFILING_TRACES_HZ: Integer profilingTracesHz = reader.nextIntegerOrNull(); if (profilingTracesHz != null) { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 717a039c455..590c7a33ebe 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1801,6 +1801,17 @@ public double getProfileSessionSampleRate() { return experimental.getProfileSessionSampleRate(); } + /** + * Returns whether the profiling lifecycle is controlled manually or based on the trace lifecycle. + * Defaults to {@link ProfileLifecycle#MANUAL}. + * + * @return the profile lifecycle + */ + @ApiStatus.Experimental + public ProfileLifecycle getProfileLifecycle() { + return experimental.getProfileLifecycle(); + } + /** * Returns the profiling traces dir. path if set * diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 943dc349f2e..13d59c58818 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -90,7 +90,7 @@ public SentryTracer( final @NotNull SentryId continuousProfilerId = scopes.getOptions().getContinuousProfiler().getProfilerId(); - if (!continuousProfilerId.equals(SentryId.EMPTY_ID)) { + if (!continuousProfilerId.equals(SentryId.EMPTY_ID) && Boolean.TRUE.equals(isSampled())) { this.contexts.setProfile(new ProfileContext(continuousProfilerId)); } @@ -246,6 +246,10 @@ public void finish( .getTransactionProfiler() .onTransactionFinish(this, performanceCollectionData.get(), scopes.getOptions()); } + if (scopes.getOptions().isContinuousProfilingEnabled() + && scopes.getOptions().getProfileLifecycle() == ProfileLifecycle.TRACE) { + scopes.getOptions().getContinuousProfiler().stopProfileSession(ProfileLifecycle.TRACE); + } if (performanceCollectionData.get() != null) { performanceCollectionData.get().clear(); } @@ -522,7 +526,7 @@ private ISpan createChild( // span.setDescription(description); final @NotNull IThreadChecker threadChecker = scopes.getOptions().getThreadChecker(); final SentryId profilerId = scopes.getOptions().getContinuousProfiler().getProfilerId(); - if (!profilerId.equals(SentryId.EMPTY_ID)) { + if (!profilerId.equals(SentryId.EMPTY_ID) && Boolean.TRUE.equals(span.isSampled())) { span.setData(SpanDataConvention.PROFILER_ID, profilerId.toString()); } span.setData( diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 0b63116ee64..a8f1a1e80c5 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -1233,8 +1233,8 @@ class JsonSerializerTest { val actual = serializeToString(appStartProfilingOptions) val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"continuous_profile_sampled\":true," + - "\"trace_sampled\":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null," + - "\"is_profiling_enabled\":false,\"is_continuous_profiling_enabled\":false,\"profiling_traces_hz\":65}" + "\"trace_sampled\":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false," + + "\"is_continuous_profiling_enabled\":false,\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65}" assertEquals(expected, actual) } @@ -1243,7 +1243,7 @@ class JsonSerializerTest { fun `deserializing SentryAppStartProfilingOptions`() { val jsonAppStartProfilingOptions = "{\"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," + - "\"profiling_traces_hz\":65,\"continuous_profile_sampled\":true}" + "\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65,\"continuous_profile_sampled\":true}" val actual = fixture.serializer.deserialize(StringReader(jsonAppStartProfilingOptions), SentryAppStartProfilingOptions::class.java) assertNotNull(actual) @@ -1256,6 +1256,7 @@ class JsonSerializerTest { assertEquals(appStartProfilingOptions.isContinuousProfilingEnabled, actual.isContinuousProfilingEnabled) assertEquals(appStartProfilingOptions.profilingTracesHz, actual.profilingTracesHz) assertEquals(appStartProfilingOptions.profilingTracesDirPath, actual.profilingTracesDirPath) + assertEquals(appStartProfilingOptions.profileLifecycle, actual.profileLifecycle) assertNull(actual.unknown) } @@ -1560,6 +1561,7 @@ class JsonSerializerTest { isProfilingEnabled = false isContinuousProfilingEnabled = false profilingTracesHz = 65 + profileLifecycle = ProfileLifecycle.TRACE } private fun createSpan(): ISpan { diff --git a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt index 30ab1d090a5..081c72169d6 100644 --- a/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt @@ -11,11 +11,11 @@ class NoOpContinuousProfilerTest { @Test fun `start does not throw`() = - profiler.start(mock()) + profiler.startProfileSession(mock(), mock()) @Test fun `stop does not throw`() = - profiler.stop() + profiler.stopProfileSession(mock()) @Test fun `isRunning returns false`() { diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 9cb8ca177af..e1e61279c24 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -53,6 +53,7 @@ class ScopesTest { private lateinit var file: File private lateinit var profilingTraceFile: File + private val mockProfiler = spy(NoOpContinuousProfiler.getInstance()) @BeforeTest fun `set up`() { @@ -772,6 +773,8 @@ class ScopesTest { } } + //endregion + //region captureCheckIn tests @Test @@ -1870,6 +1873,49 @@ class ScopesTest { val transaction = scopes.startTransaction(TransactionContext("name", "op", TracesSamplingDecision(true))) assertTrue(transaction is NoOpTransaction) } + + @Test + fun `when startTransaction, trace profile session is started`() { + val scopes = generateScopes { + it.tracesSampleRate = 1.0 + it.setContinuousProfiler(mockProfiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.TRACE + } + + val transaction = scopes.startTransaction("name", "op") + assertTrue(transaction.isSampled!!) + verify(mockProfiler).startProfileSession(eq(ProfileLifecycle.TRACE), any()) + } + + @Test + fun `when startTransaction, manual profile session is not started`() { + val scopes = generateScopes { + it.tracesSampleRate = 1.0 + it.setContinuousProfiler(mockProfiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.MANUAL + } + + val transaction = scopes.startTransaction("name", "op") + assertTrue(transaction.isSampled!!) + verify(mockProfiler, never()).startProfileSession(any(), any()) + } + + @Test + fun `when startTransaction not sampled, trace profile session is not started`() { + val scopes = generateScopes { + // If transaction is not sampled, profiler should not start + it.tracesSampleRate = 0.0 + it.setContinuousProfiler(mockProfiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.TRACE + } + val transaction = scopes.startTransaction("name", "op") + transaction.spanContext.setSampled(false, false) + assertFalse(transaction.isSampled!!) + verify(mockProfiler, never()).startProfileSession(any(), any()) + } //endregion //region getSpan tests @@ -2161,6 +2207,8 @@ class ScopesTest { assertEquals("other.span.origin", transaction.spanContext.origin) } + //region profileSession + @Test fun `startProfileSession starts the continuous profiler`() { val profiler = mock() @@ -2169,7 +2217,7 @@ class ScopesTest { it.experimental.profileSessionSampleRate = 1.0 } scopes.startProfileSession() - verify(profiler).start(any()) + verify(profiler).startProfileSession(eq(ProfileLifecycle.MANUAL), any()) } @Test @@ -2184,10 +2232,26 @@ class ScopesTest { it.isDebug = true } scopes.startProfileSession() - verify(profiler, never()).start(any()) + verify(profiler, never()).startProfileSession(eq(ProfileLifecycle.MANUAL), any()) verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) } + @Test + fun `startProfileSession is ignored on trace lifecycle`() { + val profiler = mock() + val logger = mock() + val scopes = generateScopes { + it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.TRACE + it.setLogger(logger) + it.isDebug = true + } + scopes.startProfileSession() + verify(logger).log(eq(SentryLevel.WARNING), eq("Profiling lifecycle is %s. Profiling cannot be started manually."), eq(ProfileLifecycle.TRACE.name)) + verify(profiler, never()).startProfileSession(any(), any()) + } + @Test fun `stopProfileSession stops the continuous profiler`() { val profiler = mock() @@ -2196,7 +2260,7 @@ class ScopesTest { it.experimental.profileSessionSampleRate = 1.0 } scopes.stopProfileSession() - verify(profiler).stop() + verify(profiler).stopProfileSession(eq(ProfileLifecycle.MANUAL)) } @Test @@ -2211,10 +2275,28 @@ class ScopesTest { it.isDebug = true } scopes.stopProfileSession() - verify(profiler, never()).stop() + verify(profiler, never()).stopProfileSession(eq(ProfileLifecycle.MANUAL)) verify(logger).log(eq(SentryLevel.WARNING), eq("Continuous Profiling is not enabled. Set profilesSampleRate and profilesSampler to null to enable it.")) } + @Test + fun `stopProfileSession is ignored on trace lifecycle`() { + val profiler = mock() + val logger = mock() + val scopes = generateScopes { + it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.TRACE + it.setLogger(logger) + it.isDebug = true + } + scopes.stopProfileSession() + verify(logger).log(eq(SentryLevel.WARNING), eq("Profiling lifecycle is %s. Profiling cannot be stopped manually."), eq(ProfileLifecycle.TRACE.name)) + verify(profiler, never()).stopProfileSession(any()) + } + + //endregion + private val dsnTest = "https://key@sentry.io/proj" private fun generateScopes(optionsConfiguration: Sentry.OptionsConfiguration? = null): IScopes { diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index c5cbf6b11f4..084d745f9d8 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -281,6 +281,20 @@ class SentryOptionsTest { assertFailsWith { SentryOptions().experimental.profileSessionSampleRate = -0.0000000000001 } } + @Test + fun `when profileLifecycleSessionSampleRate is set to a value, value is set`() { + val options = SentryOptions().apply { + this.experimental.profileLifecycle = ProfileLifecycle.TRACE + } + assertEquals(ProfileLifecycle.TRACE, options.profileLifecycle) + } + + @Test + fun `profileLifecycleSessionSampleRate defaults to MANUAL`() { + val options = SentryOptions() + assertEquals(ProfileLifecycle.MANUAL, options.profileLifecycle) + } + @Test fun `when options is initialized, compositePerformanceCollector is set`() { assertIs(SentryOptions().compositePerformanceCollector) diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index ef337dff90f..cf7f7384f89 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -1315,7 +1315,7 @@ class SentryTest { it.experimental.profileSessionSampleRate = 1.0 } Sentry.startProfileSession() - verify(profiler).start(any()) + verify(profiler).startProfileSession(eq(ProfileLifecycle.MANUAL), any()) } @Test @@ -1327,7 +1327,28 @@ class SentryTest { it.profilesSampleRate = 1.0 } Sentry.startProfileSession() - verify(profiler, never()).start(any()) + verify(profiler, never()).startProfileSession(eq(ProfileLifecycle.MANUAL), any()) + } + + @Test + fun `startProfileSession is ignored when profile lifecycle is TRACE`() { + val profiler = mock() + val logger = mock() + Sentry.init { + it.dsn = dsn + it.setContinuousProfiler(profiler) + it.experimental.profileSessionSampleRate = 1.0 + it.experimental.profileLifecycle = ProfileLifecycle.TRACE + it.isDebug = true + it.setLogger(logger) + } + Sentry.startProfileSession() + verify(profiler, never()).startProfileSession(any(), any()) + verify(logger).log( + eq(SentryLevel.WARNING), + eq("Profiling lifecycle is %s. Profiling cannot be started manually."), + eq(ProfileLifecycle.TRACE.name) + ) } @Test @@ -1339,7 +1360,7 @@ class SentryTest { it.experimental.profileSessionSampleRate = 1.0 } Sentry.stopProfileSession() - verify(profiler).stop() + verify(profiler).stopProfileSession(eq(ProfileLifecycle.MANUAL)) } @Test @@ -1351,7 +1372,7 @@ class SentryTest { it.profilesSampleRate = 1.0 } Sentry.stopProfileSession() - verify(profiler, never()).stop() + verify(profiler, never()).stopProfileSession(eq(ProfileLifecycle.MANUAL)) } private class InMemoryOptionsObserver : IOptionsObserver { diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 2be549cba6c..ed0c3bdc90e 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -220,7 +220,7 @@ class SentryTracerTest { whenever(continuousProfiler.profilerId).thenReturn(profilerId) val tracer = fixture.getSut(optionsConfiguration = { it.setContinuousProfiler(continuousProfiler) - }) + }, samplingDecision = TracesSamplingDecision(true)) tracer.finish() verify(fixture.scopes).captureTransaction( check { @@ -234,6 +234,42 @@ class SentryTracerTest { ) } + @Test + fun `when transaction is not sampled, profile context is not set`() { + val continuousProfiler = mock() + val profilerId = SentryId() + whenever(continuousProfiler.profilerId).thenReturn(profilerId) + val tracer = fixture.getSut(optionsConfiguration = { + it.setContinuousProfiler(continuousProfiler) + }, samplingDecision = TracesSamplingDecision(false)) + tracer.finish() + // profiler is never stopped, as it was never started + verify(continuousProfiler, never()).stopProfileSession(any()) + // profile context is not set + verify(fixture.scopes).captureTransaction( + check { + assertNull(it.contexts.profile) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when continuous profiler is running in MANUAL mode, profiler is not stopped on transaction finish`() { + val continuousProfiler = mock() + val profilerId = SentryId() + whenever(continuousProfiler.profilerId).thenReturn(profilerId) + val tracer = fixture.getSut(optionsConfiguration = { + it.setContinuousProfiler(continuousProfiler) + it.experimental.profileLifecycle = ProfileLifecycle.MANUAL + }, samplingDecision = TracesSamplingDecision(true)) + tracer.finish() + // profiler is never stopped, as it should be stopped manually + verify(continuousProfiler, never()).stopProfileSession(any()) + } + @Test fun `when continuous profiler is not running, profile context is not set`() { val tracer = fixture.getSut(optionsConfiguration = { @@ -258,11 +294,24 @@ class SentryTracerTest { val tracer = fixture.getSut(optionsConfiguration = { options -> options.setContinuousProfiler(profiler) - }) + }, samplingDecision = TracesSamplingDecision(true)) val span = tracer.startChild("span.op") assertEquals(profilerId.toString(), span.getData(SpanDataConvention.PROFILER_ID)) } + @Test + fun `when transaction is not sampled, profiler id is NOT set in span data`() { + val profilerId = SentryId() + val profiler = mock() + whenever(profiler.profilerId).thenReturn(profilerId) + + val tracer = fixture.getSut(optionsConfiguration = { options -> + options.setContinuousProfiler(profiler) + }, samplingDecision = TracesSamplingDecision(false)) + val span = tracer.startChild("span.op") + assertNull(span.getData(SpanDataConvention.PROFILER_ID)) + } + @Test fun `when continuous profiler is not running, profiler id is not set in span data`() { val tracer = fixture.getSut(optionsConfiguration = { options -> From 79742ca49b8111fc973e8ca3c881b5bae339d856 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 27 Feb 2025 18:45:59 +0100 Subject: [PATCH 10/15] merged base branch --- sentry/api/sentry.api | 8 ++++---- sentry/src/main/java/io/sentry/ExperimentalOptions.java | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 56d6ce19cd0..70f60b8350c 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -444,10 +444,10 @@ public abstract interface class io/sentry/EventProcessor { public final class io/sentry/ExperimentalOptions { public fun (Z)V public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; - public fun getProfileSessionSampleRate ()D + public fun getProfileSessionSampleRate ()Ljava/lang/Double; public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; public fun setProfileLifecycle (Lio/sentry/ProfileLifecycle;)V - public fun setProfileSessionSampleRate (D)V + public fun setProfileSessionSampleRate (Ljava/lang/Double;)V public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V } @@ -3007,7 +3007,7 @@ public class io/sentry/SentryOptions { public fun getOutboxPath ()Ljava/lang/String; public fun getPerformanceCollectors ()Ljava/util/List; public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; - public fun getProfileSessionSampleRate ()D + public fun getProfileSessionSampleRate ()Ljava/lang/Double; public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; public fun getProfilingTracesDirPath ()Ljava/lang/String; @@ -6340,7 +6340,7 @@ public final class io/sentry/util/Random : java/io/Serializable { public final class io/sentry/util/SampleRateUtils { public fun ()V - public static fun isValidContinuousProfilesSampleRate (D)Z + public static fun isValidContinuousProfilesSampleRate (Ljava/lang/Double;)Z public static fun isValidProfilesSampleRate (Ljava/lang/Double;)Z public static fun isValidSampleRate (Ljava/lang/Double;)Z public static fun isValidTracesSampleRate (Ljava/lang/Double;)Z diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index c13c98e1a8f..4a99c003778 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -47,8 +47,7 @@ public void setSessionReplay(final @NotNull SentryReplayOptions sessionReplayOpt * @return the profile lifecycle */ @ApiStatus.Experimental - @NotNull - public ProfileLifecycle getProfileLifecycle() { + public @NotNull ProfileLifecycle getProfileLifecycle() { return profileLifecycle; } From dd735c2ce052ac995236142e4f1cb40fe423ed9e Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 28 Feb 2025 17:22:05 +0100 Subject: [PATCH 11/15] added isStartProfilerOnAppStart experimental option --- .../android/core/ManifestMetadataReader.java | 6 +++++ .../core/ManifestMetadataReaderTest.kt | 25 +++++++++++++++++++ .../java/io/sentry/ExperimentalOptions.java | 17 +++++++++++++ sentry/src/main/java/io/sentry/Sentry.java | 7 +++--- .../main/java/io/sentry/SentryOptions.java | 6 +++++ .../test/java/io/sentry/SentryOptionsTest.kt | 14 +++++++++++ 6 files changed, 72 insertions(+), 3 deletions(-) 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 e85e5de9234..39eed498aab 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 @@ -70,6 +70,8 @@ final class ManifestMetadataReader { static final String PROFILE_LIFECYCLE = "io.sentry.traces.profiling.lifecycle"; + static final String PROFILER_START_ON_APP_START = "io.sentry.traces.profiling.start-on-app-start"; + @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets"; @@ -342,6 +344,10 @@ static void applyMetadata( ProfileLifecycle.valueOf(profileLifecycle.toUpperCase(Locale.ROOT))); } + options.getExperimental().setStartProfilerOnAppStart( + readBool( + metadata, logger, PROFILER_START_ON_APP_START, options.isStartProfilerOnAppStart())); + options.setEnableUserInteractionTracing( readBool(metadata, logger, TRACES_UI_ENABLE, options.isEnableUserInteractionTracing())); 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 9190beb916d..40143a74c3c 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 @@ -868,6 +868,31 @@ class ManifestMetadataReaderTest { assertEquals(ProfileLifecycle.TRACE, fixture.options.profileLifecycle) } + @Test + fun `applyMetadata without specifying isStartProfilerOnAppStart, stays false`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertFalse(fixture.options.isStartProfilerOnAppStart) + } + + @Test + fun `applyMetadata reads isStartProfilerOnAppStart from metadata`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.PROFILER_START_ON_APP_START to true) + val context = fixture.getContext(metaData = bundle) + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertTrue(fixture.options.isStartProfilerOnAppStart) + } + @Test fun `applyMetadata reads tracePropagationTargets to options`() { // Arrange diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 4a99c003778..00954b6bac2 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -27,6 +27,13 @@ public final class ExperimentalOptions { */ private @NotNull ProfileLifecycle profileLifecycle = ProfileLifecycle.MANUAL; + /** + * Whether profiling can automatically be started as early as possible during the app lifecycle, to capture more of app startup. + * If {@link ExperimentalOptions#profileLifecycle} is {@link ProfileLifecycle#MANUAL} Profiling is started automatically on startup and stopProfileSession must be called manually whenever the app startup is completed + * If {@link ExperimentalOptions#profileLifecycle} is {@link ProfileLifecycle#TRACE} Profiling is started automatically on startup, and will automatically be stopped when the root span that is associated with app startup ends + */ + private boolean startProfilerOnAppStart = false; + public ExperimentalOptions(final boolean empty) { this.sessionReplay = new SentryReplayOptions(empty); } @@ -74,4 +81,14 @@ public void setProfileSessionSampleRate(final @Nullable Double profileSessionSam } this.profileSessionSampleRate = profileSessionSampleRate; } + + @ApiStatus.Experimental + public boolean isStartProfilerOnAppStart() { + return startProfilerOnAppStart; + } + + @ApiStatus.Experimental + public void setStartProfilerOnAppStart(boolean startProfilerOnAppStart) { + this.startProfilerOnAppStart = startProfilerOnAppStart; + } } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 02e7d168c79..cb2f32350e3 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -370,9 +370,10 @@ private static void handleAppStartProfilingConfig( try { // We always delete the config file for app start profiling FileUtils.deleteRecursively(appStartProfilingConfigFile); - if (!options.isEnableAppStartProfiling()) { + if (!options.isEnableAppStartProfiling() && !options.isStartProfilerOnAppStart()) { return; } + todo isStartProfilerOnAppStart doesn't need tracing! - SentryTest takes hours to run! if (!options.isTracingEnabled()) { options .getLogger() @@ -382,8 +383,8 @@ private static void handleAppStartProfilingConfig( return; } if (appStartProfilingConfigFile.createNewFile()) { - final @NotNull TracesSamplingDecision appStartSamplingDecision = - sampleAppStartProfiling(options); + // If old app start profiling is false, it means the transaction will not be sampled, but we create the file anyway to allow continuous profiling on app start + final @NotNull TracesSamplingDecision appStartSamplingDecision = options.isEnableAppStartProfiling() ? sampleAppStartProfiling(options) : new TracesSamplingDecision(false); final @NotNull SentryAppStartProfilingOptions appStartProfilingOptions = new SentryAppStartProfilingOptions(options, appStartSamplingDecision); try (final OutputStream outputStream = diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index b7b57f4bebd..538bbc0ff0c 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1813,6 +1813,12 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { return experimental.getProfileLifecycle(); } + /** Whether profiling can automatically be started as early as possible during the app lifecycle. */ + @ApiStatus.Experimental + public boolean isStartProfilerOnAppStart() { + return experimental.isStartProfilerOnAppStart(); + } + /** * Returns the profiling traces dir. path if set * diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 22aec0c9a08..fc850e4bd4d 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -303,6 +303,20 @@ class SentryOptionsTest { assertEquals(ProfileLifecycle.MANUAL, options.profileLifecycle) } + @Test + fun `when isStartProfilerOnAppStart is set to a value, value is set`() { + val options = SentryOptions().apply { + this.experimental.isStartProfilerOnAppStart = true + } + assertTrue(options.isStartProfilerOnAppStart) + } + + @Test + fun `isStartProfilerOnAppStart defaults to false`() { + val options = SentryOptions() + assertFalse(options.isStartProfilerOnAppStart) + } + @Test fun `when options is initialized, compositePerformanceCollector is set`() { assertIs(SentryOptions().compositePerformanceCollector) From d790035377fa3c2ebaef64ced3ab7e20891db237 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 4 Mar 2025 13:19:19 +0100 Subject: [PATCH 12/15] added isStartProfilerOnAppStart logic and tests --- .../android/core/ManifestMetadataReader.java | 11 +++-- sentry/api/sentry.api | 9 ++++ .../java/io/sentry/ExperimentalOptions.java | 10 +++-- sentry/src/main/java/io/sentry/Sentry.java | 14 +++++-- .../SentryAppStartProfilingOptions.java | 38 +++++++++++++++++ .../main/java/io/sentry/SentryOptions.java | 4 +- .../test/java/io/sentry/JsonSerializerTest.kt | 11 +++-- sentry/src/test/java/io/sentry/SentryTest.kt | 41 ++++++++++++++++++- 8 files changed, 123 insertions(+), 15 deletions(-) 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 39eed498aab..a01ae5a83ff 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 @@ -344,9 +344,14 @@ static void applyMetadata( ProfileLifecycle.valueOf(profileLifecycle.toUpperCase(Locale.ROOT))); } - options.getExperimental().setStartProfilerOnAppStart( - readBool( - metadata, logger, PROFILER_START_ON_APP_START, options.isStartProfilerOnAppStart())); + options + .getExperimental() + .setStartProfilerOnAppStart( + readBool( + metadata, + logger, + PROFILER_START_ON_APP_START, + options.isStartProfilerOnAppStart())); options.setEnableUserInteractionTracing( readBool(metadata, logger, TRACES_UI_ENABLE, options.isEnableUserInteractionTracing())); diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 70f60b8350c..31a4d7d6da8 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -446,9 +446,11 @@ public final class io/sentry/ExperimentalOptions { public fun getProfileLifecycle ()Lio/sentry/ProfileLifecycle; public fun getProfileSessionSampleRate ()Ljava/lang/Double; public fun getSessionReplay ()Lio/sentry/SentryReplayOptions; + public fun isStartProfilerOnAppStart ()Z public fun setProfileLifecycle (Lio/sentry/ProfileLifecycle;)V public fun setProfileSessionSampleRate (Ljava/lang/Double;)V public fun setSessionReplay (Lio/sentry/SentryReplayOptions;)V + public fun setStartProfilerOnAppStart (Z)V } public final class io/sentry/ExternalOptions { @@ -2506,18 +2508,22 @@ public final class io/sentry/SentryAppStartProfilingOptions : io/sentry/JsonSeri public fun getUnknown ()Ljava/util/Map; public fun isContinuousProfileSampled ()Z public fun isContinuousProfilingEnabled ()Z + public fun isEnableAppStartProfiling ()Z public fun isProfileSampled ()Z public fun isProfilingEnabled ()Z + public fun isStartProfilerOnAppStart ()Z public fun isTraceSampled ()Z public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V public fun setContinuousProfileSampled (Z)V public fun setContinuousProfilingEnabled (Z)V + public fun setEnableAppStartProfiling (Z)V public fun setProfileLifecycle (Lio/sentry/ProfileLifecycle;)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 setStartProfilerOnAppStart (Z)V public fun setTraceSampleRate (Ljava/lang/Double;)V public fun setTraceSampled (Z)V public fun setUnknown (Ljava/util/Map;)V @@ -2532,7 +2538,9 @@ public final class io/sentry/SentryAppStartProfilingOptions$Deserializer : io/se public final class io/sentry/SentryAppStartProfilingOptions$JsonKeys { public static final field CONTINUOUS_PROFILE_SAMPLED Ljava/lang/String; public static final field IS_CONTINUOUS_PROFILING_ENABLED Ljava/lang/String; + public static final field IS_ENABLE_APP_START_PROFILING Ljava/lang/String; public static final field IS_PROFILING_ENABLED Ljava/lang/String; + public static final field IS_START_PROFILER_ON_APP_START Ljava/lang/String; public static final field PROFILE_LIFECYCLE Ljava/lang/String; public static final field PROFILE_SAMPLED Ljava/lang/String; public static final field PROFILE_SAMPLE_RATE Ljava/lang/String; @@ -3065,6 +3073,7 @@ public class io/sentry/SentryOptions { public fun isSendClientReports ()Z public fun isSendDefaultPii ()Z public fun isSendModules ()Z + public fun isStartProfilerOnAppStart ()Z public fun isTraceOptionsRequests ()Z public fun isTraceSampling ()Z public fun isTracingEnabled ()Z diff --git a/sentry/src/main/java/io/sentry/ExperimentalOptions.java b/sentry/src/main/java/io/sentry/ExperimentalOptions.java index 00954b6bac2..fc93416d02d 100644 --- a/sentry/src/main/java/io/sentry/ExperimentalOptions.java +++ b/sentry/src/main/java/io/sentry/ExperimentalOptions.java @@ -28,9 +28,13 @@ public final class ExperimentalOptions { private @NotNull ProfileLifecycle profileLifecycle = ProfileLifecycle.MANUAL; /** - * Whether profiling can automatically be started as early as possible during the app lifecycle, to capture more of app startup. - * If {@link ExperimentalOptions#profileLifecycle} is {@link ProfileLifecycle#MANUAL} Profiling is started automatically on startup and stopProfileSession must be called manually whenever the app startup is completed - * If {@link ExperimentalOptions#profileLifecycle} is {@link ProfileLifecycle#TRACE} Profiling is started automatically on startup, and will automatically be stopped when the root span that is associated with app startup ends + * Whether profiling can automatically be started as early as possible during the app lifecycle, + * to capture more of app startup. If {@link ExperimentalOptions#profileLifecycle} is {@link + * ProfileLifecycle#MANUAL} Profiling is started automatically on startup and stopProfileSession + * must be called manually whenever the app startup is completed If {@link + * ExperimentalOptions#profileLifecycle} is {@link ProfileLifecycle#TRACE} Profiling is started + * automatically on startup, and will automatically be stopped when the root span that is + * associated with app startup ends */ private boolean startProfilerOnAppStart = false; diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index cb2f32350e3..a3ecbacf8d0 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -373,8 +373,9 @@ private static void handleAppStartProfilingConfig( if (!options.isEnableAppStartProfiling() && !options.isStartProfilerOnAppStart()) { return; } - todo isStartProfilerOnAppStart doesn't need tracing! - SentryTest takes hours to run! - if (!options.isTracingEnabled()) { + // isStartProfilerOnAppStart doesn't need tracing, as it can be started/stopped + // manually + if (!options.isStartProfilerOnAppStart() && !options.isTracingEnabled()) { options .getLogger() .log( @@ -383,8 +384,13 @@ private static void handleAppStartProfilingConfig( return; } if (appStartProfilingConfigFile.createNewFile()) { - // If old app start profiling is false, it means the transaction will not be sampled, but we create the file anyway to allow continuous profiling on app start - final @NotNull TracesSamplingDecision appStartSamplingDecision = options.isEnableAppStartProfiling() ? sampleAppStartProfiling(options) : new TracesSamplingDecision(false); + // If old app start profiling is false, it means the transaction will not be + // sampled, but we create the file anyway to allow continuous profiling on app + // start + final @NotNull TracesSamplingDecision appStartSamplingDecision = + options.isEnableAppStartProfiling() + ? sampleAppStartProfiling(options) + : new TracesSamplingDecision(false); final @NotNull SentryAppStartProfilingOptions appStartProfilingOptions = new SentryAppStartProfilingOptions(options, appStartSamplingDecision); try (final OutputStream outputStream = diff --git a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java index e764a42218d..75ab308af1f 100644 --- a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java @@ -21,6 +21,8 @@ public final class SentryAppStartProfilingOptions implements JsonUnknown, JsonSe boolean isContinuousProfilingEnabled; int profilingTracesHz; boolean continuousProfileSampled; + boolean isEnableAppStartProfiling; + boolean isStartProfilerOnAppStart; @NotNull ProfileLifecycle profileLifecycle; private @Nullable Map unknown; @@ -37,6 +39,8 @@ public SentryAppStartProfilingOptions() { isContinuousProfilingEnabled = false; profileLifecycle = ProfileLifecycle.MANUAL; profilingTracesHz = 0; + isEnableAppStartProfiling = true; + isStartProfilerOnAppStart = false; } SentryAppStartProfilingOptions( @@ -52,6 +56,8 @@ public SentryAppStartProfilingOptions() { isContinuousProfilingEnabled = options.isContinuousProfilingEnabled(); profileLifecycle = options.getProfileLifecycle(); profilingTracesHz = options.getProfilingTracesHz(); + isEnableAppStartProfiling = options.isEnableAppStartProfiling(); + isStartProfilerOnAppStart = options.isStartProfilerOnAppStart(); } public void setProfileSampled(final boolean profileSampled) { @@ -134,6 +140,22 @@ public int getProfilingTracesHz() { return profilingTracesHz; } + public void setEnableAppStartProfiling(final boolean enableAppStartProfiling) { + isEnableAppStartProfiling = enableAppStartProfiling; + } + + public boolean isEnableAppStartProfiling() { + return isEnableAppStartProfiling; + } + + public void setStartProfilerOnAppStart(final boolean startProfilerOnAppStart) { + isStartProfilerOnAppStart = startProfilerOnAppStart; + } + + public boolean isStartProfilerOnAppStart() { + return isStartProfilerOnAppStart; + } + // JsonSerializable public static final class JsonKeys { @@ -147,6 +169,8 @@ public static final class JsonKeys { public static final String IS_CONTINUOUS_PROFILING_ENABLED = "is_continuous_profiling_enabled"; public static final String PROFILE_LIFECYCLE = "profile_lifecycle"; public static final String PROFILING_TRACES_HZ = "profiling_traces_hz"; + public static final String IS_ENABLE_APP_START_PROFILING = "is_enable_app_start_profiling"; + public static final String IS_START_PROFILER_ON_APP_START = "is_start_profiler_on_app_start"; } @Override @@ -165,6 +189,8 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger .value(logger, isContinuousProfilingEnabled); writer.name(JsonKeys.PROFILE_LIFECYCLE).value(logger, profileLifecycle.name()); writer.name(JsonKeys.PROFILING_TRACES_HZ).value(logger, profilingTracesHz); + writer.name(JsonKeys.IS_ENABLE_APP_START_PROFILING).value(logger, isEnableAppStartProfiling); + writer.name(JsonKeys.IS_START_PROFILER_ON_APP_START).value(logger, isStartProfilerOnAppStart); if (unknown != null) { for (String key : unknown.keySet()) { @@ -266,6 +292,18 @@ public static final class Deserializer options.profilingTracesHz = profilingTracesHz; } break; + case JsonKeys.IS_ENABLE_APP_START_PROFILING: + Boolean isEnableAppStartProfiling = reader.nextBooleanOrNull(); + if (isEnableAppStartProfiling != null) { + options.isEnableAppStartProfiling = isEnableAppStartProfiling; + } + break; + case JsonKeys.IS_START_PROFILER_ON_APP_START: + Boolean isStartProfilerOnAppStart = reader.nextBooleanOrNull(); + if (isStartProfilerOnAppStart != null) { + options.isStartProfilerOnAppStart = isStartProfilerOnAppStart; + } + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 538bbc0ff0c..c7087dbb36d 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1813,7 +1813,9 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { return experimental.getProfileLifecycle(); } - /** Whether profiling can automatically be started as early as possible during the app lifecycle. */ + /** + * Whether profiling can automatically be started as early as possible during the app lifecycle. + */ @ApiStatus.Experimental public boolean isStartProfilerOnAppStart() { return experimental.isStartProfilerOnAppStart(); diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index a8f1a1e80c5..38a3f49f24c 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -1234,8 +1234,8 @@ class JsonSerializerTest { val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"continuous_profile_sampled\":true," + "\"trace_sampled\":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false," + - "\"is_continuous_profiling_enabled\":false,\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65}" - + "\"is_continuous_profiling_enabled\":false,\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65," + + "\"is_enable_app_start_profiling\":false,\"is_start_profiler_on_app_start\":true}" assertEquals(expected, actual) } @@ -1243,7 +1243,8 @@ class JsonSerializerTest { fun `deserializing SentryAppStartProfilingOptions`() { val jsonAppStartProfilingOptions = "{\"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," + - "\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65,\"continuous_profile_sampled\":true}" + "\"profile_lifecycle\":\"TRACE\",\"profiling_traces_hz\":65,\"continuous_profile_sampled\":true," + + "\"is_enable_app_start_profiling\":false,\"is_start_profiler_on_app_start\":true}" val actual = fixture.serializer.deserialize(StringReader(jsonAppStartProfilingOptions), SentryAppStartProfilingOptions::class.java) assertNotNull(actual) @@ -1257,6 +1258,8 @@ class JsonSerializerTest { assertEquals(appStartProfilingOptions.profilingTracesHz, actual.profilingTracesHz) assertEquals(appStartProfilingOptions.profilingTracesDirPath, actual.profilingTracesDirPath) assertEquals(appStartProfilingOptions.profileLifecycle, actual.profileLifecycle) + assertEquals(appStartProfilingOptions.isEnableAppStartProfiling, actual.isEnableAppStartProfiling) + assertEquals(appStartProfilingOptions.isStartProfilerOnAppStart, actual.isStartProfilerOnAppStart) assertNull(actual.unknown) } @@ -1562,6 +1565,8 @@ class JsonSerializerTest { isContinuousProfilingEnabled = false profilingTracesHz = 65 profileLifecycle = ProfileLifecycle.TRACE + isEnableAppStartProfiling = false + isStartProfilerOnAppStart = true } private fun createSpan(): ISpan { diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index cf7f7384f89..bf68c63d077 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -1130,6 +1130,25 @@ class SentryTest { ) } + @Test + fun `init calls samplers if isStartProfilerOnAppStart is true`() { + val mockSampleTracer = mock() + val mockProfilesSampler = mock() + Sentry.init { + it.dsn = dsn + it.tracesSampleRate = 1.0 + it.experimental.isStartProfilerOnAppStart = true + it.profilesSampleRate = 1.0 + it.tracesSampler = mockSampleTracer + it.profilesSampler = mockProfilesSampler + it.executorService = ImmediateExecutorService() + it.cacheDirPath = getTempPath() + } + // Samplers are not called + verify(mockSampleTracer, never()).sample(any()) + verify(mockProfilesSampler, never()).sample(any()) + } + @Test fun `init calls app start profiling samplers in the background`() { val mockSampleTracer = mock() @@ -1220,6 +1239,24 @@ class SentryTest { assertTrue(appStartProfilingConfigFile.exists()) } + @Test + fun `init creates app start profiling config if isStartProfilerOnAppStart, even with performance disabled`() { + val path = getTempPath() + File(path).mkdirs() + val appStartProfilingConfigFile = File(path, "app_start_profiling_config") + appStartProfilingConfigFile.createNewFile() + assertTrue(appStartProfilingConfigFile.exists()) + Sentry.init { + it.dsn = dsn + it.cacheDirPath = path + it.isEnableAppStartProfiling = false + it.experimental.isStartProfilerOnAppStart = true + it.tracesSampleRate = 0.0 + it.executorService = ImmediateExecutorService() + } + assertTrue(appStartProfilingConfigFile.exists()) + } + @Test fun `init saves SentryAppStartProfilingOptions to disk`() { var options = SentryOptions() @@ -1227,9 +1264,9 @@ class SentryTest { Sentry.init { it.dsn = dsn it.cacheDirPath = path - it.tracesSampleRate = 1.0 it.tracesSampleRate = 0.5 it.isEnableAppStartProfiling = true + it.experimental.isStartProfilerOnAppStart = true it.profilesSampleRate = 0.2 it.executorService = ImmediateExecutorService() options = it @@ -1242,6 +1279,8 @@ class SentryTest { assertEquals(0.5, appStartOption.traceSampleRate) assertEquals(0.2, appStartOption.profileSampleRate) assertTrue(appStartOption.isProfilingEnabled) + assertTrue(appStartOption.isEnableAppStartProfiling) + assertTrue(appStartOption.isStartProfilerOnAppStart) } @Test From d72b09fb76b46124a183bc44b837422b5e19fe0b Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 7 Mar 2025 11:28:01 +0100 Subject: [PATCH 13/15] added app start option checks in SentryPerformanceProvider --- .../core/SentryPerformanceProvider.java | 8 +++++--- .../core/SentryPerformanceProviderTest.kt | 20 +++++++++++++++++++ .../src/main/AndroidManifest.xml | 8 ++++++-- 3 files changed, 31 insertions(+), 5 deletions(-) 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 df05f880d8a..93801b0cf5d 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 @@ -139,7 +139,8 @@ private void launchAppStartProfiler(final @NotNull AppStartMetrics appStartMetri return; } - if (profilingOptions.isContinuousProfilingEnabled()) { + if (profilingOptions.isContinuousProfilingEnabled() + && profilingOptions.isStartProfilerOnAppStart()) { createAndStartContinuousProfiler(context, profilingOptions, appStartMetrics); return; } @@ -150,8 +151,9 @@ private void launchAppStartProfiler(final @NotNull AppStartMetrics appStartMetri return; } - createAndStartTransactionProfiler(context, profilingOptions, appStartMetrics); - + if (profilingOptions.isEnableAppStartProfiling()) { + createAndStartTransactionProfiler(context, profilingOptions, appStartMetrics); + } } catch (FileNotFoundException e) { logger.log(SentryLevel.ERROR, "App start profiling config file not found. ", e); } catch (Throwable e) { 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 16cce1a2093..83f5795f13e 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 @@ -326,6 +326,22 @@ class SentryPerformanceProviderTest { assertFalse(AppStartMetrics.getInstance().appStartProfiler!!.isRunning) } + @Test + fun `when isEnableAppStartProfiling is false, transaction profiler is not started`() { + fixture.getSut { config -> + writeConfig(config, profilingEnabled = true, continuousProfilingEnabled = false, isEnableAppStartProfiling = false) + } + assertNull(AppStartMetrics.getInstance().appStartProfiler) + } + + @Test + fun `when isStartProfilerOnAppStart is false, continuous profiler is not started`() { + fixture.getSut { config -> + writeConfig(config, profilingEnabled = false, continuousProfilingEnabled = true, isStartProfilerOnAppStart = false) + } + assertNull(AppStartMetrics.getInstance().appStartContinuousProfiler) + } + @Test fun `when provider is closed, continuous profiler is stopped`() { val provider = fixture.getSut { config -> @@ -345,6 +361,8 @@ class SentryPerformanceProviderTest { profileSampled: Boolean = true, profileSampleRate: Double = 1.0, continuousProfileSampled: Boolean = true, + isEnableAppStartProfiling: Boolean = true, + isStartProfilerOnAppStart: Boolean = true, profilingTracesDirPath: String = traceDir.absolutePath ) { val appStartProfilingOptions = SentryAppStartProfilingOptions() @@ -357,6 +375,8 @@ class SentryPerformanceProviderTest { appStartProfilingOptions.isContinuousProfileSampled = continuousProfileSampled appStartProfilingOptions.profilingTracesDirPath = profilingTracesDirPath appStartProfilingOptions.profilingTracesHz = 101 + appStartProfilingOptions.isEnableAppStartProfiling = isEnableAppStartProfiling + appStartProfilingOptions.isStartProfilerOnAppStart = isStartProfilerOnAppStart JsonSerializer(SentryOptions.empty()).serialize(appStartProfilingOptions, FileWriter(configFile)) } //endregion diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index 5afe6ac1808..03fb4e5f208 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -112,8 +112,12 @@ - - + + + + + + From 6b168b6085a302281dee383268b9acde114c899b Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 12 Mar 2025 09:57:35 +0100 Subject: [PATCH 14/15] added @Nullable annotation to SentryAppStartProfilingOptions json decoding --- .../SentryAppStartProfilingOptions.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java index 75ab308af1f..3c16504eb7c 100644 --- a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java @@ -227,55 +227,55 @@ public static final class Deserializer final String nextName = reader.nextName(); switch (nextName) { case JsonKeys.PROFILE_SAMPLED: - Boolean profileSampled = reader.nextBooleanOrNull(); + @Nullable Boolean profileSampled = reader.nextBooleanOrNull(); if (profileSampled != null) { options.profileSampled = profileSampled; } break; case JsonKeys.PROFILE_SAMPLE_RATE: - Double profileSampleRate = reader.nextDoubleOrNull(); + @Nullable Double profileSampleRate = reader.nextDoubleOrNull(); if (profileSampleRate != null) { options.profileSampleRate = profileSampleRate; } break; case JsonKeys.CONTINUOUS_PROFILE_SAMPLED: - Boolean continuousProfileSampled = reader.nextBooleanOrNull(); + @Nullable Boolean continuousProfileSampled = reader.nextBooleanOrNull(); if (continuousProfileSampled != null) { options.continuousProfileSampled = continuousProfileSampled; } break; case JsonKeys.TRACE_SAMPLED: - Boolean traceSampled = reader.nextBooleanOrNull(); + @Nullable Boolean traceSampled = reader.nextBooleanOrNull(); if (traceSampled != null) { options.traceSampled = traceSampled; } break; case JsonKeys.TRACE_SAMPLE_RATE: - Double traceSampleRate = reader.nextDoubleOrNull(); + @Nullable Double traceSampleRate = reader.nextDoubleOrNull(); if (traceSampleRate != null) { options.traceSampleRate = traceSampleRate; } break; case JsonKeys.PROFILING_TRACES_DIR_PATH: - String profilingTracesDirPath = reader.nextStringOrNull(); + @Nullable String profilingTracesDirPath = reader.nextStringOrNull(); if (profilingTracesDirPath != null) { options.profilingTracesDirPath = profilingTracesDirPath; } break; case JsonKeys.IS_PROFILING_ENABLED: - Boolean isProfilingEnabled = reader.nextBooleanOrNull(); + @Nullable Boolean isProfilingEnabled = reader.nextBooleanOrNull(); if (isProfilingEnabled != null) { options.isProfilingEnabled = isProfilingEnabled; } break; case JsonKeys.IS_CONTINUOUS_PROFILING_ENABLED: - Boolean isContinuousProfilingEnabled = reader.nextBooleanOrNull(); + @Nullable Boolean isContinuousProfilingEnabled = reader.nextBooleanOrNull(); if (isContinuousProfilingEnabled != null) { options.isContinuousProfilingEnabled = isContinuousProfilingEnabled; } break; case JsonKeys.PROFILE_LIFECYCLE: - String profileLifecycle = reader.nextStringOrNull(); + @Nullable String profileLifecycle = reader.nextStringOrNull(); if (profileLifecycle != null) { try { options.profileLifecycle = ProfileLifecycle.valueOf(profileLifecycle); @@ -287,19 +287,19 @@ public static final class Deserializer } break; case JsonKeys.PROFILING_TRACES_HZ: - Integer profilingTracesHz = reader.nextIntegerOrNull(); + @Nullable Integer profilingTracesHz = reader.nextIntegerOrNull(); if (profilingTracesHz != null) { options.profilingTracesHz = profilingTracesHz; } break; case JsonKeys.IS_ENABLE_APP_START_PROFILING: - Boolean isEnableAppStartProfiling = reader.nextBooleanOrNull(); + @Nullable Boolean isEnableAppStartProfiling = reader.nextBooleanOrNull(); if (isEnableAppStartProfiling != null) { options.isEnableAppStartProfiling = isEnableAppStartProfiling; } break; case JsonKeys.IS_START_PROFILER_ON_APP_START: - Boolean isStartProfilerOnAppStart = reader.nextBooleanOrNull(); + @Nullable Boolean isStartProfilerOnAppStart = reader.nextBooleanOrNull(); if (isStartProfilerOnAppStart != null) { options.isStartProfilerOnAppStart = isStartProfilerOnAppStart; } From 1381d4a4ca6e591216391cd424223efa7ea88d0a Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 12 Mar 2025 10:11:52 +0100 Subject: [PATCH 15/15] added @Nullable annotation to ManifestMetadataReader json decoding --- .../android/core/ManifestMetadataReader.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) 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 a01ae5a83ff..bf1eb79be84 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 @@ -139,7 +139,7 @@ static void applyMetadata( options.setDebug(readBool(metadata, logger, DEBUG, options.isDebug())); if (options.isDebug()) { - final String level = + final @Nullable String level = readString( metadata, logger, @@ -161,7 +161,7 @@ static void applyMetadata( options.isEnableAutoSessionTracking())); if (options.getSampleRate() == null) { - final Double sampleRate = readDouble(metadata, logger, SAMPLE_RATE); + final double sampleRate = readDouble(metadata, logger, SAMPLE_RATE); if (sampleRate != -1) { options.setSampleRate(sampleRate); } @@ -180,7 +180,7 @@ static void applyMetadata( options.setAttachAnrThreadDump( readBool(metadata, logger, ANR_ATTACH_THREAD_DUMPS, options.isAttachAnrThreadDump())); - final String dsn = readString(metadata, logger, DSN, options.getDsn()); + final @Nullable String dsn = readString(metadata, logger, DSN, options.getDsn()); final boolean enabled = readBool(metadata, logger, ENABLE_SENTRY, options.isEnabled()); if (!enabled || (dsn != null && dsn.isEmpty())) { @@ -293,7 +293,7 @@ static void applyMetadata( options.isCollectAdditionalContext())); if (options.getTracesSampleRate() == null) { - final Double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); + final double tracesSampleRate = readDouble(metadata, logger, TRACES_SAMPLE_RATE); if (tracesSampleRate != -1) { options.setTracesSampleRate(tracesSampleRate); } @@ -317,7 +317,7 @@ static void applyMetadata( options.isEnableActivityLifecycleTracingAutoFinish())); if (options.getProfilesSampleRate() == null) { - final Double profilesSampleRate = readDouble(metadata, logger, PROFILES_SAMPLE_RATE); + final double profilesSampleRate = readDouble(metadata, logger, PROFILES_SAMPLE_RATE); if (profilesSampleRate != -1) { options.setProfilesSampleRate(profilesSampleRate); } @@ -331,7 +331,7 @@ static void applyMetadata( } } - final String profileLifecycle = + final @Nullable String profileLifecycle = readString( metadata, logger, @@ -393,6 +393,7 @@ static void applyMetadata( // sdkInfo.addIntegration(); + @Nullable List integrationsFromGradlePlugin = readList(metadata, logger, SENTRY_GRADLE_PLUGIN_INTEGRATIONS); if (integrationsFromGradlePlugin != null) { @@ -418,7 +419,7 @@ static void applyMetadata( metadata, logger, ENABLE_SCOPE_PERSISTENCE, options.isEnableScopePersistence())); if (options.getExperimental().getSessionReplay().getSessionSampleRate() == null) { - final Double sessionSampleRate = + final double sessionSampleRate = readDouble(metadata, logger, REPLAYS_SESSION_SAMPLE_RATE); if (sessionSampleRate != -1) { options.getExperimental().getSessionReplay().setSessionSampleRate(sessionSampleRate); @@ -426,7 +427,7 @@ static void applyMetadata( } if (options.getExperimental().getSessionReplay().getOnErrorSampleRate() == null) { - final Double onErrorSampleRate = readDouble(metadata, logger, REPLAYS_ERROR_SAMPLE_RATE); + final double onErrorSampleRate = readDouble(metadata, logger, REPLAYS_ERROR_SAMPLE_RATE); if (onErrorSampleRate != -1) { options.getExperimental().getSessionReplay().setOnErrorSampleRate(onErrorSampleRate); } @@ -512,10 +513,10 @@ private static boolean readBool( } } - private static @NotNull Double readDouble( + private static double readDouble( final @NotNull Bundle metadata, final @NotNull ILogger logger, final @NotNull String key) { // manifest meta-data only reads float - final Double value = ((Number) metadata.getFloat(key, metadata.getInt(key, -1))).doubleValue(); + final double value = ((Number) metadata.getFloat(key, metadata.getInt(key, -1))).doubleValue(); logger.log(SentryLevel.DEBUG, key + " read: " + value); return value; }