Merge dc9ca95 into fd21b24

lucas-zimerman · web-flow · commit 5ba6409b29ca · 2024-09-24T15:43:09.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Enhanced accuracy of time-to-display spans. ([#4042](https://github.com/getsentry/sentry-react-native/pull/4042))
+
 ## 5.33.1
 
 ### Internal
diff --git a/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
@@ -135,10 +135,13 @@ public class RNSentryModuleImpl {
     /** Max trace file size in bytes. */
     private long maxTraceFileSize = 5 * 1024 * 1024;
 
+     private final RNSentryTimeToDisplay timeToDisplay;
+     
     public RNSentryModuleImpl(ReactApplicationContext reactApplicationContext) {
       packageInfo = getPackageInfo(reactApplicationContext);
       this.reactApplicationContext = reactApplicationContext;
       this.emitNewFrameEvent = createEmitNewFrameEvent();
+      this.timeToDisplay = new RNSentryTimeToDisplay();
     }
 
     private ReactApplicationContext getReactApplicationContext() {
@@ -693,6 +696,10 @@ public void disableNativeFramesTracking() {
         }
     }
 
+    public void getNewScreenTimeToDisplay(Promise promise) {
+        timeToDisplay.GetTimeToDisplay(promise);
+    }
+
     private String getProfilingTracesDirPath() {
         if (cacheDirPath == null) {
             cacheDirPath = new File(getReactApplicationContext().getCacheDir(), "sentry/react").getAbsolutePath();
diff --git a/android/src/main/java/io/sentry/react/RNSentryPackage.java b/android/src/main/java/io/sentry/react/RNSentryPackage.java
@@ -55,5 +55,4 @@ public List<ViewManager> createViewManagers(
           new RNSentryOnDrawReporterManager(reactContext)
     );
   }
-
 }
diff --git a/android/src/main/java/io/sentry/react/RNSentryTimeToDisplay.java b/android/src/main/java/io/sentry/react/RNSentryTimeToDisplay.java
@@ -0,0 +1,28 @@
+package io.sentry.react;
+
+import com.facebook.react.bridge.Promise;
+
+import android.view.Choreographer;
+
+import androidx.annotation.NonNull;
+
+import org.jetbrains.annotations.NotNull;
+import io.sentry.SentryDate;
+import io.sentry.SentryDateProvider;
+import io.sentry.android.core.SentryAndroidDateProvider;
+
+public class RNSentryTimeToDisplay {
+
+    public void GetTimeToDisplay(Promise promise) {
+        Choreographer choreographer = Choreographer.getInstance();
+
+        // Invoke the callback after the frame is rendered
+        choreographer.postFrameCallback(frameTimeNanos -> {
+                final @NotNull SentryDateProvider dateProvider = new SentryAndroidDateProvider();
+
+                final SentryDate endDate = dateProvider.now();
+
+                promise.resolve(endDate.nanoTimestamp() / 1e9);
+        });
+    }
+}
diff --git a/android/src/newarch/java/io/sentry/react/RNSentryModule.java b/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);
+    }
 }
diff --git a/android/src/oldarch/java/io/sentry/react/RNSentryModule.java b/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);
+    }
 }
diff --git a/ios/RNSentry.mm b/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
@@ -776,4 +780,9 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
 }
 #endif
 
+RCT_EXPORT_METHOD(getNewScreenTimeToDisplay:(RCTPromiseResolveBlock)resolve
+                  rejecter:(RCTPromiseRejectBlock)reject) {
+    [_timeToDisplay getTimeToDisplay:resolve];
+}
+
 @end
diff --git a/ios/RNSentryTimeToDisplay.h b/ios/RNSentryTimeToDisplay.h
@@ -0,0 +1,7 @@
+#import <React/RCTBridgeModule.h>
+
+@interface RNSentryTimeToDisplay : NSObject
+
+- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback;
+
+@end
diff --git a/ios/RNSentryTimeToDisplay.m b/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
diff --git a/src/js/NativeRNSentry.ts b/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;
diff --git a/src/js/tracing/reactnavigation.ts b/src/js/tracing/reactnavigation.ts
@@ -5,6 +5,7 @@ import { logger, timestampInSeconds } from '@sentry/utils';
 
 import type { NewFrameEvent } from '../utils/sentryeventemitter';
 import { type SentryEventEmitter, createSentryEventEmitter, NewFrameEventName } from '../utils/sentryeventemitter';
+import { type SentryEventEmitterFallback, createSentryFallbackEventEmitter } from '../utils/sentryeventemitterfallback';
 import { RN_GLOBAL_OBJ } from '../utils/worldwide';
 import { NATIVE } from '../wrapper';
 import type { OnConfirmRoute, TransactionCreator } from './routingInstrumentation';
