diff --git a/CHANGELOG.md b/CHANGELOG.md index f7d968b1ab..7f9895421d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Attach the on-error Session Replay buffer to feedback submitted from the feedback form ([#6870](https://github.com/getsentry/sentry-react-native/pull/6870)) - Do not mark `sendDefaultPii` as deprecated in the React Native options ([#6864](https://github.com/getsentry/sentry-react-native/pull/6864)) +- Include CPU and memory measurements in Android profiles ([#6891](https://github.com/getsentry/sentry-react-native/pull/6891)) ### Dependencies diff --git a/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryProfilingPerformanceCollectionTest.kt b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryProfilingPerformanceCollectionTest.kt new file mode 100644 index 0000000000..9a4501a38f --- /dev/null +++ b/packages/core/RNSentryAndroidTester/app/src/test/java/io/sentry/react/RNSentryProfilingPerformanceCollectionTest.kt @@ -0,0 +1,174 @@ +package io.sentry.react + +import io.sentry.CompositePerformanceCollector +import io.sentry.IScopes +import io.sentry.PerformanceCollectionData +import io.sentry.Sentry +import io.sentry.android.core.AndroidProfiler +import io.sentry.android.core.SentryAndroidOptions +import org.junit.After +import org.junit.Assert.assertNull +import org.junit.Assert.assertSame +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.robolectric.RobolectricTestRunner +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference + +/** + * Verifies the CPU/memory performance collection paired with the Android profiler, which feeds + * the profile's cpu_usage and memory_footprint measurements. Collections started by id never time + * out on their own, so every exit path must stop them. + * + * startProfiling()/stopProfiling() allocate WritableNativeMap and calls HermesSamplingProfiler, both JNI-backed + * and not loadable or mockable in a JVM test, so it is covered on-device instead. + */ +@RunWith(RobolectricTestRunner::class) +class RNSentryProfilingPerformanceCollectionTest { + private lateinit var collector: CompositePerformanceCollector + private lateinit var module: RNSentryModuleImpl + private lateinit var sentryMock: MockedStatic + + @Before + fun setUp() { + collector = mock() + val options = SentryAndroidOptions().apply { setCompositePerformanceCollector(collector) } + val scopes = mock() + whenever(scopes.options).thenReturn(options) + + sentryMock = mockStatic(Sentry::class.java) + sentryMock.`when` { Sentry.getCurrentScopes() }.thenReturn(scopes) + + module = Utils.createRNSentryModuleWithMockedContext() + } + + @After + fun tearDown() { + sentryMock.close() + } + + @Test + fun `stop returns the data collected under the started id`() { + val data = listOf(PerformanceCollectionData(1L)) + whenever(collector.stop(any())).thenReturn(data) + + module.startPerformanceCollection() + val result = module.stopPerformanceCollection() + + val startId = argumentCaptor() + verify(collector).start(startId.capture()) + verify(collector).stop(eq(startId.firstValue)) + assertSame(data, result) + } + + @Test + fun `stop without a running collection returns null`() { + assertNull(module.stopPerformanceCollection()) + verify(collector, never()).stop(any()) + } + + @Test + fun `stop is idempotent`() { + module.startPerformanceCollection() + module.stopPerformanceCollection() + assertNull(module.stopPerformanceCollection()) + + verify(collector, times(1)).stop(any()) + } + + @Test + fun `starting again stops the previous collection`() { + module.startPerformanceCollection() + module.startPerformanceCollection() + + val startIds = argumentCaptor() + verify(collector, times(2)).start(startIds.capture()) + verify(collector).stop(eq(startIds.firstValue)) + verify(collector, never()).stop(eq(startIds.secondValue)) + } + + @Test + fun `a failing collector start is swallowed and leaves nothing to stop`() { + doThrow(RuntimeException("boom")).whenever(collector).start(any()) + + module.startPerformanceCollection() + + assertNull(module.stopPerformanceCollection()) + verify(collector, never()).stop(any()) + } + + @Test + fun `a failing collector stop is swallowed`() { + whenever(collector.stop(any())).thenThrow(RuntimeException("boom")) + + module.startPerformanceCollection() + + assertNull(module.stopPerformanceCollection()) + } + + @Test + fun `invalidate stops the performance collection`() { + val profiler = mock() + seedRunningProfiler(profiler) + module.startPerformanceCollection() + + // HermesSamplingProfiler.disable() fails in the JVM; invalidate() catches it. + module.invalidate() + + val startId = argumentCaptor() + verify(collector).start(startId.capture()) + verify(collector).stop(eq(startId.firstValue)) + verify(profiler).endAndCollect(eq(false), anyOrNull()) + } + + @Test + fun `profile collection keeps running while profiling is active`() { + seedRunningProfiler(mock()) + + module.startProfilePerformanceCollection() + + verify(collector).start(any()) + verify(collector, never()).stop(any()) + } + + @Test + fun `profile collection is stopped when invalidate races its start`() { + seedRunningProfiler(mock()) + // invalidate() runs after the collector started but before the id is published, so it + // cannot see the collection; the re-check after publishing must stop it. + doAnswer { module.invalidate() }.whenever(collector).start(any()) + + module.startProfilePerformanceCollection() + + val startId = argumentCaptor() + verify(collector).start(startId.capture()) + verify(collector).stop(eq(startId.firstValue)) + assertNull(module.stopPerformanceCollection()) + } + + private fun seedRunningProfiler(profiler: AndroidProfiler) { + @Suppress("UNCHECKED_CAST") + (field("androidProfiler") as AtomicReference).set(profiler) + (field("isProfiling") as AtomicBoolean).set(true) + } + + private fun field(name: String): Any = + RNSentryModuleImpl::class.java + .getDeclaredField(name) + .apply { isAccessible = true } + .get(module)!! +} diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 7d5c3c2731..d100f388f1 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -33,6 +33,7 @@ import io.sentry.IScope; import io.sentry.ISentryExecutorService; import io.sentry.ISerializer; +import io.sentry.PerformanceCollectionData; import io.sentry.PropagationContext; import io.sentry.ScopesAdapter; import io.sentry.Sentry; @@ -82,6 +83,7 @@ import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.UUID; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; @@ -133,6 +135,10 @@ public class RNSentryModuleImpl { // observe a stale false and skip disabling a running Hermes sampler. private final AtomicBoolean isProfiling = new AtomicBoolean(false); + // Id of the composite performance collection (CPU/memory) running alongside the Android + // profiler. Collections started by id never time out, so every exit path must stop it. + private final AtomicReference performanceCollectionId = new AtomicReference<>(); + private boolean isProguardDebugMetaLoaded = false; private @Nullable String proguardUuid = null; private String cacheDirPath = null; @@ -1008,10 +1014,14 @@ public WritableMap startProfiling(boolean platformProfilers) { final AndroidProfiler profiler = androidProfiler.get(); if (profiler != null) { profiler.start(); + startProfilePerformanceCollection(); } result.putBoolean("started", true); } catch (Throwable e) { // NOPMD - We don't want to crash in any case + // Unconditional: a concurrent invalidate() may already have claimed isProfiling, and + // stopping is idempotent. + stopPerformanceCollection(); // Unwind Hermes if we enabled it but failed before returning. getAndSet(false) makes // this idempotent with a concurrent invalidate()/stopProfiling(). if (isProfiling.getAndSet(false)) { @@ -1038,11 +1048,13 @@ public WritableMap stopProfiling() { // Take sole ownership of the AndroidProfiler reference so a concurrent invalidate() // cannot also call endAndCollect() on the same instance. final AndroidProfiler profiler = androidProfiler.getAndSet(null); + final @Nullable List performanceCollectionData = + stopPerformanceCollection(); File output = null; try { AndroidProfiler.ProfileEndData end = null; if (profiler != null) { - end = profiler.endAndCollect(false, null); + end = profiler.endAndCollect(false, performanceCollectionData); } HermesSamplingProfiler.disable(); @@ -1139,6 +1151,10 @@ public void invalidate() { logger.log(SentryLevel.WARNING, "Failed to stop Hermes sampling profiler on teardown: " + e); } + // The profile is discarded, but the collection must still be stopped or its timer keeps + // sampling forever. stopPerformanceCollection() never throws. + stopPerformanceCollection(); + // Android platform profiler is independent of Hermes; a failure here must not leak // back into the Hermes shutdown above. The profile is being discarded; delete the // trace file to avoid leaking it in cacheDir. @@ -1157,6 +1173,53 @@ public void invalidate() { } } + /** + * Starts the performance collection for the profile being started. invalidate() can claim cleanup + * after isProfiling is set but before the collection id is published, and so miss the collection. + * Re-checking isProfiling after publishing guarantees that either invalidate() or this method + * stops it. + */ + @VisibleForTesting + void startProfilePerformanceCollection() { + startPerformanceCollection(); + if (!isProfiling.get()) { + stopPerformanceCollection(); + } + } + + /** + * Starts sampling CPU and memory via the SDK's composite performance collector so the Android + * profile carries {@code cpu_usage} and {@code memory_footprint} measurements. The collector + * samples on its own timer thread. Best-effort: the profile still records if this fails. + */ + @VisibleForTesting + void startPerformanceCollection() { + // Stop a leftover collection (start without a matching stop) so its timer doesn't leak. + stopPerformanceCollection(); + try { + final String id = UUID.randomUUID().toString(); + ScopesAdapter.getInstance().getOptions().getCompositePerformanceCollector().start(id); + performanceCollectionId.set(id); + } catch (Throwable e) { // NOPMD - Measurements are best-effort; the profile still records. + logger.log(SentryLevel.WARNING, "Failed to start profile performance collection: " + e); + } + } + + @VisibleForTesting + @Nullable + List stopPerformanceCollection() { + final String id = performanceCollectionId.getAndSet(null); + if (id == null) { + return null; // NOPMD - null means no collection, as endAndCollect expects. + } + try { + return ScopesAdapter.getInstance().getOptions().getCompositePerformanceCollector().stop(id); + } catch (Throwable e) { // NOPMD - Measurements are best-effort; the profile still records. + logger.log(SentryLevel.WARNING, "Failed to stop profile performance collection: " + e); + return null; // NOPMD - null means no collection, as endAndCollect expects. + } + } + private @Nullable String getProguardUuid() { if (isProguardDebugMetaLoaded) { return proguardUuid;