From fe75dedf54b39841fb44dd6e12c440740bfa7295 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 4 Jan 2024 17:50:26 +0100 Subject: [PATCH 1/2] first activity transaction now inherits startup sampling decision, if available if a startup profiler was instantiated, it will be reused in AndroidOptionsInitializer, instead of creating a new one added ITransactionProfiler.isRunning startup profiler and sampling decision is stored in AppStartMetrics startup profile is bound to the startup transaction added io.sentry.profiling.enable-startup manifest option moved profilingTracesHz from SentryAndroidOptions to SentryOptions added SentryStartupProfilingProvider --- .../api/sentry-android-core.api | 13 +- .../src/main/AndroidManifest.xml | 6 + .../core/ActivityLifecycleIntegration.java | 18 +- .../core/AndroidOptionsInitializer.java | 35 ++- .../core/AndroidTransactionProfiler.java | 9 + .../android/core/ManifestMetadataReader.java | 6 + .../android/core/SentryAndroidOptions.java | 23 -- .../core/SentryStartupProfilingProvider.java | 147 ++++++++++++ .../core/performance/AppStartMetrics.java | 28 +++ .../core/ActivityLifecycleIntegrationTest.kt | 75 +++++- .../core/AndroidOptionsInitializerTest.kt | 6 + .../core/AndroidTransactionProfilerTest.kt | 32 +++ .../core/ManifestMetadataReaderTest.kt | 27 +++ .../SentryStartupProfilingProviderTest.kt | 214 ++++++++++++++++++ .../core/performance/AppStartMetricsTest.kt | 6 +- .../src/main/AndroidManifest.xml | 2 + sentry/api/sentry.api | 49 ++++ sentry/src/main/java/io/sentry/Hub.java | 23 +- .../src/main/java/io/sentry/HubAdapter.java | 8 + sentry/src/main/java/io/sentry/IHub.java | 15 ++ .../java/io/sentry/ITransactionProfiler.java | 2 + sentry/src/main/java/io/sentry/NoOpHub.java | 8 + .../io/sentry/NoOpTransactionProfiler.java | 5 + sentry/src/main/java/io/sentry/Sentry.java | 100 +++++--- .../main/java/io/sentry/SentryOptions.java | 26 ++- .../sentry/SentryStartupProfilingOptions.java | 75 +++++- .../src/test/java/io/sentry/HubAdapterTest.kt | 10 + sentry/src/test/java/io/sentry/HubTest.kt | 42 ++++ .../test/java/io/sentry/JsonSerializerTest.kt | 7 +- .../io/sentry/NoOpTransactionProfilerTest.kt | 6 + .../test/java/io/sentry/SentryOptionsTest.kt | 12 + sentry/src/test/java/io/sentry/SentryTest.kt | 8 +- 32 files changed, 947 insertions(+), 96 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/SentryStartupProfilingProvider.java create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/SentryStartupProfilingProviderTest.kt diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 360364d59d8..a6a1d9071b5 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -260,7 +260,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getBeforeViewHierarchyCaptureCallback ()Lio/sentry/android/core/SentryAndroidOptions$BeforeCaptureCallback; public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader; public fun getNativeSdkName ()Ljava/lang/String; - public fun getProfilingTracesHz ()I public fun getProfilingTracesIntervalMillis ()I public fun getStartupCrashDurationThresholdMillis ()J public fun isAnrEnabled ()Z @@ -305,7 +304,6 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setEnableScopeSync (Z)V public fun setEnableSystemEventBreadcrumbs (Z)V public fun setNativeSdkName (Ljava/lang/String;)V - public fun setProfilingTracesHz (I)V public fun setProfilingTracesIntervalMillis (I)V public fun setReportHistoricalAnrs (Z)V } @@ -347,6 +345,13 @@ public final class io/sentry/android/core/SentryPerformanceProvider { public fun onCreate ()Z } +public final class io/sentry/android/core/SentryStartupProfilingProvider { + public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V + public fun getType (Landroid/net/Uri;)Ljava/lang/String; + public fun onCreate ()Z + public fun shutdown ()V +} + public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { public fun (Landroid/content/Context;)V public fun (Landroid/content/Context;Ljava/util/List;)V @@ -426,12 +431,16 @@ public class io/sentry/android/core/performance/AppStartMetrics { public fun getContentProviderOnCreateTimeSpans ()Ljava/util/List; public static fun getInstance ()Lio/sentry/android/core/performance/AppStartMetrics; public fun getSdkInitTimeSpan ()Lio/sentry/android/core/performance/TimeSpan; + public fun getStartupProfiler ()Lio/sentry/ITransactionProfiler; + public fun getStartupSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun isAppLaunchedInForeground ()Z public static fun onApplicationCreate (Landroid/app/Application;)V public static fun onApplicationPostCreate (Landroid/app/Application;)V public static fun onContentProviderCreate (Landroid/content/ContentProvider;)V public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V + public fun setStartupProfiler (Lio/sentry/ITransactionProfiler;)V + public fun setStartupSamplingDecision (Lio/sentry/TracesSamplingDecision;)V } public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum { diff --git a/sentry-android-core/src/main/AndroidManifest.xml b/sentry-android-core/src/main/AndroidManifest.xml index dba3e7df8e7..64c0f9b5a0e 100644 --- a/sentry-android-core/src/main/AndroidManifest.xml +++ b/sentry-android-core/src/main/AndroidManifest.xml @@ -8,6 +8,12 @@ android:authorities="${applicationId}.SentryInitProvider" android:exported="false"/> + + contentProviderOnCreates; private final @NotNull List activityLifecycles; + private @Nullable ITransactionProfiler startupProfiler = null; + private @Nullable TracesSamplingDecision startupSamplingDecision = null; public static @NotNull AppStartMetrics getInstance() { @@ -134,6 +139,7 @@ public void addActivityLifecycleTimeSpans(final @NotNull ActivityLifecycleTimeSp return getSdkInitTimeSpan(); } + @TestOnly public void clear() { appStartType = AppStartType.UNKNOWN; appStartSpan.reset(); @@ -141,6 +147,28 @@ public void clear() { applicationOnCreate.reset(); contentProviderOnCreates.clear(); activityLifecycles.clear(); + if (startupProfiler != null) { + startupProfiler.close(); + } + startupProfiler = null; + startupSamplingDecision = null; + } + + public @Nullable ITransactionProfiler getStartupProfiler() { + return startupProfiler; + } + + public void setStartupProfiler(final @Nullable ITransactionProfiler startupProfiler) { + this.startupProfiler = startupProfiler; + } + + public void setStartupSamplingDecision( + final @Nullable TracesSamplingDecision startupSamplingDecision) { + this.startupSamplingDecision = startupSamplingDecision; + } + + public @Nullable TracesSamplingDecision getStartupSamplingDecision() { + return startupSamplingDecision; } /** diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 81979027eb0..c27960eb375 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -97,7 +97,8 @@ class ActivityLifecycleIntegrationTest { // We let the ActivityLifecycleIntegration create the proper transaction here val optionCaptor = argumentCaptor() val contextCaptor = argumentCaptor() - whenever(hub.startTransaction(contextCaptor.capture(), optionCaptor.capture())).thenAnswer { + val startupFlagCaptor = argumentCaptor() + whenever(hub.startTransaction(contextCaptor.capture(), optionCaptor.capture(), startupFlagCaptor.capture())).thenAnswer { val t = SentryTracer(contextCaptor.lastValue, hub, optionCaptor.lastValue) transaction = t return@thenAnswer t @@ -191,7 +192,7 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) sut.onActivityCreated(activity, fixture.bundle) - verify(fixture.hub).startTransaction(any(), any()) + verify(fixture.hub).startTransaction(any(), any(), any()) } @Test @@ -213,7 +214,8 @@ class ActivityLifecycleIntegrationTest { check { transactionOptions -> assertEquals(fixture.options.idleTimeout, transactionOptions.idleTimeout) assertEquals(TransactionOptions.DEFAULT_DEADLINE_TIMEOUT_AUTO_TRANSACTION, transactionOptions.deadlineTimeout) - } + }, + any() ) } @@ -245,7 +247,8 @@ class ActivityLifecycleIntegrationTest { assertEquals("Activity", it.name) assertEquals(TransactionNameSource.COMPONENT, it.transactionNameSource) }, - any() + any(), + any() ) } @@ -585,7 +588,7 @@ class ActivityLifecycleIntegrationTest { val activity = mock() sut.onActivityCreated(activity, mock()) - verify(fixture.hub).startTransaction(any(), any()) + verify(fixture.hub).startTransaction(any(), any(), any()) } @Test @@ -678,10 +681,64 @@ class ActivityLifecycleIntegrationTest { any(), check { assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp()) - } + }, + any() ) } + @Test + fun `When firstActivityCreated is true and startup sampling decision is set, start transaction with isStartup true`() { + AppStartMetrics.getInstance().startupSamplingDecision = mock() + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val date = SentryNanotimeDate(Date(1), 0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction( + any(), + check { + assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp()) + }, + eq(true) + ) + } + + @Test + fun `When firstActivityCreated is true and startup sampling decision is not set, start transaction with isStartup false`() { + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val date = SentryNanotimeDate(Date(1), 0) + setAppStartTime(date) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction( + any(), + check { + assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp()) + }, + eq(false) + ) + } + + @Test + fun `When firstActivityCreated is false and startup sampling decision is set, start transaction with isStartup false`() { + AppStartMetrics.getInstance().startupSamplingDecision = mock() + val sut = fixture.getSut { it.tracesSampleRate = 1.0 } + sut.register(fixture.hub, fixture.options) + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).startTransaction(any(), any(), eq(false)) + } + @Test fun `When firstActivityCreated is true, do not create app start span if not foregroundImportance`() { val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_BACKGROUND) @@ -697,7 +754,11 @@ class ActivityLifecycleIntegrationTest { sut.onActivityCreated(activity, fixture.bundle) // call only once - verify(fixture.hub).startTransaction(any(), check { assertNotEquals(date, it.startTimestamp) }) + verify(fixture.hub).startTransaction( + any(), + check { assertNotEquals(date, it.startTimestamp) }, + any() + ) } @Test diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 2f34b2a0662..dd79cfe9191 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -247,6 +247,12 @@ class AndroidOptionsInitializerTest { ) } + @Test + fun `getCacheDir returns sentry subfolder`() { + fixture.initSut() + assertTrue(AndroidOptionsInitializer.getCacheDir(fixture.context).path.endsWith("${File.separator}cache${File.separator}sentry")) + } + @Test fun `profilingTracesDirPath should be set at initialization`() { fixture.initSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 57410d49d09..405aa6dc98b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -170,6 +170,18 @@ class AndroidTransactionProfilerTest { assertEquals(1, profiler.transactionsCounter) } + @Test + fun `isStarted reflects inner counter`() { + val profiler = fixture.getSut(context) + profiler.start() + profiler.bindTransaction(fixture.transaction1) + assertEquals(1, profiler.transactionsCounter) + assertTrue(profiler.isRunning) + profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) + assertEquals(0, profiler.transactionsCounter) + assertFalse(profiler.isRunning) + } + @Test fun `profiler multiple starts are ignored`() { val profiler = fixture.getSut(context) @@ -491,4 +503,24 @@ class AndroidTransactionProfilerTest { val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) assertNull(profilingTraceData) } + + @Test + fun `profiler stops profiling on close, even if not bound to a transaction`() { + val profiler = fixture.getSut(context) + profiler.start() + assertEquals(1, profiler.transactionsCounter) + + profiler.close() + assertEquals(0, profiler.transactionsCounter) + + // The timeout scheduled job should be cleared + val androidProfiler = profiler.getProperty("profiler") + val scheduledJob = androidProfiler?.getProperty?>("scheduledFinish") + assertNull(scheduledJob) + + // Binding and calling transaction finish returns null, as the profiler was stopped + profiler.bindTransaction(fixture.transaction1) + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null, fixture.options) + assertNull(profilingTraceData) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index acc62be070a..beb273e8fc2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -1343,4 +1343,31 @@ class ManifestMetadataReaderTest { // Assert assertFalse(fixture.options.isEnablePerformanceV2) } + + @Test + fun `applyMetadata reads startupProfiling flag to options`() { + // Arrange + val bundle = bundleOf(ManifestMetadataReader.ENABLE_STARTUP_PROFILING to true) + val context = fixture.getContext(metaData = bundle) + fixture.options.profilesSampleRate = 1.0 + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertTrue(fixture.options.isEnableStartupProfiling) + } + + @Test + fun `applyMetadata reads startupProfiling flag to options and keeps default if not found`() { + // Arrange + val context = fixture.getContext() + fixture.options.profilesSampleRate = 1.0 + + // Act + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + + // Assert + assertFalse(fixture.options.isEnableStartupProfiling) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryStartupProfilingProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryStartupProfilingProviderTest.kt new file mode 100644 index 00000000000..c414361f268 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryStartupProfilingProviderTest.kt @@ -0,0 +1,214 @@ +package io.sentry.android.core + +import android.content.pm.ProviderInfo +import android.os.Build +import android.os.Bundle +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.ILogger +import io.sentry.JsonSerializer +import io.sentry.Sentry +import io.sentry.Sentry.STARTUP_PROFILING_CONFIG_FILE_NAME +import io.sentry.SentryLevel +import io.sentry.SentryOptions +import io.sentry.SentryStartupProfilingOptions +import io.sentry.android.core.performance.AppStartMetrics +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.io.File +import java.io.FileWriter +import java.nio.file.Files +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +@RunWith(AndroidJUnit4::class) +class SentryStartupProfilingProviderTest { + private var startupProfilingProvider = SentryStartupProfilingProvider() + + private lateinit var cache: File + private lateinit var sentryCache: File + private lateinit var traceDir: File + + private inner class Fixture { + val mockContext = ContextUtilsTestHelper.mockMetaData(metaData = Bundle()) + val providerInfo = ProviderInfo() + val logger = mock() + lateinit var configFile: File + + fun getSut(sdkVersion: Int = Build.VERSION_CODES.S, authority: String = AUTHORITY, handleFile: ((config: File) -> Unit)? = null): SentryStartupProfilingProvider { + val buildInfoProvider: BuildInfoProvider = mock() + whenever(buildInfoProvider.sdkInfoVersion).thenReturn(sdkVersion) + whenever(mockContext.cacheDir).thenReturn(cache) + whenever(mockContext.applicationContext).thenReturn(mockContext) + configFile = File(sentryCache, STARTUP_PROFILING_CONFIG_FILE_NAME) + handleFile?.invoke(configFile) + + providerInfo.authority = authority + return SentryStartupProfilingProvider(logger, buildInfoProvider).apply { + attachInfo(mockContext, providerInfo) + } + } + } + + private val fixture = Fixture() + + @BeforeTest + fun `set up`() { + AppStartMetrics.getInstance().clear() + cache = Files.createTempDirectory("sentry-disk-cache-test").toAbsolutePath().toFile() + sentryCache = File(cache, "sentry") + traceDir = File(sentryCache, "traces") + cache.mkdir() + sentryCache.mkdir() + traceDir.mkdir() + } + + @AfterTest + fun cleanup() { + AppStartMetrics.getInstance().clear() + cache.deleteRecursively() + Sentry.close() + } + + @Test + fun `when missing applicationId, SentryInitProvider throws`() { + assertFailsWith { + fixture.getSut(authority = SentryStartupProfilingProvider::class.java.name) + } + } + + @Test + fun `when config file does not exists, nothing happens`() { + fixture.getSut() + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when config file is not readable, nothing happens`() { + fixture.getSut { config -> + writeConfig(config) + config.setReadable(false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when SDK is lower than 21, nothing happens`() { + fixture.getSut(sdkVersion = Build.VERSION_CODES.KITKAT) { config -> + writeConfig(config) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when config file is empty, profiler is not started`() { + fixture.getSut { config -> + config.createNewFile() + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger).log( + eq(SentryLevel.WARNING), + eq("Unable to deserialize the SentryStartupProfilingOptions. Startup profiling will not start.") + ) + } + + @Test + fun `when profiling is disabled, profiler is not started`() { + fixture.getSut { config -> + writeConfig(config, profilingEnabled = false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + verify(fixture.logger).log( + eq(SentryLevel.INFO), + eq("Profiling is not enabled. Startup profiling will not start.") + ) + } + + @Test + fun `when trace is not sampled, profiler is not started and sample decision is stored`() { + fixture.getSut { config -> + writeConfig(config, traceSampled = false, profileSampled = true) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + // If trace is not sampled, profile is not sample, either + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when profile is not sampled, profiler is not started and sample decision is stored`() { + fixture.getSut { config -> + writeConfig(config, traceSampled = true, profileSampled = false) + } + assertNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + assertFalse(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger, never()).log(any(), any()) + } + + @Test + fun `when profiler starts, it is set in AppStartMetrics`() { + fixture.getSut { config -> + writeConfig(config) + } + assertNotNull(AppStartMetrics.getInstance().startupProfiler) + assertNotNull(AppStartMetrics.getInstance().startupSamplingDecision) + assertTrue(AppStartMetrics.getInstance().startupProfiler!!.isRunning) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.sampled) + assertTrue(AppStartMetrics.getInstance().startupSamplingDecision!!.profileSampled) + verify(fixture.logger).log( + eq(SentryLevel.DEBUG), + eq("Startup profiling started.") + ) + } + + @Test + fun `when provider is closed, profiler is stopped`() { + val provider = fixture.getSut { config -> + writeConfig(config) + } + provider.shutdown() + assertNotNull(AppStartMetrics.getInstance().startupProfiler) + assertFalse(AppStartMetrics.getInstance().startupProfiler!!.isRunning) + } + + private fun writeConfig( + configFile: File, + profilingEnabled: Boolean = true, + traceSampled: Boolean = true, + traceSampleRate: Double = 1.0, + profileSampled: Boolean = true, + profileSampleRate: Double = 1.0, + profilingTracesDirPath: String = traceDir.absolutePath + ) { + val startupProfilingOptions = SentryStartupProfilingOptions() + startupProfilingOptions.isProfilingEnabled = profilingEnabled + startupProfilingOptions.isTraceSampled = traceSampled + startupProfilingOptions.traceSampleRate = traceSampleRate + startupProfilingOptions.isProfileSampled = profileSampled + startupProfilingOptions.profileSampleRate = profileSampleRate + startupProfilingOptions.profilingTracesDirPath = profilingTracesDirPath + startupProfilingOptions.profilingTracesHz = 101 + JsonSerializer(SentryOptions.empty()).serialize(startupProfilingOptions, FileWriter(configFile)) + } + + companion object { + private const val AUTHORITY = "io.sentry.sample.SentryStartupProfilingProvider" + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt index bec98f04fc1..c66293c55dc 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/performance/AppStartMetricsTest.kt @@ -12,6 +12,7 @@ import org.mockito.kotlin.mock import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -43,6 +44,8 @@ class AppStartMetricsTest { metrics.addActivityLifecycleTimeSpans(ActivityLifecycleTimeSpan()) AppStartMetrics.onApplicationCreate(mock()) AppStartMetrics.onContentProviderCreate(mock()) + metrics.setStartupProfiler(mock()) + metrics.startupSamplingDecision = mock() metrics.clear() @@ -50,10 +53,11 @@ class AppStartMetricsTest { assertTrue(metrics.sdkInitTimeSpan.hasNotStarted()) assertTrue(metrics.applicationOnCreateTimeSpan.hasNotStarted()) assertEquals(AppStartMetrics.AppStartType.UNKNOWN, metrics.appStartType) - assertTrue(metrics.applicationOnCreateTimeSpan.hasNotStarted()) assertTrue(metrics.activityLifecycleTimeSpans.isEmpty()) assertTrue(metrics.contentProviderOnCreateTimeSpans.isEmpty()) + assertNull(metrics.startupProfiler) + assertNull(metrics.startupSamplingDecision) } @Test diff --git a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml index ad6889e253f..1df500b3f6c 100644 --- a/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml +++ b/sentry-samples/sentry-samples-android/src/main/AndroidManifest.xml @@ -109,6 +109,8 @@ + + diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index eb3d91b37ba..11050010099 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -453,6 +453,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public fun setUser (Lio/sentry/protocol/User;)V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;Z)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -504,6 +505,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public fun setUser (Lio/sentry/protocol/User;)V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;Z)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -600,6 +602,7 @@ public abstract interface class io/sentry/IHub { 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 abstract fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;Z)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 traceHeaders ()Lio/sentry/SentryTraceHeader; @@ -800,6 +803,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { public abstract interface class io/sentry/ITransactionProfiler { public abstract fun bindTransaction (Lio/sentry/ITransaction;)V public abstract fun close ()V + public abstract fun isRunning ()Z public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;Lio/sentry/SentryOptions;)Lio/sentry/ProfilingTraceData; public abstract fun start ()V } @@ -1142,6 +1146,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public fun setUser (Lio/sentry/protocol/User;)V public fun startSession ()V public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;)Lio/sentry/ITransaction; + public fun startTransaction (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;Z)Lio/sentry/ITransaction; public fun traceHeaders ()Lio/sentry/SentryTraceHeader; public fun withScope (Lio/sentry/ScopeCallback;)V } @@ -1307,6 +1312,7 @@ public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionPro public fun bindTransaction (Lio/sentry/ITransaction;)V public fun close ()V public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; + public fun isRunning ()Z public fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;Lio/sentry/SentryOptions;)Lio/sentry/ProfilingTraceData; public fun start ()V } @@ -1635,6 +1641,7 @@ public final class io/sentry/SendFireAndForgetOutboxSender : io/sentry/SendCache } public final class io/sentry/Sentry { + public static final field STARTUP_PROFILING_CONFIG_FILE_NAME Ljava/lang/String; public static fun addBreadcrumb (Lio/sentry/Breadcrumb;)V public static fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V public static fun addBreadcrumb (Ljava/lang/String;)V @@ -1692,6 +1699,7 @@ public final class io/sentry/Sentry { 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 (Lio/sentry/TransactionContext;Lio/sentry/TransactionOptions;Z)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; @@ -2090,6 +2098,7 @@ public class io/sentry/SentryOptions { public fun addOptionsObserver (Lio/sentry/IOptionsObserver;)V public fun addScopeObserver (Lio/sentry/IScopeObserver;)V public fun addTracingOrigin (Ljava/lang/String;)V + public static fun empty ()Lio/sentry/SentryOptions; public fun getBackpressureMonitor ()Lio/sentry/backpressure/IBackpressureMonitor; public fun getBeforeBreadcrumb ()Lio/sentry/SentryOptions$BeforeBreadcrumbCallback; public fun getBeforeSend ()Lio/sentry/SentryOptions$BeforeSendCallback; @@ -2140,6 +2149,7 @@ public class io/sentry/SentryOptions { public fun getProfilesSampleRate ()Ljava/lang/Double; public fun getProfilesSampler ()Lio/sentry/SentryOptions$ProfilesSamplerCallback; public fun getProfilingTracesDirPath ()Ljava/lang/String; + public fun getProfilingTracesHz ()I public fun getProguardUuid ()Ljava/lang/String; public fun getProxy ()Lio/sentry/SentryOptions$Proxy; public fun getReadTimeoutMillis ()I @@ -2244,6 +2254,7 @@ public class io/sentry/SentryOptions { public fun setProfilesSampleRate (Ljava/lang/Double;)V public fun setProfilesSampler (Lio/sentry/SentryOptions$ProfilesSamplerCallback;)V public fun setProfilingEnabled (Z)V + public fun setProfilingTracesHz (I)V public fun setProguardUuid (Ljava/lang/String;)V public fun setProxy (Lio/sentry/SentryOptions$Proxy;)V public fun setReadTimeoutMillis (I)V @@ -2331,6 +2342,44 @@ public final class io/sentry/SentryStackTraceFactory { public fun isInApp (Ljava/lang/String;)Ljava/lang/Boolean; } +public final class io/sentry/SentryStartupProfilingOptions : io/sentry/JsonSerializable, io/sentry/JsonUnknown { + public fun ()V + public fun getProfileSampleRate ()Ljava/lang/Double; + public fun getProfilingTracesDirPath ()Ljava/lang/String; + public fun getProfilingTracesHz ()I + public fun getTraceSampleRate ()Ljava/lang/Double; + public fun getUnknown ()Ljava/util/Map; + public fun isProfileSampled ()Z + public fun isProfilingEnabled ()Z + public fun isTraceSampled ()Z + public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V + public fun setProfileSampleRate (Ljava/lang/Double;)V + public fun setProfileSampled (Z)V + public fun setProfilingEnabled (Z)V + public fun setProfilingTracesDirPath (Ljava/lang/String;)V + public fun setProfilingTracesHz (I)V + public fun setTraceSampleRate (Ljava/lang/Double;)V + public fun setTraceSampled (Z)V + public fun setUnknown (Ljava/util/Map;)V +} + +public final class io/sentry/SentryStartupProfilingOptions$Deserializer : io/sentry/JsonDeserializer { + public fun ()V + public fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Lio/sentry/SentryStartupProfilingOptions; + public synthetic fun deserialize (Lio/sentry/JsonObjectReader;Lio/sentry/ILogger;)Ljava/lang/Object; +} + +public final class io/sentry/SentryStartupProfilingOptions$JsonKeys { + public static final field IS_PROFILING_ENABLED Ljava/lang/String; + public static final field PROFILE_SAMPLED Ljava/lang/String; + public static final field PROFILE_SAMPLE_RATE Ljava/lang/String; + public static final field PROFILING_TRACES_DIR_PATH Ljava/lang/String; + public static final field PROFILING_TRACES_HZ Ljava/lang/String; + public static final field TRACE_SAMPLED Ljava/lang/String; + public static final field TRACE_SAMPLE_RATE Ljava/lang/String; + public fun ()V +} + public final class io/sentry/SentryThreadFactory { public fun (Lio/sentry/SentryStackTraceFactory;Lio/sentry/SentryOptions;)V } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 60857c20ef1..75050339e2e 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -700,12 +700,21 @@ public void flush(long timeoutMillis) { public @NotNull ITransaction startTransaction( final @NotNull TransactionContext transactionContext, final @NotNull TransactionOptions transactionOptions) { - return createTransaction(transactionContext, transactionOptions); + return startTransaction(transactionContext, transactionOptions, false); + } + + @Override + public @NotNull ITransaction startTransaction( + final @NotNull TransactionContext transactionContext, + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction) { + return createTransaction(transactionContext, transactionOptions, isStartupTransaction); } private @NotNull ITransaction createTransaction( final @NotNull TransactionContext transactionContext, - final @NotNull TransactionOptions transactionOptions) { + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction) { Objects.requireNonNull(transactionContext, "transactionContext is required"); ITransaction transaction; @@ -745,8 +754,14 @@ public void flush(long timeoutMillis) { // stop it if (samplingDecision.getSampled() && samplingDecision.getProfileSampled()) { final ITransactionProfiler transactionProfiler = options.getTransactionProfiler(); - transactionProfiler.start(); - transactionProfiler.bindTransaction(transaction); + // If the profiler is not running, we start and bind it here. + if (!transactionProfiler.isRunning()) { + transactionProfiler.start(); + transactionProfiler.bindTransaction(transaction); + } else if (isStartupTransaction) { + // If the profiler is running and the current transaction is the app startup, we bind it. + transactionProfiler.bindTransaction(transaction); + } } } if (transactionOptions.isBindToScope()) { diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 68a9bdf11dd..7efbdcffc5d 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -200,6 +200,14 @@ public void flush(long timeoutMillis) { return Sentry.startTransaction(transactionContext, transactionOptions); } + @Override + public @NotNull ITransaction startTransaction( + final @NotNull TransactionContext transactionContext, + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction) { + return Sentry.startTransaction(transactionContext, transactionOptions, isStartupTransaction); + } + @Deprecated @Override public @Nullable SentryTraceHeader traceHeaders() { diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index 01431043f09..da365270300 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -459,6 +459,21 @@ ITransaction startTransaction( final @NotNull TransactionContext transactionContext, final @NotNull TransactionOptions transactionOptions); + /** + * Creates a Transaction and returns the instance. Based on the passed transaction context and + * transaction options the decision if transaction is sampled will be taken by {@link + * TracesSampler}. + * + * @param transactionContext the transaction context + * @param transactionOptions the transaction options + * @return created transaction. + */ + @NotNull + ITransaction startTransaction( + final @NotNull TransactionContext transactionContext, + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction); + /** * Returns the "sentry-trace" header that allows tracing across services. Can also be used in * <meta> HTML tags. Also see {@link IHub#getBaggage()}. diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index 2eaec510b50..8b283fe47ee 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -8,6 +8,8 @@ /** Used for performing operations when a transaction is started or ended. */ @ApiStatus.Internal public interface ITransactionProfiler { + boolean isRunning(); + void start(); void bindTransaction(@NotNull ITransaction transaction); diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index d186c69ca2f..55c8f2a7db2 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -160,6 +160,14 @@ public void flush(long timeoutMillis) {} return NoOpTransaction.getInstance(); } + @Override + public @NotNull ITransaction startTransaction( + final @NotNull TransactionContext transactionContext, + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction) { + return NoOpTransaction.getInstance(); + } + @Override @Deprecated @SuppressWarnings("InlineMeSuggester") diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index 79d5832c1e7..b0aa1fdc2ea 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -17,6 +17,11 @@ public static NoOpTransactionProfiler getInstance() { @Override public void start() {} + @Override + public boolean isRunning() { + return false; + } + @Override public void bindTransaction(@NotNull ITransaction transaction) {} diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index dfd51c9db7d..c0e353b50ee 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -50,9 +50,13 @@ private Sentry() {} /** whether to use a single (global) Hub as opposed to one per thread. */ private static volatile boolean globalHubMode = GLOBAL_HUB_DEFAULT_MODE; - private static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = + @ApiStatus.Internal + public static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = "startup_profiling_config"; + /** Timestamp used to check old profiles to delete. */ + private static final long classCreationTimestamp = System.currentTimeMillis(); + /** * Returns the current (threads) hub, if none, clones the mainHub and returns it. * @@ -254,42 +258,51 @@ private static synchronized void init( private static void handleStartupProfilingConfig( final @NotNull SentryOptions options, final @NotNull ISentryExecutorService sentryExecutorService) { - sentryExecutorService.submit( - () -> { - final String cacheDirPath = options.getCacheDirPathWithoutDsn(); - if (cacheDirPath != null) { - final @NotNull File startupProfilingConfigFile = - new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); - // We always delete the config file for startup profiling - FileUtils.deleteRecursively(startupProfilingConfigFile); - if (!options.isEnableStartupProfiling()) { - return; - } - if (!options.isTracingEnabled()) { - options - .getLogger() - .log( - SentryLevel.INFO, - "Tracing is disabled and startup profiling will not start."); - return; - } - try { - if (startupProfilingConfigFile.createNewFile()) { - final @NotNull TracesSamplingDecision startupSamplingDecision = - sampleStartupProfiling(options); - final @NotNull SentryStartupProfilingOptions startupProfilingOptions = - new SentryStartupProfilingOptions(options, startupSamplingDecision); - try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) { - options.getSerializer().serialize(startupProfilingOptions, fileWriter); + try { + sentryExecutorService.submit( + () -> { + final String cacheDirPath = options.getCacheDirPathWithoutDsn(); + if (cacheDirPath != null) { + final @NotNull File startupProfilingConfigFile = + new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); + // We always delete the config file for startup profiling + FileUtils.deleteRecursively(startupProfilingConfigFile); + if (!options.isEnableStartupProfiling()) { + return; + } + if (!options.isTracingEnabled()) { + options + .getLogger() + .log( + SentryLevel.INFO, + "Tracing is disabled and startup profiling will not start."); + return; + } + try { + if (startupProfilingConfigFile.createNewFile()) { + final @NotNull TracesSamplingDecision startupSamplingDecision = + sampleStartupProfiling(options); + final @NotNull SentryStartupProfilingOptions startupProfilingOptions = + new SentryStartupProfilingOptions(options, startupSamplingDecision); + try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) { + options.getSerializer().serialize(startupProfilingOptions, fileWriter); + } } + } catch (IOException e) { + options + .getLogger() + .log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); } - } catch (IOException e) { - options - .getLogger() - .log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); } - } - }); + }); + } catch (RejectedExecutionException e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Startup profiling config will not be changed. Did you call Sentry.close()?", + e); + } } private static @NotNull TracesSamplingDecision sampleStartupProfiling( @@ -393,7 +406,6 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) final File profilingTracesDir = new File(profilingTracesDirPath); profilingTracesDir.mkdirs(); - final long timestamp = System.currentTimeMillis(); try { options @@ -405,7 +417,7 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) // Method trace files are normally deleted at the end of traces, but if that fails // for some reason we try to clear any old files here. for (File f : oldTracesDirContent) { - if (f.lastModified() < timestamp) { + if (f.lastModified() < classCreationTimestamp) { FileUtils.deleteRecursively(f); } } @@ -887,6 +899,22 @@ public static void endSession() { return getCurrentHub().startTransaction(transactionContext, transactionOptions); } + /** + * Creates a Transaction and returns the instance. + * + * @param transactionContext the transaction context + * @param transactionOptions options for the transaction + * @param isStartupTransaction if the transaction is the app startup one + * @return created transaction. + */ + public static @NotNull ITransaction startTransaction( + final @NotNull TransactionContext transactionContext, + final @NotNull TransactionOptions transactionOptions, + final boolean isStartupTransaction) { + return getCurrentHub() + .startTransaction(transactionContext, transactionOptions, isStartupTransaction); + } + /** * Returns the "sentry-trace" header that allows tracing across services. Can also be used in * <meta> HTML tags. Also see {@link Sentry#getBaggage()}. diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 3486d7f9ef3..487f2e7e0e7 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -450,6 +450,13 @@ public class SentryOptions { /** Whether to enable startup profiling, depending on profilesSampler or profilesSampleRate. */ private boolean enableStartupProfiling = false; + /** + * Profiling traces rate. 101 hz means 101 traces in 1 second. Defaults to 101 to avoid possible + * lockstep sampling. More on + * https://stackoverflow.com/questions/45470758/what-is-lockstep-sampling + */ + private int profilingTracesHz = 101; + /** * Adds an event processor * @@ -2251,6 +2258,22 @@ public void setEnableBackpressureHandling(final boolean enableBackpressureHandli this.enableBackpressureHandling = enableBackpressureHandling; } + /** + * Returns the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. + * + * @return Rate the profiler will sample rates at. + */ + @ApiStatus.Internal + public int getProfilingTracesHz() { + return profilingTracesHz; + } + + /** Sets the rate the profiler will sample rates at. 100 hz means 100 traces in 1 second. */ + @ApiStatus.Internal + public void setProfilingTracesHz(final int profilingTracesHz) { + this.profilingTracesHz = profilingTracesHz; + } + @ApiStatus.Experimental public boolean isEnableBackpressureHandling() { return enableBackpressureHandling; @@ -2334,7 +2357,8 @@ public interface ProfilesSamplerCallback { * * @return SentryOptions */ - static @NotNull SentryOptions empty() { + @ApiStatus.Internal + public static @NotNull SentryOptions empty() { return new SentryOptions(true); } diff --git a/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java index c630e80d4ff..08e405c500e 100644 --- a/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java +++ b/sentry/src/main/java/io/sentry/SentryStartupProfilingOptions.java @@ -4,10 +4,13 @@ import java.io.IOException; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; -final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { +@ApiStatus.Internal +public final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { boolean profileSampled; @Nullable Double profileSampleRate; @@ -15,16 +18,19 @@ final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializab @Nullable Double traceSampleRate; @Nullable String profilingTracesDirPath; boolean isProfilingEnabled; + int profilingTracesHz; private @Nullable Map unknown; - SentryStartupProfilingOptions() { + @VisibleForTesting + public SentryStartupProfilingOptions() { traceSampled = false; traceSampleRate = null; profileSampled = false; profileSampleRate = null; profilingTracesDirPath = null; isProfilingEnabled = false; + profilingTracesHz = 0; } SentryStartupProfilingOptions( @@ -36,6 +42,63 @@ final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializab profileSampleRate = samplingDecision.getProfileSampleRate(); profilingTracesDirPath = options.getProfilingTracesDirPath(); isProfilingEnabled = options.isProfilingEnabled(); + profilingTracesHz = options.getProfilingTracesHz(); + } + + public void setProfileSampled(final boolean profileSampled) { + this.profileSampled = profileSampled; + } + + public boolean isProfileSampled() { + return profileSampled; + } + + public void setProfileSampleRate(final @Nullable Double profileSampleRate) { + this.profileSampleRate = profileSampleRate; + } + + public @Nullable Double getProfileSampleRate() { + return profileSampleRate; + } + + public void setTraceSampled(final boolean traceSampled) { + this.traceSampled = traceSampled; + } + + public boolean isTraceSampled() { + return traceSampled; + } + + public void setTraceSampleRate(final @Nullable Double traceSampleRate) { + this.traceSampleRate = traceSampleRate; + } + + public @Nullable Double getTraceSampleRate() { + return traceSampleRate; + } + + public void setProfilingTracesDirPath(final @Nullable String profilingTracesDirPath) { + this.profilingTracesDirPath = profilingTracesDirPath; + } + + public @Nullable String getProfilingTracesDirPath() { + return profilingTracesDirPath; + } + + public void setProfilingEnabled(final boolean profilingEnabled) { + isProfilingEnabled = profilingEnabled; + } + + public boolean isProfilingEnabled() { + return isProfilingEnabled; + } + + public void setProfilingTracesHz(final int profilingTracesHz) { + this.profilingTracesHz = profilingTracesHz; + } + + public int getProfilingTracesHz() { + return profilingTracesHz; } // JsonSerializable @@ -47,6 +110,7 @@ public static final class JsonKeys { public static final String TRACE_SAMPLE_RATE = "trace_sample_rate"; public static final String PROFILING_TRACES_DIR_PATH = "profiling_traces_dir_path"; public static final String IS_PROFILING_ENABLED = "is_profiling_enabled"; + public static final String PROFILING_TRACES_HZ = "profiling_traces_hz"; } @Override @@ -59,6 +123,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger writer.name(JsonKeys.TRACE_SAMPLE_RATE).value(logger, traceSampleRate); writer.name(JsonKeys.PROFILING_TRACES_DIR_PATH).value(logger, profilingTracesDirPath); writer.name(JsonKeys.IS_PROFILING_ENABLED).value(logger, isProfilingEnabled); + writer.name(JsonKeys.PROFILING_TRACES_HZ).value(logger, profilingTracesHz); if (unknown != null) { for (String key : unknown.keySet()) { @@ -130,6 +195,12 @@ public static final class Deserializer options.isProfilingEnabled = isProfilingEnabled; } break; + case JsonKeys.PROFILING_TRACES_HZ: + Integer profilingTracesHz = reader.nextIntegerOrNull(); + if (profilingTracesHz != null) { + options.profilingTracesHz = profilingTracesHz; + } + break; default: if (unknown == null) { unknown = new ConcurrentHashMap<>(); diff --git a/sentry/src/test/java/io/sentry/HubAdapterTest.kt b/sentry/src/test/java/io/sentry/HubAdapterTest.kt index df4c0bdbcc5..9144c9f1cf2 100644 --- a/sentry/src/test/java/io/sentry/HubAdapterTest.kt +++ b/sentry/src/test/java/io/sentry/HubAdapterTest.kt @@ -214,6 +214,16 @@ class HubAdapterTest { HubAdapter.getInstance().startTransaction(transactionContext, transactionOptions) verify(hub).startTransaction(eq(transactionContext), eq(transactionOptions)) + + reset(hub) + + HubAdapter.getInstance().startTransaction(transactionContext, transactionOptions, true) + verify(hub).startTransaction(eq(transactionContext), eq(transactionOptions), eq(true)) + + reset(hub) + + HubAdapter.getInstance().startTransaction(transactionContext, transactionOptions, false) + verify(hub).startTransaction(eq(transactionContext), eq(transactionOptions), eq(false)) } @Test fun `traceHeaders calls Hub`() { diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 067274ec9a0..860e112db01 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1503,6 +1503,48 @@ class HubTest { transaction.finish() verify(mockClient, never()).captureEnvelope(any()) } + + @Test + fun `when profiler is running and isStartupTransaction is false, startTransaction does not interact with profiler`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(true) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + hub.startTransaction(context, TransactionOptions(), false) + verify(mockTransactionProfiler, never()).start() + verify(mockTransactionProfiler, never()).bindTransaction(any()) + } + + @Test + fun `when profiler is running and isStartupTransaction is true, startTransaction binds current profile`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(true) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + val transaction = hub.startTransaction(context, TransactionOptions(), true) + verify(mockTransactionProfiler, never()).start() + verify(mockTransactionProfiler).bindTransaction(eq(transaction)) + } + + @Test + fun `when profiler is not running, startTransaction starts and binds current profile`() { + val mockTransactionProfiler = mock() + whenever(mockTransactionProfiler.isRunning).thenReturn(false) + val hub = generateHub { + it.profilesSampleRate = 1.0 + it.setTransactionProfiler(mockTransactionProfiler) + } + val context = TransactionContext("name", "op") + val transaction = hub.startTransaction(context, TransactionOptions(), false) + verify(mockTransactionProfiler).start() + verify(mockTransactionProfiler).bindTransaction(eq(transaction)) + } //endregion //region startTransaction tests diff --git a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt index 2af7250d835..8ff50e94d82 100644 --- a/sentry/src/test/java/io/sentry/JsonSerializerTest.kt +++ b/sentry/src/test/java/io/sentry/JsonSerializerTest.kt @@ -978,7 +978,7 @@ class JsonSerializerTest { val actual = serializeToString(startupProfilingOptions) val expected = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\":false," + - "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + "\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false,\"profiling_traces_hz\":65}" assertEquals(expected, actual) } @@ -986,7 +986,7 @@ class JsonSerializerTest { @Test fun `deserializing SentryStartupProfilingOptions`() { val jsonStartupProfilingOptions = "{\"profile_sampled\":true,\"profile_sample_rate\":0.8,\"trace_sampled\"" + - ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false}" + ":false,\"trace_sample_rate\":0.1,\"profiling_traces_dir_path\":null,\"is_profiling_enabled\":false,\"profiling_traces_hz\":65}" val actual = fixture.serializer.deserialize(StringReader(jsonStartupProfilingOptions), SentryStartupProfilingOptions::class.java) assertNotNull(actual) @@ -995,6 +995,8 @@ class JsonSerializerTest { assertEquals(startupProfilingOptions.profileSampled, actual.profileSampled) assertEquals(startupProfilingOptions.profileSampleRate, actual.profileSampleRate) assertEquals(startupProfilingOptions.isProfilingEnabled, actual.isProfilingEnabled) + assertEquals(startupProfilingOptions.profilingTracesHz, actual.profilingTracesHz) + assertEquals(startupProfilingOptions.profilingTracesDirPath, actual.profilingTracesDirPath) assertNull(actual.unknown) } @@ -1280,6 +1282,7 @@ class JsonSerializerTest { profileSampled = true profileSampleRate = 0.8 isProfilingEnabled = false + profilingTracesHz = 65 } private fun createSpan(): ISpan { diff --git a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt index 186ee39f2a4..372ccc747d5 100644 --- a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt @@ -2,6 +2,7 @@ package io.sentry import org.mockito.kotlin.mock import kotlin.test.Test +import kotlin.test.assertFalse import kotlin.test.assertNull class NoOpTransactionProfilerTest { @@ -15,6 +16,11 @@ class NoOpTransactionProfilerTest { fun `bindTransaction does not throw`() = profiler.bindTransaction(mock()) + @Test + fun `isRunning returns false`() { + assertFalse(profiler.isRunning) + } + @Test fun `onTransactionFinish does returns null`() { assertNull(profiler.onTransactionFinish(NoOpTransaction.getInstance(), null, mock())) diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index dd02b29dea0..634e7133062 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -574,4 +574,16 @@ class SentryOptionsTest { options.profilesSampleRate = 0.0 assertFalse(options.isEnableStartupProfiling) } + + @Test + fun `when options are initialized, profilingTracesHz is set to 101 by default`() { + assertEquals(101, SentryOptions().profilingTracesHz) + } + + @Test + fun `when setProfilingTracesHz is called, overrides default`() { + val options = SentryOptions() + options.profilingTracesHz = 13 + assertEquals(13, options.profilingTracesHz) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 7fa8fbafb70..6f84af6cf56 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -945,11 +945,11 @@ class SentryTest { @Test fun `backpressure monitor is a NoOp if handling is disabled`() { var sentryOptions: SentryOptions? = null - Sentry.init({ + Sentry.init { it.dsn = dsn it.isEnableBackpressureHandling = false sentryOptions = it - }) + } assertIs(sentryOptions?.backpressureMonitor) } @@ -957,11 +957,11 @@ class SentryTest { fun `backpressure monitor is set if handling is enabled`() { var sentryOptions: SentryOptions? = null - Sentry.init({ + Sentry.init { it.dsn = dsn it.isEnableBackpressureHandling = true sentryOptions = it - }) + } assertIs(sentryOptions?.backpressureMonitor) } From 186c8df10d62b50320ac4ee91c4ca7cced3899c5 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 4 Jan 2024 17:57:51 +0100 Subject: [PATCH 2/2] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4131f014bd0..da767f1fc0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Handle `monitor`/`check_in` in client reports and rate limiter ([#3096](https://github.com/getsentry/sentry-java/pull/3096)) - Startup profiling 1 - Decouple Profiler from Transaction ([#3101](https://github.com/getsentry/sentry-java/pull/3101)) - Startup profiling 2 - Add options and sampling logic ([#3121](https://github.com/getsentry/sentry-java/pull/3121)) +- Startup Profiling 3 - Add ContentProvider and start profile ([#3128](https://github.com/getsentry/sentry-java/pull/3128)) ### Fixes