Merge a99c3c1 into 7259f12

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- Enhanced accuracy of time-to-display spans. ([#4042](https://github.com/getsentry/sentry-react-native/pull/4042))
 - Handles error with string cause ([#4163](https://github.com/getsentry/sentry-react-native/pull/4163))
 - Use `appLaunchedInForeground` to determine invalid app start data on Android ([#4146](https://github.com/getsentry/sentry-react-native/pull/4146))
 - Upload source maps for all release variants on Android (not only the last found) ([#4125](https://github.com/getsentry/sentry-react-native/pull/4125))

android/src/main/java/io/sentry/react/RNSentryModuleImpl.java

@@ -41,15 +41,11 @@
 import java.io.InputStream;
 import java.nio.charset.Charset;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
-import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 
-import io.sentry.Breadcrumb;
-import io.sentry.DateUtils;
 import io.sentry.HubAdapter;
 import io.sentry.ILogger;
 import io.sentry.ISentryExecutorService;
@@ -135,10 +131,13 @@ public class RNSentryModuleImpl {
     /** Max trace file size in bytes. */
     private long maxTraceFileSize = 5 * 1024 * 1024;
 
+    private final @NotNull SentryDateProvider dateProvider;
+
     public RNSentryModuleImpl(ReactApplicationContext reactApplicationContext) {
       packageInfo = getPackageInfo(reactApplicationContext);
       this.reactApplicationContext = reactApplicationContext;
       this.emitNewFrameEvent = createEmitNewFrameEvent();
+      this.dateProvider = new SentryAndroidDateProvider();
     }
 
     private ReactApplicationContext getReactApplicationContext() {
@@ -150,8 +149,6 @@ private ReactApplicationContext getReactApplicationContext() {
     }
 
     private @NotNull Runnable createEmitNewFrameEvent() {
-        final @NotNull SentryDateProvider dateProvider = new SentryAndroidDateProvider();
-
         return () -> {
           final SentryDate endDate = dateProvider.now();
           WritableMap event = Arguments.createMap();
@@ -701,6 +698,10 @@ public void disableNativeFramesTracking() {
         }
     }
 
+    public void getNewScreenTimeToDisplay(Promise promise) {
+        RNSentryTimeToDisplay.GetTimeToDisplay(promise, dateProvider);
+    }
+
     private String getProfilingTracesDirPath() {
         if (cacheDirPath == null) {
             cacheDirPath = new File(getReactApplicationContext().getCacheDir(), "sentry/react").getAbsolutePath();

android/src/main/java/io/sentry/react/RNSentryPackage.java

@@ -55,5 +55,4 @@ public List<ViewManager> createViewManagers(
           new RNSentryOnDrawReporterManager(reactContext)
     );
   }
-
 }

android/src/main/java/io/sentry/react/RNSentryTimeToDisplay.java

@@ -0,0 +1,23 @@
+package io.sentry.react;
+
+import com.facebook.react.bridge.Promise;
+
+import android.view.Choreographer;
+
+import org.jetbrains.annotations.NotNull;
+import io.sentry.SentryDate;
+import io.sentry.SentryDateProvider;
+import io.sentry.android.core.SentryAndroidDateProvider;
+
+public class RNSentryTimeToDisplay {
+    public static void GetTimeToDisplay(Promise promise, SentryDateProvider dateProvider) {
+        Choreographer choreographer = Choreographer.getInstance();
+
+        // Invoke the callback after the frame is rendered
+        choreographer.postFrameCallback(frameTimeNanos -> {
+                final SentryDate endDate = dateProvider.now();
+
+                promise.resolve(endDate.nanoTimestamp() / 1e9);
+        });
+    }
+}

android/src/newarch/java/io/sentry/react/RNSentryModule.java

@@ -173,4 +173,9 @@ public String getCurrentReplayId() {
     public void crashedLastRun(Promise promise) {
         this.impl.crashedLastRun(promise);
     }
+
+    @Override
+    public void getNewScreenTimeToDisplay(Promise promise) {
+        this.impl.getNewScreenTimeToDisplay(promise);
+    }
 }

android/src/oldarch/java/io/sentry/react/RNSentryModule.java

@@ -173,4 +173,9 @@ public String getCurrentReplayId() {
     public void crashedLastRun(Promise promise) {
         this.impl.crashedLastRun(promise);
     }
+
+    @ReactMethod()
+    public void getNewScreenTimeToDisplay(Promise promise) {
+        this.impl.getNewScreenTimeToDisplay(promise);
+    }
 }

ios/RNSentry.mm

@@ -1,5 +1,6 @@
 #import <dlfcn.h>
 #import "RNSentry.h"
+#import "RNSentryTimeToDisplay.h"
 
 #if __has_include(<React/RCTConvert.h>)
 #import <React/RCTConvert.h>
@@ -62,6 +63,7 @@ + (void)storeEnvelope:(SentryEnvelope *)envelope;
 @implementation RNSentry {
     bool sentHybridSdkDidBecomeActive;
     bool hasListeners;
+    RNSentryTimeToDisplay *_timeToDisplay;
 }
 
 - (dispatch_queue_t)methodQueue
@@ -106,6 +108,8 @@ + (BOOL)requiresMainQueueSetup {
         sentHybridSdkDidBecomeActive = true;
     }
 
+    _timeToDisplay = [[RNSentryTimeToDisplay alloc] init];
+
 #if SENTRY_TARGET_REPLAY_SUPPORTED
     [RNSentryReplay postInit];
 #endif
@@ -778,4 +782,9 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
 }
 #endif
 
+RCT_EXPORT_METHOD(getNewScreenTimeToDisplay:(RCTPromiseResolveBlock)resolve
+                  rejecter:(RCTPromiseRejectBlock)reject) {
+    [_timeToDisplay getTimeToDisplay:resolve];
+}
+
 @end

ios/RNSentryTimeToDisplay.h

@@ -0,0 +1,7 @@
+#import <React/RCTBridgeModule.h>
+
+@interface RNSentryTimeToDisplay : NSObject
+
+- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback;
+
+@end

ios/RNSentryTimeToDisplay.m

@@ -0,0 +1,43 @@
+#import "RNSentryTimeToDisplay.h"
+#import <QuartzCore/QuartzCore.h>
+#import <React/RCTLog.h>
+
+@implementation RNSentryTimeToDisplay
+{
+  CADisplayLink *displayLink;
+  RCTResponseSenderBlock resolveBlock;
+}
+
+// Rename requestAnimationFrame to getTimeToDisplay
+- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback
+{
+  // Store the resolve block to use in the callback.
+  resolveBlock = callback;
+
+#if TARGET_OS_IOS
+  // Create and add a display link to get the callback after the screen is rendered.
+  displayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(handleDisplayLink:)];
+  [displayLink addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
+#else
+  resolveBlock(@[]); // Return nothing if not iOS.
+#endif
+}
+
+#if TARGET_OS_IOS
+- (void)handleDisplayLink:(CADisplayLink *)link {
+  // Get the current time
+  NSTimeInterval currentTime = [[NSDate date] timeIntervalSince1970] * 1000.0; // Convert to milliseconds
+
+  // Ensure the callback is valid and pass the current time back
+  if (resolveBlock) {
+    resolveBlock(@[@(currentTime)]); // Call the callback with the current time
+    resolveBlock = nil; // Clear the block after it's called
+  }
+
+  // Invalidate the display link to stop future callbacks
+  [displayLink invalidate];
+  displayLink = nil;
+}
+#endif
+
+@end

src/js/NativeRNSentry.ts

@@ -9,13 +9,15 @@ import type { UnsafeObject } from './utils/rnlibrariesinterface';
 export interface Spec extends TurboModule {
   addListener: (eventType: string) => void;
   removeListeners: (id: number) => void;
+  getNewScreenTimeToDisplay(): Promise<number | undefined | null>;
   addBreadcrumb(breadcrumb: UnsafeObject): void;
   captureEnvelope(
     bytes: string,
     options: {
       hardCrashed: boolean;
     },
   ): Promise<boolean>;
+
   captureScreenshot(): Promise<NativeScreenshot[] | undefined | null>;
   clearBreadcrumbs(): void;
   crash(): void;

src/js/tracing/reactnavigation.ts

@@ -78,7 +78,6 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
 
   private _navigationContainer: NavigationContainer | null = null;
   private _newScreenFrameEventEmitter: SentryEventEmitter | null = null;
-
   private readonly _maxRecentRouteLen: number = 200;
 
   private _latestRoute?: NavigationRoute;
@@ -247,8 +246,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
               isAutoInstrumented: true,
             });
 
-          !routeHasBeenSeen &&
-            latestTtidSpan &&
+          if (!routeHasBeenSeen && latestTtidSpan) {
             this._newScreenFrameEventEmitter?.once(
               NewFrameEventName,
               ({ newFrameTimestampInSeconds }: NewFrameEvent) => {
@@ -265,6 +263,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
                 setSpanDurationAsMeasurementOnTransaction(latestTransaction, 'time_to_initial_display', latestTtidSpan);
               },
             );
+          }
 
           this._navigationProcessingSpan?.updateName(`Processing navigation to ${route.name}`);
           this._navigationProcessingSpan?.setStatus('ok');

src/js/utils/sentryeventemitter.ts

@@ -3,6 +3,7 @@ import type { EmitterSubscription, NativeModule } from 'react-native';
 import { NativeEventEmitter } from 'react-native';
 
 import { getRNSentryModule } from '../wrapper';
+import { createSentryFallbackEventEmitter } from './sentryeventemitterfallback';
 
 export const NewFrameEventName = 'rn_sentry_new_frame';
 export type NewFrameEventName = typeof NewFrameEventName;
@@ -32,6 +33,7 @@ export function createSentryEventEmitter(
   if (!sentryNativeModule) {
     return createNoopSentryEventEmitter();
   }
+  const fallbackEventEmitter = createSentryFallbackEventEmitter();
 
   const openNativeListeners = new Map<NewFrameEventName, EmitterSubscription>();
   const listenersMap = new Map<NewFrameEventName, Map<(event: NewFrameEvent) => void, true>>();
@@ -70,6 +72,8 @@ export function createSentryEventEmitter(
       openNativeListeners.set(eventType, nativeListener);
 
       listenersMap.set(eventType, new Map());
+
+      fallbackEventEmitter.initAsync();
     },
     closeAllAsync() {
       openNativeListeners.forEach(subscription => {
@@ -81,6 +85,8 @@ export function createSentryEventEmitter(
     addListener,
     removeListener,
     once(eventType: NewFrameEventName, listener: (event: NewFrameEvent) => void) {
+      fallbackEventEmitter?.startListenerAsync();
+
       const tmpListener = (event: NewFrameEvent): void => {
         listener(event);
         removeListener(eventType, tmpListener);

src/js/utils/sentryeventemitterfallback.ts

@@ -0,0 +1,93 @@
+import { logger, timestampInSeconds } from '@sentry/utils';
+import { DeviceEventEmitter } from 'react-native';
+
+import { NATIVE } from '../wrapper';
+import { NewFrameEventName } from './sentryeventemitter';
+
+export type FallBackNewFrameEvent = { newFrameTimestampInSeconds: number; isFallback?: boolean };
+export interface SentryEventEmitterFallback {
+  /**
+   * Initializes the fallback event emitter
+   * This method is synchronous in JS but the event emitter starts asynchronously.
+   */
+  initAsync: () => void;
+  startListenerAsync: () => void;
+}
+
+/**
+ * Creates emitter that allows to listen to UI Frame events when ready.
+ */
+export function createSentryFallbackEventEmitter(): SentryEventEmitterFallback {
+  let NativeEmitterCalled: boolean = false;
+  let isListening = false;
+
+  function defaultFallbackEventEmitter(): void {
+    // https://reactnative.dev/docs/timers#timers
+    // NOTE: The current implementation of requestAnimationFrame is the same
+    // as setTimeout(0). This isn't exactly how requestAnimationFrame
+    // is supposed to work on web, so it doesn't get called when UI Frames are rendered.: https://github.com/facebook/react-native/blob/5106933c750fee2ce49fe1945c3e3763eebc92bc/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp#L442-L443
+    requestAnimationFrame(() => {
+      if (NativeEmitterCalled) {
+        NativeEmitterCalled = false;
+        isListening = false;
+        return;
+      }
+      const seconds = timestampInSeconds();
+      waitForNativeResponseOrFallback(seconds, 'JavaScript');
+    });
+  }
+
+  function waitForNativeResponseOrFallback(fallbackSeconds: number, origin: string): void {
+    let firstAttemptCompleted = false;
+
+    const checkNativeResponse = (): void => {
+      if (NativeEmitterCalled) {
+        NativeEmitterCalled = false;
+        isListening = false;
+        return; // Native Replied the bridge with a timestamp.
+      }
+      if (!firstAttemptCompleted) {
+        firstAttemptCompleted = true;
+        setTimeout(checkNativeResponse, 3_000);
+      } else {
+        logger.log(`[Sentry] Native event emitter did not reply in time. Using ${origin} fallback emitter.`);
+        isListening = false;
+        DeviceEventEmitter.emit(NewFrameEventName, {
+          newFrameTimestampInSeconds: fallbackSeconds,
+          isFallback: true,
+        });
+      }
+    };
+
+    // Start the retry process
+    checkNativeResponse();
+  }
+
+  return {
+    initAsync() {
+      DeviceEventEmitter.addListener(NewFrameEventName, () => {
+        // Avoid noise from pages that we do not want to track.
+        if (isListening) {
+          NativeEmitterCalled = true;
+        }
+      });
+    },
+
+    startListenerAsync() {
+      isListening = true;
+
+      NATIVE.getNewScreenTimeToDisplay()
+        .then(resolve => {
+          if (resolve) {
+            waitForNativeResponseOrFallback(resolve, 'Native');
+          } else {
+            defaultFallbackEventEmitter();
+          }
+        })
+        .catch((reason: Error) => {
+          logger.error('Failed to recceive Native fallback timestamp.', reason);
+          defaultFallbackEventEmitter();
+        });
+    },
+  };
+}

src/js/wrapper.ts

@@ -114,6 +114,7 @@ interface SentryNativeWrapper {
   getCurrentReplayId(): string | null;
 
   crashedLastRun(): Promise<boolean | null>;
+  getNewScreenTimeToDisplay(): Promise<number | null | undefined>;
 }
 
 const EOL = utf8ToBytes('\n');
@@ -656,6 +657,14 @@ export const NATIVE: SentryNativeWrapper = {
     return typeof result === 'boolean' ? result : null;
   },
 
+  getNewScreenTimeToDisplay(): Promise<number | null | undefined> {
+    if (!this.enableNative || !this._isModuleLoaded(RNSentry)) {
+      return Promise.resolve(null);
+    }
+
+    return RNSentry.getNewScreenTimeToDisplay();
+  },
+
   /**
    * Gets the event from envelopeItem and applies the level filter to the selected event.
    * @param data An envelope item containing the event.

test/mockWrapper.ts

@@ -55,6 +55,7 @@ const NATIVE: MockInterface<NativeType> = {
   initNativeReactNavigationNewFrameTracking: jest.fn(),
 
   crashedLastRun: jest.fn(),
+  getNewScreenTimeToDisplay: jest.fn().mockResolvedValue(42),
 };
 
 NATIVE.isNativeAvailable.mockReturnValue(true);