Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

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

### Improvements
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,25 +182,7 @@ 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 +246,26 @@ 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 @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 +274,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 +304,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