Skip to content
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
### Fixes

- Fix the issue with uploading iOS Debug Symbols in EAS Build when using pnpm ([#6076](https://github.com/getsentry/sentry-react-native/issues/6076))
- Improve frame delay collection performance by using sentry-java `getFramesDelay` API ([#6074](https://github.com/getsentry/sentry-react-native/pull/6074))

### Dependencies

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import io.sentry.android.core.InternalSentrySdk;
import io.sentry.android.core.SentryAndroidDateProvider;
import io.sentry.android.core.SentryAndroidOptions;
import io.sentry.android.core.SentryFramesDelayResult;
import io.sentry.android.core.SentryShakeDetector;
import io.sentry.android.core.ViewHierarchyEventProcessor;
import io.sentry.android.core.internal.debugmeta.AssetsDebugMetaLoader;
Expand Down Expand Up @@ -98,7 +99,8 @@
private final ReactApplicationContext reactApplicationContext;
private final PackageInfo packageInfo;
private FrameMetricsAggregator frameMetricsAggregator = null;
private final RNSentryFrameDelayCollector frameDelayCollector = new RNSentryFrameDelayCollector();
@VisibleForTesting @Nullable SentryFrameMetricsCollector frameMetricsCollector = null;
private @Nullable String frameMetricsListenerId = null;
private boolean androidXAvailable;

@VisibleForTesting static long lastStartTimestampMs = -1;
Expand Down Expand Up @@ -413,9 +415,14 @@
long startNanos = nowNanos - (long) (startOffsetSeconds * 1e9);
long endNanos = nowNanos - (long) (endOffsetSeconds * 1e9);

double delaySeconds = frameDelayCollector.getFramesDelay(startNanos, endNanos);
if (delaySeconds >= 0) {
promise.resolve(delaySeconds);
if (frameMetricsCollector == null) {
promise.resolve(null);
return;
}

SentryFramesDelayResult result = frameMetricsCollector.getFramesDelay(startNanos, endNanos);
if (result != null && result.getDelaySeconds() >= 0) {
promise.resolve(result.getDelaySeconds());
Comment thread
antonis marked this conversation as resolved.
} else {
promise.resolve(null);
}
Expand Down Expand Up @@ -747,12 +754,27 @@
if (options instanceof SentryAndroidOptions) {
final SentryFrameMetricsCollector collector =
((SentryAndroidOptions) options).getFrameMetricsCollector();
if (frameDelayCollector.start(collector)) {
logger.log(SentryLevel.INFO, "RNSentryFrameDelayCollector installed.");
if (collector != null) {
// Register a no-op listener to ensure frame metrics collection is active.
// This is needed so that getFramesDelay() has data to query.
stopFrameMetricsCollection();
frameMetricsCollector = collector;
frameMetricsListenerId =
collector.startCollection(
(startNanos,
endNanos,
Comment thread
antonis marked this conversation as resolved.
durationNanos,
delayNanos,
isSlow,
isFrozen,
refreshRate) -> {});
if (frameMetricsListenerId != null) {
logger.log(SentryLevel.INFO, "SentryFrameMetricsCollector listener installed.");
}
}

Check warning on line 774 in packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

View check run for this annotation

@sentry/warden / warden: find-bugs

frameMetricsCollector kept non-null when startCollection returns null, causing getFramesDelay to be called without active collection

In enableNativeFramesTracking, frameMetricsCollector is assigned before startCollection() is invoked. If startCollection returns null (e.g., the listener could not be registered), frameMetricsCollector remains non-null while frameMetricsListenerId stays null. fetchNativeFramesDelay only guards on frameMetricsCollector == null (line 418), so it will still call collector.getFramesDelay() even though no listener is actively collecting frame metrics, returning stale or zero data instead of resolving null as intended.
Comment thread
antonis marked this conversation as resolved.
Outdated
}
} catch (Throwable ignored) { // NOPMD - We don't want to crash in any case
logger.log(SentryLevel.WARNING, "Error starting RNSentryFrameDelayCollector.");
logger.log(SentryLevel.WARNING, "Error starting frame metrics collection.");
}
}

Expand All @@ -761,7 +783,15 @@
frameMetricsAggregator.stop();
frameMetricsAggregator = null;
}
frameDelayCollector.stop();
stopFrameMetricsCollection();
}

private void stopFrameMetricsCollection() {
if (frameMetricsCollector != null && frameMetricsListenerId != null) {
frameMetricsCollector.stopCollection(frameMetricsListenerId);
}
frameMetricsCollector = null;
frameMetricsListenerId = null;
}

public void getNewScreenTimeToDisplay(Promise promise) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package io.sentry.react;

import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
import com.facebook.react.bridge.Promise;
import com.facebook.react.bridge.ReactApplicationContext;
import io.sentry.android.core.SentryFramesDelayResult;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import org.junit.Before;
import org.junit.Test;

public class RNSentryFramesDelayTest {

private RNSentryModuleImpl module;
private Promise promise;

@Before
public void setUp() throws Exception {
ReactApplicationContext reactContext = mock(ReactApplicationContext.class);
PackageManager packageManager = mock(PackageManager.class);
when(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(new PackageInfo());
when(reactContext.getPackageManager()).thenReturn(packageManager);
when(reactContext.getPackageName()).thenReturn("com.test.app");
module = new RNSentryModuleImpl(reactContext);
promise = mock(Promise.class);
}

@Test
public void resolvesNullWhenCollectorIsNull() {
module.frameMetricsCollector = null;
double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now - 1.0, now, promise);
verify(promise).resolve(isNull());
}

@Test
public void resolvesDelayFromCollector() {
SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class);
when(collector.getFramesDelay(anyLong(), anyLong()))
.thenReturn(new SentryFramesDelayResult(0.123, 2));
module.frameMetricsCollector = collector;

double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now - 1.0, now, promise);
verify(promise).resolve(eq(0.123));
}

@Test
public void resolvesNullWhenDelayIsNegative() {
SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class);
when(collector.getFramesDelay(anyLong(), anyLong()))
.thenReturn(new SentryFramesDelayResult(-1, 0));
module.frameMetricsCollector = collector;

double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now - 1.0, now, promise);
verify(promise).resolve(isNull());
}

@Test
public void resolvesNullWhenResultIsNull() {
SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class);
when(collector.getFramesDelay(anyLong(), anyLong())).thenReturn(null);
module.frameMetricsCollector = collector;

double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now - 1.0, now, promise);
verify(promise).resolve(isNull());
}

@Test
public void resolvesNullWhenStartIsInFuture() {
SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class);
module.frameMetricsCollector = collector;

double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now + 100.0, now + 200.0, promise);
verify(promise).resolve(isNull());
}

@Test
public void resolvesZeroDelayWhenNoSlowFrames() {
SentryFrameMetricsCollector collector = mock(SentryFrameMetricsCollector.class);
when(collector.getFramesDelay(anyLong(), anyLong()))
.thenReturn(new SentryFramesDelayResult(0.0, 0));
module.frameMetricsCollector = collector;

double now = System.currentTimeMillis() / 1e3;
module.fetchNativeFramesDelay(now - 1.0, now, promise);
verify(promise).resolve(eq(0.0));
}
}
Loading