@@ -78,7 +79,7 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
 
   private _navigationContainer: NavigationContainer | null = null;
   private _newScreenFrameEventEmitter: SentryEventEmitter | null = null;
-
+  private _newFallbackEventEmitter: SentryEventEmitterFallback | null = null;
   private readonly _maxRecentRouteLen: number = 200;
 
   private _latestRoute?: NavigationRoute;
@@ -101,7 +102,9 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
 
     if (this._options.enableTimeToInitialDisplay) {
       this._newScreenFrameEventEmitter = createSentryEventEmitter();
+      this._newFallbackEventEmitter = createSentryFallbackEventEmitter();
       this._newScreenFrameEventEmitter.initAsync(NewFrameEventName);
+      this._newFallbackEventEmitter.initAsync();
       NATIVE.initNativeReactNavigationNewFrameTracking().catch((reason: unknown) => {
         logger.error(`[ReactNavigationInstrumentation] Failed to initialize native new frame tracking: ${reason}`);
       });
@@ -247,8 +250,8 @@ export class ReactNavigationInstrumentation extends InternalRoutingInstrumentati
               isAutoInstrumented: true,
             });
 
-          !routeHasBeenSeen &&
-            latestTtidSpan &&
+          if (!routeHasBeenSeen && latestTtidSpan) {
+            this._newFallbackEventEmitter?.startListenerAsync();
             this._newScreenFrameEventEmitter?.once(
               NewFrameEventName,
               ({ newFrameTimestampInSeconds }: NewFrameEvent) => {
@@ -265,6 +268,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');
diff --git a/src/js/utils/sentryeventemitterfallback.ts b/src/js/utils/sentryeventemitterfallback.ts
@@ -0,0 +1,103 @@
+import { logger } from '@sentry/utils';
+import type { EmitterSubscription } from 'react-native';
+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;
+  closeAll: () => void;
+  startListenerAsync: () => void;
+}
+
+function timeNowNanosecond(): number {
+  return Date.now() / 1000; // Convert to nanoseconds
+}
+
+/**
+ * Creates emitter that allows to listen to UI Frame events when ready.
+ */
+export function createSentryFallbackEventEmitter(): SentryEventEmitterFallback {
+  let NativeEmitterCalled: boolean = false;
+  let subscription: EmitterSubscription | undefined = undefined;
+  let isListening = false;
+
+  function defaultFallbackEventEmitter(): void {
+    // Schedule the callback to be executed when all UI Frames have flushed.
+    requestAnimationFrame(() => {
+      if (NativeEmitterCalled) {
+        NativeEmitterCalled = false;
+        isListening = false;
+        return;
+      }
+      const timestampInSeconds = timeNowNanosecond();
+      waitForNativeResponseOrFallback(timestampInSeconds, 'JavaScript');
+    });
+  }
+
+  function waitForNativeResponseOrFallback(fallbackSeconds: number, origin: string): void {
+    const maxRetries = 3;
+    let retries = 0;
+
+    const retryCheck = (): void => {
+      if (NativeEmitterCalled) {
+        NativeEmitterCalled = false;
+        isListening = false;
+        return; // Native Replied the bridge with a timestamp.
+      }
+
+      retries++;
+      if (retries < maxRetries) {
+        setTimeout(retryCheck, 1_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
+    retryCheck();
+  }
+
+  return {
+    initAsync() {
+      subscription = 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();
+        });
+    },
+
+    closeAll() {
+      subscription?.remove();
+    },
+  };
+}
diff --git a/src/js/wrapper.ts b/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.
diff --git a/test/mockWrapper.ts b/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);
diff --git a/test/utils/sentryeventemitterfallback.test.ts b/test/utils/sentryeventemitterfallback.test.ts

Original file line number	Diff line number	Diff line change
`@@ -55,5 +55,4 @@ public List<ViewManager> createViewManagers(`
`55`	`55`	`new RNSentryOnDrawReporterManager(reactContext)`
`56`	`56`	`);`
`57`	`57`	`}`
`58`		`-`
`59`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -173,4 +173,9 @@ public String getCurrentReplayId() {`
`173`	`173`	`public void crashedLastRun(Promise promise) {`
`174`	`174`	`this.impl.crashedLastRun(promise);`
`175`	`175`	`}`
	`176`	`+`
	`177`	`+ @Override`
	`178`	`+ public void getNewScreenTimeToDisplay(Promise promise) {`
	`179`	`+ this.impl.getNewScreenTimeToDisplay(promise);`
	`180`	`+ }`
`176`	`181`	`}`