From 10591949a06f90f00951bee7e95053c42f6d309e Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 23 Dec 2024 16:57:12 +0100 Subject: [PATCH 1/2] added SentryOptions.continuousProfilesSampleRate now continuous profiling is disabled if continuousProfilesSampleRate is 0 profiles directory is created when continuous profiling is enabled, too continuous profiling decision is passed to SentryAppStartProfilingOptions app start continuous profiling is sampled, too --- .../android/core/ManifestMetadataReader.java | 11 ++++ .../core/SentryPerformanceProvider.java | 6 ++ .../core/ManifestMetadataReaderTest.kt | 41 ++++++++++++++ .../core/SentryPerformanceProviderTest.kt | 15 +++++ sentry/api/sentry.api | 9 +++ .../main/java/io/sentry/ExternalOptions.java | 11 ++++ sentry/src/main/java/io/sentry/Scopes.java | 12 +++- sentry/src/main/java/io/sentry/Sentry.java | 3 +- .../SentryAppStartProfilingOptions.java | 19 +++++++ .../main/java/io/sentry/SentryOptions.java | 55 ++++++++++++++++--- .../main/java/io/sentry/TracesSampler.java | 5 ++ .../java/io/sentry/util/SampleRateUtils.java | 4 ++ .../java/io/sentry/ExternalOptionsTest.kt | 7 +++ .../test/java/io/sentry/JsonSerializerTest.kt | 11 ++-- sentry/src/test/java/io/sentry/ScopesTest.kt | 19 +++++++ .../test/java/io/sentry/SentryOptionsTest.kt | 39 ++++++++++++- sentry/src/test/java/io/sentry/SentryTest.kt | 44 +++++++++++++++ .../test/java/io/sentry/TracesSamplerTest.kt | 25 +++++++++ .../java/io/sentry/util/SampleRateUtilTest.kt | 35 ++++++++++++ 19 files changed, 354 insertions(+), 17 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 45661866fa8..f49291340fe 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,6 +64,9 @@ 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"; + @ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling"; static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets"; @@ -315,6 +318,14 @@ static void applyMetadata( } } + if (options.getContinuousProfilesSampleRate() == 1.0) { + final double continuousProfilesSampleRate = + readDouble(metadata, logger, CONTINUOUS_PROFILES_SAMPLE_RATE); + if (continuousProfilesSampleRate != -1) { + options.setContinuousProfilesSampleRate(continuousProfilesSampleRate); + } + } + 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 1d76775b3f8..583f984cc8a 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 @@ -162,6 +162,12 @@ private void createAndStartContinuousProfiler( final @NotNull Context context, final @NotNull SentryAppStartProfilingOptions profilingOptions, final @NotNull AppStartMetrics appStartMetrics) { + + if (!profilingOptions.isContinuousProfileSampled()) { + logger.log(SentryLevel.DEBUG, "App start profiling was not sampled. It will not start."); + return; + } + final @NotNull IContinuousProfiler appStartContinuousProfiler = new AndroidContinuousProfiler( buildInfoProvider, 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 5d6e915c0bc..20900ea133b 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 @@ -800,6 +800,47 @@ class ManifestMetadataReaderTest { assertNull(fixture.options.profilesSampleRate) } + @Test + fun `applyMetadata reads continuousProfilesSampleRate from metadata`() { + // Arrange + val expectedSampleRate = 0.99f + val bundle = bundleOf(ManifestMetadataReader.CONTINUOUS_PROFILES_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) + } + + @Test + fun `applyMetadata does not override continuousProfilesSampleRate from options`() { + // Arrange + val expectedSampleRate = 0.99f + fixture.options.continuousProfilesSampleRate = expectedSampleRate.toDouble() + val bundle = bundleOf(ManifestMetadataReader.CONTINUOUS_PROFILES_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) + } + + @Test + fun `applyMetadata without specifying continuousProfilesSampleRate, stays 1`() { + // Arrange + val context = fixture.getContext() + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertEquals(1.0, fixture.options.continuousProfilesSampleRate) + } + @Test fun `applyMetadata reads tracePropagationTargets to options`() { // Arrange 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 237bc548675..16cce1a2093 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 @@ -259,6 +259,19 @@ class SentryPerformanceProviderTest { ) } + @Test + fun `when continuous profile is not sampled, continuous profiler is not started`() { + fixture.getSut { config -> + writeConfig(config, continuousProfileSampled = false) + } + assertNull(AppStartMetrics.getInstance().appStartProfiler) + assertNull(AppStartMetrics.getInstance().appStartContinuousProfiler) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("App start profiling was not sampled. It will not start.") + ) + } + // This case should never happen in reality, but it's technically possible to have such configuration @Test fun `when both transaction and continuous profilers are enabled, only continuous profiler is created`() { @@ -331,6 +344,7 @@ class SentryPerformanceProviderTest { traceSampleRate: Double = 1.0, profileSampled: Boolean = true, profileSampleRate: Double = 1.0, + continuousProfileSampled: Boolean = true, profilingTracesDirPath: String = traceDir.absolutePath ) { val appStartProfilingOptions = SentryAppStartProfilingOptions() @@ -340,6 +354,7 @@ class SentryPerformanceProviderTest { appStartProfilingOptions.traceSampleRate = traceSampleRate appStartProfilingOptions.isProfileSampled = profileSampled appStartProfilingOptions.profileSampleRate = profileSampleRate + appStartProfilingOptions.isContinuousProfileSampled = continuousProfileSampled appStartProfilingOptions.profilingTracesDirPath = profilingTracesDirPath appStartProfilingOptions.profilingTracesHz = 101 JsonSerializer(SentryOptions.empty()).serialize(appStartProfilingOptions, FileWriter(configFile)) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7fa2abafa7c..a097588a7c4 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -458,6 +458,7 @@ 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; @@ -491,6 +492,7 @@ 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 @@ -2488,11 +2490,13 @@ public final class io/sentry/SentryAppStartProfilingOptions : io/sentry/JsonSeri public fun getProfilingTracesHz ()I public fun getTraceSampleRate ()Ljava/lang/Double; public fun getUnknown ()Ljava/util/Map; + public fun isContinuousProfileSampled ()Z public fun isContinuousProfilingEnabled ()Z public fun isProfileSampled ()Z public fun isProfilingEnabled ()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 setProfileSampleRate (Ljava/lang/Double;)V public fun setProfileSampled (Z)V @@ -2511,6 +2515,7 @@ 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_PROFILING_ENABLED Ljava/lang/String; public static final field PROFILE_SAMPLED Ljava/lang/String; @@ -2944,6 +2949,7 @@ 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; @@ -3060,6 +3066,7 @@ 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 @@ -3759,6 +3766,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 final class io/sentry/TracesSamplingDecision { @@ -6316,6 +6324,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 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/ExternalOptions.java b/sentry/src/main/java/io/sentry/ExternalOptions.java index d9b075e1c89..020b7aea9f6 100644 --- a/sentry/src/main/java/io/sentry/ExternalOptions.java +++ b/sentry/src/main/java/io/sentry/ExternalOptions.java @@ -28,6 +28,7 @@ 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; @@ -73,6 +74,8 @@ 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")); @@ -284,6 +287,14 @@ 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/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index 86f0f5f8bcb..638127cc547 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -926,9 +926,15 @@ public void flush(long timeoutMillis) { @Override public void startProfiler() { if (getOptions().isContinuousProfilingEnabled()) { - getOptions().getLogger().log(SentryLevel.DEBUG, "Started continuous Profiling."); - getOptions().getContinuousProfiler().start(); - } else { + 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."); + } + } else if (getOptions().isProfilingEnabled()) { getOptions() .getLogger() .log( diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 8e8f8c5b659..383ba50e841 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -514,7 +514,8 @@ private static void initConfigurations(final @NotNull SentryOptions options) { } final String profilingTracesDirPath = options.getProfilingTracesDirPath(); - if (options.isProfilingEnabled() && profilingTracesDirPath != null) { + if ((options.isProfilingEnabled() || options.isContinuousProfilingEnabled()) + && profilingTracesDirPath != null) { final File profilingTracesDir = new File(profilingTracesDirPath); profilingTracesDir.mkdirs(); diff --git a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java index b0926b9d937..c4d99a60f25 100644 --- a/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryAppStartProfilingOptions.java @@ -20,6 +20,7 @@ public final class SentryAppStartProfilingOptions implements JsonUnknown, JsonSe boolean isProfilingEnabled; boolean isContinuousProfilingEnabled; int profilingTracesHz; + boolean continuousProfileSampled; private @Nullable Map unknown; @@ -29,6 +30,7 @@ public SentryAppStartProfilingOptions() { traceSampleRate = null; profileSampled = false; profileSampleRate = null; + continuousProfileSampled = false; profilingTracesDirPath = null; isProfilingEnabled = false; isContinuousProfilingEnabled = false; @@ -42,6 +44,7 @@ public SentryAppStartProfilingOptions() { traceSampleRate = samplingDecision.getSampleRate(); profileSampled = samplingDecision.getProfileSampled(); profileSampleRate = samplingDecision.getProfileSampleRate(); + continuousProfileSampled = options.getInternalTracesSampler().sampleContinuousProfile(); profilingTracesDirPath = options.getProfilingTracesDirPath(); isProfilingEnabled = options.isProfilingEnabled(); isContinuousProfilingEnabled = options.isContinuousProfilingEnabled(); @@ -56,6 +59,14 @@ public boolean isProfileSampled() { return profileSampled; } + public void setContinuousProfileSampled(boolean continuousProfileSampled) { + this.continuousProfileSampled = continuousProfileSampled; + } + + public boolean isContinuousProfileSampled() { + return continuousProfileSampled; + } + public void setProfileSampleRate(final @Nullable Double profileSampleRate) { this.profileSampleRate = profileSampleRate; } @@ -117,6 +128,7 @@ public int getProfilingTracesHz() { public static final class JsonKeys { public static final String PROFILE_SAMPLED = "profile_sampled"; public static final String PROFILE_SAMPLE_RATE = "profile_sample_rate"; + public static final String CONTINUOUS_PROFILE_SAMPLED = "continuous_profile_sampled"; public static final String TRACE_SAMPLED = "trace_sampled"; public static final String TRACE_SAMPLE_RATE = "trace_sample_rate"; public static final String PROFILING_TRACES_DIR_PATH = "profiling_traces_dir_path"; @@ -131,6 +143,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger writer.beginObject(); writer.name(JsonKeys.PROFILE_SAMPLED).value(logger, profileSampled); writer.name(JsonKeys.PROFILE_SAMPLE_RATE).value(logger, profileSampleRate); + writer.name(JsonKeys.CONTINUOUS_PROFILE_SAMPLED).value(logger, continuousProfileSampled); writer.name(JsonKeys.TRACE_SAMPLED).value(logger, traceSampled); writer.name(JsonKeys.TRACE_SAMPLE_RATE).value(logger, traceSampleRate); writer.name(JsonKeys.PROFILING_TRACES_DIR_PATH).value(logger, profilingTracesDirPath); @@ -186,6 +199,12 @@ public static final class Deserializer options.profileSampleRate = profileSampleRate; } break; + case JsonKeys.CONTINUOUS_PROFILE_SAMPLED: + Boolean continuousProfileSampled = reader.nextBooleanOrNull(); + if (continuousProfileSampled != null) { + options.continuousProfileSampled = continuousProfileSampled; + } + break; case JsonKeys.TRACE_SAMPLED: Boolean traceSampled = reader.nextBooleanOrNull(); if (traceSampled != null) { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index cacf4a77fce..3bd13334b82 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -357,6 +357,15 @@ 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; @@ -481,7 +490,10 @@ public class SentryOptions { private boolean enableBackpressureHandling = true; - /** Whether to profile app launches, depending on profilesSampler or profilesSampleRate. */ + /** + * Whether to profile app launches, depending on profilesSampler, profilesSampleRate or + * continuousProfilesSampleRate. + */ private boolean enableAppStartProfiling = false; private @NotNull ISpanFactory spanFactory = NoOpSpanFactory.getInstance(); @@ -1724,8 +1736,7 @@ public void setContinuousProfiler(final @Nullable IContinuousProfiler continuous * @return if profiling is enabled for transactions. */ public boolean isProfilingEnabled() { - return (getProfilesSampleRate() != null && getProfilesSampleRate() > 0) - || getProfilesSampler() != null; + return (profilesSampleRate != null && profilesSampleRate > 0) || profilesSampler != null; } /** @@ -1736,7 +1747,9 @@ public boolean isProfilingEnabled() { */ @ApiStatus.Internal public boolean isContinuousProfilingEnabled() { - return getProfilesSampleRate() == null && getProfilesSampler() == null; + return profilesSampleRate == null + && profilesSampler == null + && continuousProfilesSampleRate > 0; } /** @@ -1783,6 +1796,27 @@ public void setProfilesSampleRate(final @Nullable Double profilesSampleRate) { this.profilesSampleRate = 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. + * + * @return the sample rate + */ + public double getContinuousProfilesSampleRate() { + return continuousProfilesSampleRate; + } + + public void setContinuousProfilesSampleRate(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; + } + /** * Returns the profiling traces dir. path if set * @@ -2181,17 +2215,19 @@ public void setEnablePrettySerializationOutput(boolean enablePrettySerialization } /** - * Whether to profile app launches, depending on profilesSampler or profilesSampleRate. Depends on - * {@link SentryOptions#isProfilingEnabled()} + * Whether to profile app launches, depending on profilesSampler, profilesSampleRate or + * continuousProfilesSampleRate. Depends on {@link SentryOptions#isProfilingEnabled()} and {@link + * SentryOptions#isContinuousProfilingEnabled()} * * @return true if app launches should be profiled. */ public boolean isEnableAppStartProfiling() { - return isProfilingEnabled() && enableAppStartProfiling; + return (isProfilingEnabled() || isContinuousProfilingEnabled()) && enableAppStartProfiling; } /** - * Whether to profile app launches, depending on profilesSampler or profilesSampleRate. + * Whether to profile app launches, depending on profilesSampler, profilesSampleRate or + * continuousProfilesSampleRate. * * @param enableAppStartProfiling true if app launches should be profiled. */ @@ -2707,6 +2743,9 @@ public void merge(final @NotNull ExternalOptions options) { if (options.getProfilesSampleRate() != null) { setProfilesSampleRate(options.getProfilesSampleRate()); } + if (options.getContinuousProfilesSampleRate() != null) { + 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 3ce28ab7451..fcf6e3929ae 100644 --- a/sentry/src/main/java/io/sentry/TracesSampler.java +++ b/sentry/src/main/java/io/sentry/TracesSampler.java @@ -85,6 +85,11 @@ public TracesSamplingDecision sample(final @NotNull SamplingContext samplingCont return new TracesSamplingDecision(false, null, false, null); } + public boolean sampleContinuousProfile() { + final double sampling = options.getContinuousProfilesSampleRate(); + return sample(sampling); + } + private boolean sample(final @NotNull Double aDouble) { return !(aDouble < getRandom().nextDouble()); } diff --git a/sentry/src/main/java/io/sentry/util/SampleRateUtils.java b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java index ed011ff842b..ab84cd0e9d3 100644 --- a/sentry/src/main/java/io/sentry/util/SampleRateUtils.java +++ b/sentry/src/main/java/io/sentry/util/SampleRateUtils.java @@ -23,6 +23,10 @@ public static boolean isValidProfilesSampleRate(@Nullable Double profilesSampleR return isValidRate(profilesSampleRate, true); } + public static boolean isValidContinuousProfilesSampleRate(double profilesSampleRate) { + return isValidRate(profilesSampleRate, false); + } + private static boolean isValidRate(final @Nullable Double rate, final boolean allowNull) { if (rate == null) { return allowNull; diff --git a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt index b25f67405cf..3a1d66b264c 100644 --- a/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/ExternalOptionsTest.kt @@ -120,6 +120,13 @@ 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/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 53dd7d85bfc..7b8fc219b93 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -1229,9 +1229,9 @@ class JsonSerializerTest { fun `serializing SentryAppStartProfilingOptions`() { val actual = serializeToString(appStartProfilingOptions) - val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\":false," + - "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false," + - "\"is_continuous_profiling_enabled\":false,\"profiling_traces_hz\":65}" + 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}" assertEquals(expected, actual) } @@ -1239,7 +1239,8 @@ class JsonSerializerTest { @Test 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}" + ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false," + + "\"profiling_traces_hz\":65,\"continuous_profile_sampled\":true}" val actual = fixture.serializer.deserialize(StringReader(jsonAppStartProfilingOptions), SentryAppStartProfilingOptions::class.java) assertNotNull(actual) @@ -1247,6 +1248,7 @@ class JsonSerializerTest { assertEquals(appStartProfilingOptions.traceSampleRate, actual.traceSampleRate) assertEquals(appStartProfilingOptions.profileSampled, actual.profileSampled) assertEquals(appStartProfilingOptions.profileSampleRate, actual.profileSampleRate) + assertEquals(appStartProfilingOptions.continuousProfileSampled, actual.isContinuousProfileSampled) assertEquals(appStartProfilingOptions.isProfilingEnabled, actual.isProfilingEnabled) assertEquals(appStartProfilingOptions.isContinuousProfilingEnabled, actual.isContinuousProfilingEnabled) assertEquals(appStartProfilingOptions.profilingTracesHz, actual.profilingTracesHz) @@ -1549,6 +1551,7 @@ class JsonSerializerTest { private val appStartProfilingOptions = SentryAppStartProfilingOptions().apply { traceSampled = false traceSampleRate = 0.1 + continuousProfileSampled = true profileSampled = true profileSampleRate = 0.8 isProfilingEnabled = false diff --git a/sentry/src/test/java/io/sentry/ScopesTest.kt b/sentry/src/test/java/io/sentry/ScopesTest.kt index 68a1fee5d6b..107d9375a4a 100644 --- a/sentry/src/test/java/io/sentry/ScopesTest.kt +++ b/sentry/src/test/java/io/sentry/ScopesTest.kt @@ -15,6 +15,7 @@ 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 @@ -2170,6 +2171,24 @@ class ScopesTest { verify(profiler).start() } + @Test + fun `startProfiler logs a message if not sampled`() { + val profiler = mock() + val logger = mock() + val scopes = generateScopes { + it.setContinuousProfiler(profiler) + it.continuousProfilesSampleRate = 0.1 + 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.")) + } + @Test fun `stopProfiler stops the continuous profiler`() { val profiler = mock() diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index afe962d19fa..03146f81800 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -236,6 +236,15 @@ class SentryOptionsTest { assertFalse(options.isContinuousProfilingEnabled) } + @Test + fun `when continuousProfilesSampleRate is set to a 0, isProfilingEnabled is false and isContinuousProfilingEnabled is false`() { + val options = SentryOptions().apply { + this.continuousProfilesSampleRate = 0.0 + } + assertFalse(options.isProfilingEnabled) + assertFalse(options.isContinuousProfilingEnabled) + } + @Test fun `when setProfilesSampleRate is set to exactly 0, value is set`() { val options = SentryOptions().apply { @@ -254,6 +263,24 @@ class SentryOptionsTest { assertFailsWith { SentryOptions().profilesSampleRate = -0.0000000000001 } } + @Test + fun `when setContinuousProfilesSampleRate is set to exactly 0, value is set`() { + val options = SentryOptions().apply { + this.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 } + } + + @Test + fun `when setContinuousProfilesSampleRate is set to lower than 0, setter throws`() { + assertFailsWith { SentryOptions().continuousProfilesSampleRate = -0.0000000000001 } + } + @Test fun `when options is initialized, compositePerformanceCollector is set`() { assertIs(SentryOptions().compositePerformanceCollector) @@ -322,6 +349,7 @@ 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") @@ -368,6 +396,7 @@ 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) @@ -578,10 +607,18 @@ class SentryOptionsTest { fun `when profiling is disabled, isEnableAppStartProfiling is always false`() { val options = SentryOptions() options.isEnableAppStartProfiling = true - options.profilesSampleRate = 0.0 + options.continuousProfilesSampleRate = 0.0 assertFalse(options.isEnableAppStartProfiling) } + @Test + fun `when setEnableAppStartProfiling is called and continuous profiling is enabled, isEnableAppStartProfiling is true`() { + val options = SentryOptions() + options.isEnableAppStartProfiling = true + options.continuousProfilesSampleRate = 1.0 + assertTrue(options.isEnableAppStartProfiling) + } + @Test fun `when options are initialized, profilingTracesHz is set to 101 by default`() { assertEquals(101, SentryOptions().profilingTracesHz) diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 6f7cd427daf..ae0243618a0 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -18,6 +18,7 @@ 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 @@ -400,6 +401,35 @@ class SentryTest { assertTrue(File(sentryOptions?.profilingTracesDirPath!!).list()!!.isEmpty()) } + @Test + fun `profilingTracesDirPath should be created and cleared at initialization when continuous profiling is enabled`() { + val tempPath = getTempPath() + var sentryOptions: SentryOptions? = null + Sentry.init { + it.dsn = dsn + it.continuousProfilesSampleRate = 1.0 + it.cacheDirPath = tempPath + sentryOptions = it + } + + assertTrue(File(sentryOptions?.profilingTracesDirPath!!).exists()) + assertTrue(File(sentryOptions?.profilingTracesDirPath!!).list()!!.isEmpty()) + } + + @Test + fun `profilingTracesDirPath should not be created when no profiling is enabled`() { + val tempPath = getTempPath() + var sentryOptions: SentryOptions? = null + Sentry.init { + it.dsn = dsn + it.continuousProfilesSampleRate = 0.0 + it.cacheDirPath = tempPath + sentryOptions = it + } + + assertFalse(File(sentryOptions?.profilingTracesDirPath!!).exists()) + } + @Test fun `only old profiles in profilingTracesDirPath should be cleared when profiling is enabled`() { val tempPath = getTempPath() @@ -1300,6 +1330,20 @@ class SentryTest { verify(profiler, never()).start() } + @Test + fun `startProfiler is ignored when not sampled`() { + val profiler = mock() + Sentry.init { + it.dsn = dsn + it.setContinuousProfiler(profiler) + it.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() + } + @Test fun `stopProfiler stops the continuous profiler`() { val profiler = mock() diff --git a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt index 06eb60aece2..4523a6ecd1e 100644 --- a/sentry/src/test/java/io/sentry/TracesSamplerTest.kt +++ b/sentry/src/test/java/io/sentry/TracesSamplerTest.kt @@ -18,6 +18,7 @@ class TracesSamplerTest { randomResult: Double? = null, tracesSampleRate: Double? = null, profilesSampleRate: Double? = null, + continuousProfilesSampleRate: Double? = null, tracesSamplerCallback: SentryOptions.TracesSamplerCallback? = null, profilesSamplerCallback: SentryOptions.ProfilesSamplerCallback? = null, logger: ILogger? = null @@ -33,6 +34,9 @@ class TracesSamplerTest { if (profilesSampleRate != null) { options.profilesSampleRate = profilesSampleRate } + if (continuousProfilesSampleRate != null) { + options.continuousProfilesSampleRate = continuousProfilesSampleRate + } if (tracesSamplerCallback != null) { options.tracesSampler = tracesSamplerCallback } @@ -150,6 +154,27 @@ class TracesSamplerTest { assertEquals(0.2, samplingDecision.profileSampleRate) } + @Test + fun `when continuousProfilesSampleRate is not set returns true`() { + val sampler = fixture.getSut(randomResult = 1.0) + val sampled = sampler.sampleContinuousProfile() + assertTrue(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() + 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() + assertFalse(sampled) + } + @Test fun `when tracesSampler returns null and parentSampled is set sampler uses it as a sampling decision`() { val sampler = fixture.getSut(tracesSamplerCallback = null) diff --git a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt index e5c81bc70e4..3a14f2bec5d 100644 --- a/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt +++ b/sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt @@ -130,4 +130,39 @@ class SampleRateUtilTest { fun `accepts null profiles sample rate`() { assertTrue(SampleRateUtils.isValidProfilesSampleRate(null)) } + + @Test + fun `accepts 0 for continuous profiles sample rate`() { + assertTrue(SampleRateUtils.isValidContinuousProfilesSampleRate(0.0)) + } + + @Test + fun `accepts 1 for continuous profiles sample rate`() { + assertTrue(SampleRateUtils.isValidContinuousProfilesSampleRate(1.0)) + } + + @Test + fun `rejects negative continuous profiles sample rate`() { + assertFalse(SampleRateUtils.isValidContinuousProfilesSampleRate(-0.5)) + } + + @Test + fun `rejects 1 dot 01 for continuous profiles sample rate`() { + assertFalse(SampleRateUtils.isValidContinuousProfilesSampleRate(1.01)) + } + + @Test + fun `rejects NaN continuous profiles sample rate`() { + assertFalse(SampleRateUtils.isValidContinuousProfilesSampleRate(Double.NaN)) + } + + @Test + fun `rejects positive infinite continuous profiles sample rate`() { + assertFalse(SampleRateUtils.isValidContinuousProfilesSampleRate(Double.POSITIVE_INFINITY)) + } + + @Test + fun `rejects negative infinite continuous profiles sample rate`() { + assertFalse(SampleRateUtils.isValidContinuousProfilesSampleRate(Double.NEGATIVE_INFINITY)) + } } From b1121777a990a20b618d41fb3334fedd3515d34c Mon Sep 17 00:00:00 2001 From: Stefano Date: Mon, 23 Dec 2024 19:43:26 +0100 Subject: [PATCH 2/2] Update sentry/src/main/java/io/sentry/SentryOptions.java Co-authored-by: Markus Hintersteiner --- sentry/src/main/java/io/sentry/SentryOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 3bd13334b82..8418d4cea43 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1807,7 +1807,7 @@ public double getContinuousProfilesSampleRate() { return continuousProfilesSampleRate; } - public void setContinuousProfilesSampleRate(double continuousProfilesSampleRate) { + public void setContinuousProfilesSampleRate(final double continuousProfilesSampleRate) { if (!SampleRateUtils.isValidContinuousProfilesSampleRate(continuousProfilesSampleRate)) { throw new IllegalArgumentException( "The value "