Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Start performance collection on AppStart continuous profiling ([#4752](https://github.com/getsentry/sentry-java/pull/4752))

## 8.22.0

### Features
Expand Down
1 change: 1 addition & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver {
public fun <init> (Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ILogger;Ljava/lang/String;ILio/sentry/ISentryExecutorService;)V
public fun close (Z)V
public fun getChunkId ()Lio/sentry/protocol/SentryId;
public fun getProfilerId ()Lio/sentry/protocol/SentryId;
public fun getRootSpanCounter ()I
public fun isRunning ()Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ private void start() {

isRunning = true;

if (profilerId == SentryId.EMPTY_ID) {
if (profilerId.equals(SentryId.EMPTY_ID)) {
profilerId = new SentryId();
}

if (chunkId == SentryId.EMPTY_ID) {
if (chunkId.equals(SentryId.EMPTY_ID)) {
chunkId = new SentryId();
}

Expand Down Expand Up @@ -344,6 +344,11 @@ public void close(final boolean isTerminating) {
return profilerId;
}

@Override
public @NotNull SentryId getChunkId() {
return chunkId;
}

private void sendChunks(final @NotNull IScopes scopes, final @NotNull SentryOptions options) {
try {
options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import android.app.Application;
import android.content.Context;
import android.content.pm.PackageInfo;
import io.sentry.CompositePerformanceCollector;
import io.sentry.DeduplicateMultithreadedEventProcessor;
import io.sentry.DefaultCompositePerformanceCollector;
import io.sentry.DefaultVersionDetector;
Expand Down Expand Up @@ -45,6 +46,7 @@
import io.sentry.internal.gestures.GestureTargetLocator;
import io.sentry.internal.modules.NoOpModulesLoader;
import io.sentry.internal.viewhierarchy.ViewHierarchyExporter;
import io.sentry.protocol.SentryId;
import io.sentry.transport.CurrentDateProvider;
import io.sentry.transport.NoOpEnvelopeCache;
import io.sentry.transport.NoOpTransportGate;
Expand Down Expand Up @@ -180,26 +182,6 @@ static void initializeIntegrationsAndProcessors(
options.setTransportGate(new AndroidTransportGate(options));
}

// Check if the profiler was already instantiated in the app start.
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
// state of the profiler, and it's only possible retaining the instance.
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
final @Nullable ITransactionProfiler appStartTransactionProfiler;
final @Nullable IContinuousProfiler appStartContinuousProfiler;
try (final @NotNull ISentryLifecycleToken ignored = AppStartMetrics.staticLock.acquire()) {
appStartTransactionProfiler = appStartMetrics.getAppStartProfiler();
appStartContinuousProfiler = appStartMetrics.getAppStartContinuousProfiler();
appStartMetrics.setAppStartProfiler(null);
appStartMetrics.setAppStartContinuousProfiler(null);
}

setupProfiler(
options,
context,
buildInfoProvider,
appStartTransactionProfiler,
appStartContinuousProfiler);

if (options.getModulesLoader() instanceof NoOpModulesLoader) {
options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger()));
}
Expand Down Expand Up @@ -262,6 +244,27 @@ static void initializeIntegrationsAndProcessors(
if (options.getCompositePerformanceCollector() instanceof NoOpCompositePerformanceCollector) {
options.setCompositePerformanceCollector(new DefaultCompositePerformanceCollector(options));
}

// Check if the profiler was already instantiated in the app start.
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
// state of the profiler, and it's only possible retaining the instance.
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
Comment thread
stefanosiano marked this conversation as resolved.
Outdated
final @Nullable ITransactionProfiler appStartTransactionProfiler;
final @Nullable IContinuousProfiler appStartContinuousProfiler;
try (final @NotNull ISentryLifecycleToken ignored = AppStartMetrics.staticLock.acquire()) {
appStartTransactionProfiler = appStartMetrics.getAppStartProfiler();
appStartContinuousProfiler = appStartMetrics.getAppStartContinuousProfiler();
appStartMetrics.setAppStartProfiler(null);
appStartMetrics.setAppStartContinuousProfiler(null);
}

setupProfiler(
options,
context,
buildInfoProvider,
appStartTransactionProfiler,
appStartContinuousProfiler,
options.getCompositePerformanceCollector());
}

/** Setup the correct profiler (transaction or continuous) based on the options. */
Expand All @@ -270,7 +273,8 @@ private static void setupProfiler(
final @NotNull Context context,
final @NotNull BuildInfoProvider buildInfoProvider,
final @Nullable ITransactionProfiler appStartTransactionProfiler,
final @Nullable IContinuousProfiler appStartContinuousProfiler) {
final @Nullable IContinuousProfiler appStartContinuousProfiler,
final @NotNull CompositePerformanceCollector performanceCollector) {
if (options.isProfilingEnabled() || options.getProfilesSampleRate() != null) {
options.setContinuousProfiler(NoOpContinuousProfiler.getInstance());
// This is a safeguard, but it should never happen, as the app start profiler should be the
Expand Down Expand Up @@ -299,6 +303,12 @@ private static void setupProfiler(
}
if (appStartContinuousProfiler != null) {
options.setContinuousProfiler(appStartContinuousProfiler);
// If the profiler is running, we start the performance collector too, otherwise we'd miss
// measurements in app launch profiles
final @NotNull SentryId chunkId = appStartContinuousProfiler.getChunkId();
if (appStartContinuousProfiler.isRunning() && !chunkId.equals(SentryId.EMPTY_ID)) {
performanceCollector.start(chunkId.toString());
}
} else {
options.setContinuousProfiler(
new AndroidContinuousProfiler(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import kotlin.test.Test
import kotlin.test.assertContains
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
import kotlin.test.assertTrue
Expand Down Expand Up @@ -167,9 +168,13 @@ class AndroidContinuousProfilerTest {
// We are scheduling the profiler to stop at the end of the chunk, so it should still be running
profiler.stopProfiler(ProfileLifecycle.MANUAL)
assertTrue(profiler.isRunning)
assertNotEquals(SentryId.EMPTY_ID, profiler.profilerId)
assertNotEquals(SentryId.EMPTY_ID, profiler.chunkId)
// We run the executor service to trigger the chunk finish, and the profiler shouldn't restart
fixture.executor.runAll()
assertFalse(profiler.isRunning)
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
}

@Test
Expand Down Expand Up @@ -397,6 +402,7 @@ class AndroidContinuousProfilerTest {
val profiler = fixture.getSut()
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
assertTrue(profiler.isRunning)
val oldChunkId = profiler.chunkId

fixture.executor.runAll()
verify(fixture.mockLogger)
Expand All @@ -407,6 +413,7 @@ class AndroidContinuousProfilerTest {
verify(fixture.mockLogger, times(2))
.log(eq(SentryLevel.DEBUG), eq("Profile chunk finished. Starting a new one."))
assertTrue(profiler.isRunning)
assertNotEquals(oldChunkId, profiler.chunkId)
}

@Test
Expand Down Expand Up @@ -508,6 +515,7 @@ class AndroidContinuousProfilerTest {
profiler.onRateLimitChanged(rateLimiter)
assertFalse(profiler.isRunning)
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
verify(fixture.mockLogger)
.log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
}
Expand All @@ -523,6 +531,7 @@ class AndroidContinuousProfilerTest {
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
assertFalse(profiler.isRunning)
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
verify(fixture.mockLogger)
.log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
}
Expand All @@ -541,6 +550,7 @@ class AndroidContinuousProfilerTest {
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
assertFalse(profiler.isRunning)
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
verify(fixture.mockLogger)
.log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler."))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.sentry.IConnectionStatusProvider
import io.sentry.IContinuousProfiler
import io.sentry.ILogger
import io.sentry.ISocketTagger
import io.sentry.ITransaction
import io.sentry.ITransactionProfiler
import io.sentry.MainEventProcessor
import io.sentry.NoOpContinuousProfiler
Expand All @@ -34,6 +35,7 @@ import io.sentry.cache.PersistingScopeObserver
import io.sentry.compose.gestures.ComposeGestureTargetLocator
import io.sentry.internal.debugmeta.IDebugMetaLoader
import io.sentry.internal.modules.IModulesLoader
import io.sentry.protocol.SentryId
import io.sentry.test.ImmediateExecutorService
import io.sentry.transport.ITransportGate
import io.sentry.util.thread.IThreadChecker
Expand Down Expand Up @@ -426,6 +428,33 @@ class AndroidOptionsInitializerTest {
assertNull(AppStartMetrics.getInstance().appStartContinuousProfiler)
}

@Test
fun `init starts performance collector if continuous profiler of appStartMetrics is running`() {
val appStartContinuousProfiler = mock<IContinuousProfiler>()
val mockPerformanceCollector = mock<CompositePerformanceCollector>()
val chunkId = SentryId()
whenever(appStartContinuousProfiler.isRunning()).thenReturn(true)
whenever(appStartContinuousProfiler.chunkId).thenReturn(chunkId)

AppStartMetrics.getInstance().appStartContinuousProfiler = appStartContinuousProfiler
fixture.initSut(configureOptions = { compositePerformanceCollector = mockPerformanceCollector })

verify(mockPerformanceCollector).start(eq(chunkId.toString()))
}

@Test
fun `init does not start performance collector if transaction profiler of appStartMetrics is running`() {
val appStartTransactionProfiler = mock<ITransactionProfiler>()
val mockPerformanceCollector = mock<CompositePerformanceCollector>()
whenever(appStartTransactionProfiler.isRunning()).thenReturn(true)

AppStartMetrics.getInstance().appStartProfiler = appStartTransactionProfiler
fixture.initSut(configureOptions = { compositePerformanceCollector = mockPerformanceCollector })

verify(mockPerformanceCollector, never()).start(any<String>())
verify(mockPerformanceCollector, never()).start(any<ITransaction>())
}

@Test
fun `init with transaction profiling closes continuous profiler of appStartMetrics`() {
val appStartContinuousProfiler = mock<IContinuousProfiler>()
Expand Down
2 changes: 2 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ public abstract interface class io/sentry/IConnectionStatusProvider$IConnectionS

public abstract interface class io/sentry/IContinuousProfiler {
public abstract fun close (Z)V
public abstract fun getChunkId ()Lio/sentry/protocol/SentryId;
public abstract fun getProfilerId ()Lio/sentry/protocol/SentryId;
public abstract fun isRunning ()Z
public abstract fun reevaluateSampling ()V
Expand Down Expand Up @@ -1471,6 +1472,7 @@ public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectio

public final class io/sentry/NoOpContinuousProfiler : io/sentry/IContinuousProfiler {
public fun close (Z)V
public fun getChunkId ()Lio/sentry/protocol/SentryId;
public static fun getInstance ()Lio/sentry/NoOpContinuousProfiler;
public fun getProfilerId ()Lio/sentry/protocol/SentryId;
public fun isRunning ()Z
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/IContinuousProfiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,7 @@ void startProfiler(

@NotNull
SentryId getProfilerId();

@NotNull
SentryId getChunkId();
}
5 changes: 5 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public void reevaluateSampling() {}
public @NotNull SentryId getProfilerId() {
return SentryId.EMPTY_ID;
}

@Override
public @NotNull SentryId getChunkId() {
return SentryId.EMPTY_ID;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class NoOpContinuousProfilerTest {
assertEquals(profiler.profilerId, SentryId.EMPTY_ID)
}

@Test
fun `getChunkId returns Empty SentryId`() {
assertEquals(profiler.chunkId, SentryId.EMPTY_ID)
}

@Test
fun `reevaluateSampling does not throw`() {
profiler.reevaluateSampling()
Expand Down
Loading