Skip to content

Commit 7d29d34

Browse files
lbloderlbloder
authored
Merge branch 'main' into feat/poc-continuous-profiling
2 parents 8e027d8 + 806307f commit 7d29d34

File tree

10 files changed

+93
-21
lines changed

10 files changed

+93
-21
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Fixes
66

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

910
### Improvements

‎sentry-android-core/api/sentry-android-core.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
4343
public class io/sentry/android/core/AndroidContinuousProfiler : io/sentry/IContinuousProfiler, io/sentry/transport/RateLimiter$IRateLimitObserver {
4444
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
4545
public fun close (Z)V
46+
public fun getChunkId ()Lio/sentry/protocol/SentryId;
4647
public fun getProfilerId ()Lio/sentry/protocol/SentryId;
4748
public fun getRootSpanCounter ()I
4849
public fun isRunning ()Z

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidContinuousProfiler.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,11 @@ private void start() {
208208

209209
isRunning = true;
210210

211-
if (profilerId == SentryId.EMPTY_ID) {
211+
if (profilerId.equals(SentryId.EMPTY_ID)) {
212212
profilerId = new SentryId();
213213
}
214214

215-
if (chunkId == SentryId.EMPTY_ID) {
215+
if (chunkId.equals(SentryId.EMPTY_ID)) {
216216
chunkId = new SentryId();
217217
}
218218

@@ -345,6 +345,11 @@ public void close(final boolean isTerminating) {
345345
return profilerId;
346346
}
347347

348+
@Override
349+
public @NotNull SentryId getChunkId() {
350+
return chunkId;
351+
}
352+
348353
private void sendChunks(final @NotNull IScopes scopes, final @NotNull SentryOptions options) {
349354
try {
350355
options

‎sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import android.app.Application;
66
import android.content.Context;
77
import android.content.pm.PackageInfo;
8+
import io.sentry.CompositePerformanceCollector;
89
import io.sentry.DeduplicateMultithreadedEventProcessor;
910
import io.sentry.DefaultCompositePerformanceCollector;
1011
import io.sentry.DefaultVersionDetector;
@@ -45,6 +46,7 @@
4546
import io.sentry.internal.gestures.GestureTargetLocator;
4647
import io.sentry.internal.modules.NoOpModulesLoader;
4748
import io.sentry.internal.viewhierarchy.ViewHierarchyExporter;
49+
import io.sentry.protocol.SentryId;
4850
import io.sentry.transport.CurrentDateProvider;
4951
import io.sentry.transport.NoOpEnvelopeCache;
5052
import io.sentry.transport.NoOpTransportGate;
@@ -180,25 +182,7 @@ static void initializeIntegrationsAndProcessors(
180182
options.setTransportGate(new AndroidTransportGate(options));
181183
}
182184

183-
// Check if the profiler was already instantiated in the app start.
184-
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
185-
// state of the profiler, and it's only possible retaining the instance.
186185
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
187-
final @Nullable ITransactionProfiler appStartTransactionProfiler;
188-
final @Nullable IContinuousProfiler appStartContinuousProfiler;
189-
try (final @NotNull ISentryLifecycleToken ignored = AppStartMetrics.staticLock.acquire()) {
190-
appStartTransactionProfiler = appStartMetrics.getAppStartProfiler();
191-
appStartContinuousProfiler = appStartMetrics.getAppStartContinuousProfiler();
192-
appStartMetrics.setAppStartProfiler(null);
193-
appStartMetrics.setAppStartContinuousProfiler(null);
194-
}
195-
196-
setupProfiler(
197-
options,
198-
context,
199-
buildInfoProvider,
200-
appStartTransactionProfiler,
201-
appStartContinuousProfiler);
202186

203187
if (options.getModulesLoader() instanceof NoOpModulesLoader) {
204188
options.setModulesLoader(new AssetsModulesLoader(context, options.getLogger()));
@@ -262,6 +246,26 @@ static void initializeIntegrationsAndProcessors(
262246
if (options.getCompositePerformanceCollector() instanceof NoOpCompositePerformanceCollector) {
263247
options.setCompositePerformanceCollector(new DefaultCompositePerformanceCollector(options));
264248
}
249+
250+
// Check if the profiler was already instantiated in the app start.
251+
// We use the Android profiler, that uses a global start/stop api, so we need to preserve the
252+
// state of the profiler, and it's only possible retaining the instance.
253+
final @Nullable ITransactionProfiler appStartTransactionProfiler;
254+
final @Nullable IContinuousProfiler appStartContinuousProfiler;
255+
try (final @NotNull ISentryLifecycleToken ignored = AppStartMetrics.staticLock.acquire()) {
256+
appStartTransactionProfiler = appStartMetrics.getAppStartProfiler();
257+
appStartContinuousProfiler = appStartMetrics.getAppStartContinuousProfiler();
258+
appStartMetrics.setAppStartProfiler(null);
259+
appStartMetrics.setAppStartContinuousProfiler(null);
260+
}
261+
262+
setupProfiler(
263+
options,
264+
context,
265+
buildInfoProvider,
266+
appStartTransactionProfiler,
267+
appStartContinuousProfiler,
268+
options.getCompositePerformanceCollector());
265269
}
266270

267271
/** Setup the correct profiler (transaction or continuous) based on the options. */
@@ -270,7 +274,8 @@ private static void setupProfiler(
270274
final @NotNull Context context,
271275
final @NotNull BuildInfoProvider buildInfoProvider,
272276
final @Nullable ITransactionProfiler appStartTransactionProfiler,
273-
final @Nullable IContinuousProfiler appStartContinuousProfiler) {
277+
final @Nullable IContinuousProfiler appStartContinuousProfiler,
278+
final @NotNull CompositePerformanceCollector performanceCollector) {
274279
if (options.isProfilingEnabled() || options.getProfilesSampleRate() != null) {
275280
options.setContinuousProfiler(NoOpContinuousProfiler.getInstance());
276281
// This is a safeguard, but it should never happen, as the app start profiler should be the
@@ -299,6 +304,12 @@ private static void setupProfiler(
299304
}
300305
if (appStartContinuousProfiler != null) {
301306
options.setContinuousProfiler(appStartContinuousProfiler);
307+
// If the profiler is running, we start the performance collector too, otherwise we'd miss
308+
// measurements in app launch profiles
309+
final @NotNull SentryId chunkId = appStartContinuousProfiler.getChunkId();
310+
if (appStartContinuousProfiler.isRunning() && !chunkId.equals(SentryId.EMPTY_ID)) {
311+
performanceCollector.start(chunkId.toString());
312+
}
302313
} else {
303314
options.setContinuousProfiler(
304315
new AndroidContinuousProfiler(

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidContinuousProfilerTest.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import kotlin.test.Test
3030
import kotlin.test.assertContains
3131
import kotlin.test.assertEquals
3232
import kotlin.test.assertFalse
33+
import kotlin.test.assertNotEquals
3334
import kotlin.test.assertNotNull
3435
import kotlin.test.assertNull
3536
import kotlin.test.assertTrue
@@ -167,9 +168,13 @@ class AndroidContinuousProfilerTest {
167168
// We are scheduling the profiler to stop at the end of the chunk, so it should still be running
168169
profiler.stopProfiler(ProfileLifecycle.MANUAL)
169170
assertTrue(profiler.isRunning)
171+
assertNotEquals(SentryId.EMPTY_ID, profiler.profilerId)
172+
assertNotEquals(SentryId.EMPTY_ID, profiler.chunkId)
170173
// We run the executor service to trigger the chunk finish, and the profiler shouldn't restart
171174
fixture.executor.runAll()
172175
assertFalse(profiler.isRunning)
176+
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
177+
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
173178
}
174179

175180
@Test
@@ -397,6 +402,7 @@ class AndroidContinuousProfilerTest {
397402
val profiler = fixture.getSut()
398403
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
399404
assertTrue(profiler.isRunning)
405+
val oldChunkId = profiler.chunkId
400406

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

412419
@Test
@@ -508,6 +515,7 @@ class AndroidContinuousProfilerTest {
508515
profiler.onRateLimitChanged(rateLimiter)
509516
assertFalse(profiler.isRunning)
510517
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
518+
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
511519
verify(fixture.mockLogger)
512520
.log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
513521
}
@@ -523,6 +531,7 @@ class AndroidContinuousProfilerTest {
523531
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
524532
assertFalse(profiler.isRunning)
525533
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
534+
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
526535
verify(fixture.mockLogger)
527536
.log(eq(SentryLevel.WARNING), eq("SDK is rate limited. Stopping profiler."))
528537
}
@@ -541,6 +550,7 @@ class AndroidContinuousProfilerTest {
541550
profiler.startProfiler(ProfileLifecycle.MANUAL, fixture.mockTracesSampler)
542551
assertFalse(profiler.isRunning)
543552
assertEquals(SentryId.EMPTY_ID, profiler.profilerId)
553+
assertEquals(SentryId.EMPTY_ID, profiler.chunkId)
544554
verify(fixture.mockLogger)
545555
.log(eq(SentryLevel.WARNING), eq("Device is offline. Stopping profiler."))
546556
}

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import io.sentry.IConnectionStatusProvider
1212
import io.sentry.IContinuousProfiler
1313
import io.sentry.ILogger
1414
import io.sentry.ISocketTagger
15+
import io.sentry.ITransaction
1516
import io.sentry.ITransactionProfiler
1617
import io.sentry.MainEventProcessor
1718
import io.sentry.NoOpContinuousProfiler
@@ -34,6 +35,7 @@ import io.sentry.cache.PersistingScopeObserver
3435
import io.sentry.compose.gestures.ComposeGestureTargetLocator
3536
import io.sentry.internal.debugmeta.IDebugMetaLoader
3637
import io.sentry.internal.modules.IModulesLoader
38+
import io.sentry.protocol.SentryId
3739
import io.sentry.test.ImmediateExecutorService
3840
import io.sentry.transport.ITransportGate
3941
import io.sentry.util.thread.IThreadChecker
@@ -426,6 +428,33 @@ class AndroidOptionsInitializerTest {
426428
assertNull(AppStartMetrics.getInstance().appStartContinuousProfiler)
427429
}
428430

431+
@Test
432+
fun `init starts performance collector if continuous profiler of appStartMetrics is running`() {
433+
val appStartContinuousProfiler = mock<IContinuousProfiler>()
434+
val mockPerformanceCollector = mock<CompositePerformanceCollector>()
435+
val chunkId = SentryId()
436+
whenever(appStartContinuousProfiler.isRunning()).thenReturn(true)
437+
whenever(appStartContinuousProfiler.chunkId).thenReturn(chunkId)
438+
439+
AppStartMetrics.getInstance().appStartContinuousProfiler = appStartContinuousProfiler
440+
fixture.initSut(configureOptions = { compositePerformanceCollector = mockPerformanceCollector })
441+
442+
verify(mockPerformanceCollector).start(eq(chunkId.toString()))
443+
}
444+
445+
@Test
446+
fun `init does not start performance collector if transaction profiler of appStartMetrics is running`() {
447+
val appStartTransactionProfiler = mock<ITransactionProfiler>()
448+
val mockPerformanceCollector = mock<CompositePerformanceCollector>()
449+
whenever(appStartTransactionProfiler.isRunning()).thenReturn(true)
450+
451+
AppStartMetrics.getInstance().appStartProfiler = appStartTransactionProfiler
452+
fixture.initSut(configureOptions = { compositePerformanceCollector = mockPerformanceCollector })
453+
454+
verify(mockPerformanceCollector, never()).start(any<String>())
455+
verify(mockPerformanceCollector, never()).start(any<ITransaction>())
456+
}
457+
429458
@Test
430459
fun `init with transaction profiling closes continuous profiler of appStartMetrics`() {
431460
val appStartContinuousProfiler = mock<IContinuousProfiler>()

‎sentry/api/sentry.api‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,7 @@ public abstract interface class io/sentry/IConnectionStatusProvider$IConnectionS
767767

768768
public abstract interface class io/sentry/IContinuousProfiler {
769769
public abstract fun close (Z)V
770+
public abstract fun getChunkId ()Lio/sentry/protocol/SentryId;
770771
public abstract fun getProfilerId ()Lio/sentry/protocol/SentryId;
771772
public abstract fun isRunning ()Z
772773
public abstract fun reevaluateSampling ()V
@@ -1482,6 +1483,7 @@ public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectio
14821483

14831484
public final class io/sentry/NoOpContinuousProfiler : io/sentry/IContinuousProfiler {
14841485
public fun close (Z)V
1486+
public fun getChunkId ()Lio/sentry/protocol/SentryId;
14851487
public static fun getInstance ()Lio/sentry/NoOpContinuousProfiler;
14861488
public fun getProfilerId ()Lio/sentry/protocol/SentryId;
14871489
public fun isRunning ()Z

‎sentry/src/main/java/io/sentry/IContinuousProfiler.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,7 @@ void startProfiler(
2525

2626
@NotNull
2727
SentryId getProfilerId();
28+
29+
@NotNull
30+
SentryId getChunkId();
2831
}

‎sentry/src/main/java/io/sentry/NoOpContinuousProfiler.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,9 @@ public void reevaluateSampling() {}
3636
public @NotNull SentryId getProfilerId() {
3737
return SentryId.EMPTY_ID;
3838
}
39+
40+
@Override
41+
public @NotNull SentryId getChunkId() {
42+
return SentryId.EMPTY_ID;
43+
}
3944
}

‎sentry/src/test/java/io/sentry/NoOpContinuousProfilerTest.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ class NoOpContinuousProfilerTest {
2525
assertEquals(profiler.profilerId, SentryId.EMPTY_ID)
2626
}
2727

28+
@Test
29+
fun `getChunkId returns Empty SentryId`() {
30+
assertEquals(profiler.chunkId, SentryId.EMPTY_ID)
31+
}
32+
2833
@Test
2934
fun `reevaluateSampling does not throw`() {
3035
profiler.reevaluateSampling()

0 commit comments

Comments
 (0)