Skip to content
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
d0c0cf7
implement fallback system
lucas-zimerman Aug 22, 2024
1a52620
clear format
lucas-zimerman Aug 23, 2024
84301f7
backup
lucas-zimerman Aug 30, 2024
3423a61
wip
lucas-zimerman Sep 2, 2024
84c12ff
Merge branch 'main' into test/deadline
lucas-zimerman Sep 3, 2024
1e59d5b
wip iOS
lucas-zimerman Sep 11, 2024
8851215
ioscode
lucas-zimerman Sep 11, 2024
feb08cd
Merge branch 'main' into test/deadline
lucas-zimerman Sep 11, 2024
299fb34
fix typo
lucas-zimerman Sep 11, 2024
734354d
fix-time
lucas-zimerman Sep 11, 2024
dabf98b
removed wrapper placeholder, moved java implementation to separated f…
lucas-zimerman Sep 11, 2024
41879c1
refactored android to use turbo module, refactored fallback emitter
lucas-zimerman Sep 12, 2024
a181bcc
add tests and minor fixes
lucas-zimerman Sep 12, 2024
298a007
missing test file
lucas-zimerman Sep 12, 2024
a93b249
yarn fix
lucas-zimerman Sep 12, 2024
da76e21
fix ios module name
lucas-zimerman Sep 12, 2024
53e67ba
Update samples/react-native/src/Screens/PerformanceScreen.tsx
lucas-zimerman Sep 12, 2024
0344adb
Update samples/react-native/src/Screens/PerformanceScreen.tsx
lucas-zimerman Sep 12, 2024
b115fdd
Update samples/react-native/src/Screens/PerformanceScreen.tsx
lucas-zimerman Sep 12, 2024
99e74cf
name missing
lucas-zimerman Sep 12, 2024
1bb5c4d
Merge branch 'main' into test/deadline
lucas-zimerman Sep 18, 2024
6507648
fix java module name, add isAvailable to ignore macos
lucas-zimerman Sep 19, 2024
2f838b8
yarn fix
lucas-zimerman Sep 19, 2024
a8f5126
use impl method
lucas-zimerman Sep 19, 2024
fa03cc1
nit impl name
lucas-zimerman Sep 19, 2024
b6093d5
refactor how to use time to display fallback
lucas-zimerman Sep 20, 2024
d03004c
yarn fix
lucas-zimerman Sep 20, 2024
7e5f764
fix ios reference
lucas-zimerman Sep 20, 2024
49272a1
Merge branch 'main' into test/deadline
lucas-zimerman Sep 23, 2024
a22824b
changelog
lucas-zimerman Sep 23, 2024
c99200d
Apply suggestions from code review
lucas-zimerman Sep 23, 2024
64fd760
Apply suggestions from code review
lucas-zimerman Sep 24, 2024
dc9ca95
fix build
lucas-zimerman Sep 24, 2024
4710843
Removes unused java imports (#4119)
antonis Sep 25, 2024
1cd3b8e
Merge branch 'main' into test/deadline
lucas-zimerman Oct 2, 2024
54189db
Merge branch 'main' into test/deadline
lucas-zimerman Oct 10, 2024
45fca09
add missing App Metrics import
lucas-zimerman Oct 10, 2024
5c0681d
Merge branch 'main' into test/deadline
lucas-zimerman Oct 14, 2024
966ce1e
refactor tests / remove closeAll
lucas-zimerman Oct 14, 2024
4bb6434
commit java suggestion
lucas-zimerman Oct 14, 2024
add7f95
build fix
lucas-zimerman Oct 14, 2024
55e6589
small refactor and add clearTimeout
lucas-zimerman Oct 14, 2024
9885a15
use sentry timestampInSeconds
lucas-zimerman Oct 14, 2024
d87b40e
removed fallback from navigation, comment on timeout difference
lucas-zimerman Oct 14, 2024
962bb35
requested changes java
lucas-zimerman Oct 14, 2024
4b820fe
yarn fix / remove clearTimeout
lucas-zimerman Oct 14, 2024
a99c3c1
add note and fix time tests
lucas-zimerman Oct 14, 2024
4203401
Kw fallback emitter suggestion (#4171)
lucas-zimerman Oct 14, 2024
3dbcc41
Merge branch 'main' into test/deadline
lucas-zimerman Oct 14, 2024
4557f7a
fix build
lucas-zimerman Oct 14, 2024
9a5ce6d
fix test
lucas-zimerman Oct 14, 2024
342961d
forgot git add for mocked file
lucas-zimerman Oct 14, 2024
427e1c6
add try/catch block on java code
lucas-zimerman Oct 14, 2024
fcb9935
fix lint
lucas-zimerman Oct 14, 2024
2a7c8fa
always use main thread on ttid
lucas-zimerman Oct 15, 2024
d43ce2b
Merge branch 'v5' into test/deadline
lucas-zimerman Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

### Fixes

- Enhanced accuracy of time-to-display spans. ([#4042](https://github.com/getsentry/sentry-react-native/pull/4042))
### Dependencies

- Bump CLI from v2.36.1 to v2.36.5 ([#4116](https://github.com/getsentry/sentry-react-native/pull/4116), [#4131](https://github.com/getsentry/sentry-react-native/pull/4131), [#4137](https://github.com/getsentry/sentry-react-native/pull/4137))
Expand Down
12 changes: 7 additions & 5 deletions android/src/main/java/io/sentry/react/RNSentryModuleImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -79,7 +75,6 @@
import io.sentry.android.core.ViewHierarchyEventProcessor;
import io.sentry.android.core.internal.debugmeta.AssetsDebugMetaLoader;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import io.sentry.android.core.performance.AppStartMetrics;
import io.sentry.protocol.SdkVersion;
import io.sentry.protocol.SentryException;
import io.sentry.protocol.SentryId;
Expand Down Expand Up @@ -135,10 +130,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() {
Expand Down Expand Up @@ -693,6 +691,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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,4 @@ public List<ViewManager> createViewManagers(
new RNSentryOnDrawReporterManager(reactContext)
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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 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);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
9 changes: 9 additions & 0 deletions ios/RNSentry.mm
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import <dlfcn.h>
#import "RNSentry.h"
#import "RNSentryTimeToDisplay.h"

#if __has_include(<React/RCTConvert.h>)
#import <React/RCTConvert.h>
Expand Down Expand Up @@ -62,6 +63,7 @@ + (void)storeEnvelope:(SentryEnvelope *)envelope;
@implementation RNSentry {
bool sentHybridSdkDidBecomeActive;
bool hasListeners;
RNSentryTimeToDisplay *_timeToDisplay;
}

- (dispatch_queue_t)methodQueue
Expand Down Expand Up @@ -106,6 +108,8 @@ + (BOOL)requiresMainQueueSetup {
sentHybridSdkDidBecomeActive = true;
}

_timeToDisplay = [[RNSentryTimeToDisplay alloc] init];

#if SENTRY_TARGET_REPLAY_SUPPORTED
[RNSentryReplay postInit];
#endif
Expand Down Expand Up @@ -776,4 +780,9 @@ - (NSDictionary*) fetchNativeStackFramesBy: (NSArray<NSNumber*>*)instructionsAdd
}
#endif

RCT_EXPORT_METHOD(getNewScreenTimeToDisplay:(RCTPromiseResolveBlock)resolve
rejecter:(RCTPromiseRejectBlock)reject) {
[_timeToDisplay getTimeToDisplay:resolve];
}

@end
7 changes: 7 additions & 0 deletions ios/RNSentryTimeToDisplay.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#import <React/RCTBridgeModule.h>

@interface RNSentryTimeToDisplay : NSObject

- (void)getTimeToDisplay:(RCTResponseSenderBlock)callback;

@end
43 changes: 43 additions & 0 deletions ios/RNSentryTimeToDisplay.m
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions src/js/NativeRNSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 7 additions & 3 deletions src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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}`);
});
Expand Down Expand Up @@ -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) => {
Expand All @@ -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');
Expand Down
103 changes: 103 additions & 0 deletions src/js/utils/sentryeventemitterfallback.ts
Original file line number Diff line number Diff line change
@@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closeAll doesn't seem to be used anywhere.

Copy link
Collaborator Author

@lucas-zimerman lucas-zimerman Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also remove the closeAll on sentryeventemitter? It seems to not be used other than tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can clean that up, after the fallback. I agree.

subscription?.remove();
},
};
}
9 changes: 9 additions & 0 deletions src/js/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ interface SentryNativeWrapper {
getCurrentReplayId(): string | null;

crashedLastRun(): Promise<boolean | null>;
getNewScreenTimeToDisplay(): Promise<number | null | undefined>;
}

const EOL = utf8ToBytes('\n');
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions test/mockWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const NATIVE: MockInterface<NativeType> = {
initNativeReactNavigationNewFrameTracking: jest.fn(),

crashedLastRun: jest.fn(),
getNewScreenTimeToDisplay: jest.fn().mockResolvedValue(42),
};

NATIVE.isNativeAvailable.mockReturnValue(true);
Expand Down
Loading
Loading