feat(react-native): periodically clean up native spans that are open …

…for more than one hour
bugsnag · Jan 8, 2025 · 254fe8e · 254fe8e
1 parent 7d25f48
commit 254fe8e
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 39 deletions.
diff --git a/packages/platforms/react-native/__mocks__/react-native.ts b/packages/platforms/react-native/__mocks__/react-native.ts
@@ -141,7 +141,7 @@ const BugsnagReactNativePerformance = {
   isNativePerformanceAvailable: jest.fn(() => {
     return true
   }),
-  getNativeConfiguration: jest.fn(() => {
+  attachToNativeSDK: jest.fn(() => {
     return {
       apiKey: VALID_API_KEY,
       endpoint: '/traces',

diff --git a/...e/android/src/main/java/com/bugsnag/android/performance/NativeBugsnagPerformanceImpl.java b/...e/android/src/main/java/com/bugsnag/android/performance/NativeBugsnagPerformanceImpl.java
@@ -12,7 +12,11 @@
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.WritableMap;
 import java.security.SecureRandom;
-import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 
 import com.bugsnag.android.performance.BugsnagPerformance;
 import com.bugsnag.android.performance.SpanOptions;
@@ -41,7 +45,7 @@ class NativeBugsnagPerformanceImpl {
    * Since native spans are only ever started and ended from the JS thread,
    * no thread synchronization is required when accessing.
    */
-  private final HashMap<String, SpanImpl> openSpans = new HashMap<>();
+  private final ConcurrentHashMap<String, SpanImpl> openSpans = new ConcurrentHashMap<>();
 
   public NativeBugsnagPerformanceImpl(ReactApplicationContext reactContext) {
     this.reactContext = reactContext;
@@ -109,7 +113,7 @@ public boolean isNativePerformanceAvailable() {
   }
 
   @Nullable
-  public WritableMap getNativeConfiguration() {
+  public WritableMap attachToNativeSDK() {
     if (!isNativePerformanceAvailable) {
       return null;
     }
@@ -119,6 +123,9 @@ public WritableMap getNativeConfiguration() {
       return null;
     }
 
+    ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
+    scheduler.scheduleAtFixedRate(this::discardOldSpans, 1, 1, TimeUnit.HOURS);
+
     WritableMap result = Arguments.createMap();
     result.putString("apiKey", nativeConfig.getApiKey());
     result.putString("endpoint", nativeConfig.getEndpoint());
@@ -240,6 +247,16 @@ private SpanOptions readableMapToSpanOptions(ReadableMap jsOptions) {
     return spanOptions;
   }
 
+  private void discardOldSpans() {
+        long oneHourAgo = System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1);
+        for (Map.Entry<String, SpanImpl> entry : openSpans.entrySet()) {
+            SpanImpl span = entry.getValue();
+            if (span.getStartTime$internal() < oneHourAgo) {
+                openSpans.remove(entry.getKey());
+                span.discard();
+            }
+        }
+    }
 
   @Nullable
   private String abiToArchitecture(@Nullable String abi) {

diff --git a/...droid/src/newarch/java/com/bugsnag/android/performance/BugsnagReactNativePerformance.java b/...droid/src/newarch/java/com/bugsnag/android/performance/BugsnagReactNativePerformance.java
@@ -42,8 +42,8 @@ public boolean isNativePerformanceAvailable() {
   }
 
   @Override
-  public WritableMap getNativeConfiguration() {
-    return impl.getNativeConfiguration();
+  public WritableMap attachToNativeSDK() {
+    return impl.attachToNativeSDK();
   }
 
   @Override 

diff --git a/...droid/src/oldarch/java/com/bugsnag/android/performance/BugsnagReactNativePerformance.java b/...droid/src/oldarch/java/com/bugsnag/android/performance/BugsnagReactNativePerformance.java
@@ -44,8 +44,8 @@ public boolean isNativePerformanceAvailable() {
   }
 
   @ReactMethod(isBlockingSynchronousMethod = true)
-  public WritableMap getNativeConfiguration() {
-    return impl.getNativeConfiguration();
+  public WritableMap attachToNativeSDK() {
+    return impl.attachToNativeSDK();
   }
 
   @ReactMethod(isBlockingSynchronousMethod = true)

diff --git a/packages/platforms/react-native/ios/BugsnagReactNativePerformance.mm b/packages/platforms/react-native/ios/BugsnagReactNativePerformance.mm
@@ -11,6 +11,8 @@ @implementation BugsnagReactNativePerformance
 
 static NSUInteger traceIdMidpoint = 16;
 
+static NSTimeInterval hourInSeconds = 3600;
+
 /**
 * A dictionary of open native spans, keyed by the span ID and trace ID,
 * so that they can be retrieved and closed/discarded from JS.
@@ -130,7 +132,7 @@ static uint64_t hexStringToUInt64(NSString *hexString) {
     return [NSNumber numberWithBool:BugsnagReactNativePerformanceCrossTalkAPIClient.isInitialized];
 }
 
-RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(getNativeConfiguration) {
+RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(attachToNativeSDK) {
     if (!BugsnagReactNativePerformanceCrossTalkAPIClient.isInitialized) {
         return nil;
     }
@@ -140,6 +142,12 @@ static uint64_t hexStringToUInt64(NSString *hexString) {
         return nil;
     }
 
+    NSTimer *discardOldSpansTimer = [NSTimer scheduledTimerWithTimeInterval:3600
+                                                                target:self
+                                                              selector:@selector(discardOldSpans)
+                                                              userInfo:nil
+                                                               repeats:YES];
+
     NSMutableDictionary *config = [NSMutableDictionary new];
     config[@"apiKey"] = nativeConfig.apiKey;
     config[@"endpoint"] = [nativeConfig.endpoint absoluteString];
@@ -195,7 +203,10 @@ static uint64_t hexStringToUInt64(NSString *hexString) {
 
     NSString *spanId = [NSString stringWithFormat:@"%llx", nativeSpan.spanId];
     NSString *traceId = [NSString stringWithFormat:@"%llx%llx", nativeSpan.traceIdHi, nativeSpan.traceIdLo];
-    openSpans[[spanId stringByAppendingString:traceId]] = nativeSpan;
+
+    @synchronized (openSpans) {
+        openSpans[[spanId stringByAppendingString:traceId]] = nativeSpan;
+    }
 
     NSMutableDictionary *span = [NSMutableDictionary new];
     span[@"name"] = nativeSpan.name;
@@ -216,35 +227,40 @@ static uint64_t hexStringToUInt64(NSString *hexString) {
                 resolve:(RCTPromiseResolveBlock)resolve
                 reject:(RCTPromiseRejectBlock)reject) {
     NSString *spanKey = [spanId stringByAppendingString:traceId];
-    BugsnagPerformanceSpan *nativeSpan = openSpans[spanKey];
-    if (nativeSpan != nil) {
-        [openSpans removeObjectForKey:spanKey];
-
-        // Set native span attributes from JS values
-        [ReactNativeSpanAttributes setNativeAttributes:nativeSpan.attributes fromJSAttributes:attributes];
 
-        // We need to reinstate the bugsnag.sampling.p attribute here as it might not be re-populated on span end
-        nativeSpan.attributes[@"bugsnag.sampling.p"] = @(nativeSpan.samplingProbability);
-
-        // If the end time is later than the current end time, update it
-        NSDate *nativeEndTime = [NSDate dateWithTimeIntervalSince1970: endTime / NSEC_PER_SEC];
-        if ([nativeEndTime timeIntervalSinceDate:nativeSpan.endTime] > 0) {
-            [nativeSpan markEndTime:nativeEndTime];
+    @synchronized (openSpans) {
+        BugsnagPerformanceSpan *nativeSpan = openSpans[spanKey];
+        if (nativeSpan != nil) {
+            [openSpans removeObjectForKey:spanKey];
+
+            // Set native span attributes from JS values
+            [ReactNativeSpanAttributes setNativeAttributes:nativeSpan.attributes fromJSAttributes:attributes];
+
+            // We need to reinstate the bugsnag.sampling.p attribute here as it might not be re-populated on span end
+            nativeSpan.attributes[@"bugsnag.sampling.p"] = @(nativeSpan.samplingProbability);
+
+            // If the end time is later than the current end time, update it
+            NSDate *nativeEndTime = [NSDate dateWithTimeIntervalSince1970: endTime / NSEC_PER_SEC];
+            if ([nativeEndTime timeIntervalSinceDate:nativeSpan.endTime] > 0) {
+                [nativeSpan markEndTime:nativeEndTime];
+            }
+
+            [nativeSpan sendForProcessing];
         }
-
-        [nativeSpan sendForProcessing];
     }
 
     resolve(nil);
 }
 
 RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(markNativeSpanEndTime:(NSString *)spanId traceId:(NSString *)traceId endTime:(double)endTime) {
-    BugsnagPerformanceSpan *nativeSpan = openSpans[[spanId stringByAppendingString:traceId]];
-    if (nativeSpan != nil) {
-        NSDate *nativeEndTime = [NSDate dateWithTimeIntervalSince1970: endTime / NSEC_PER_SEC];
-        [nativeSpan markEndTime:nativeEndTime];
+    @synchronized (openSpans) {
+        BugsnagPerformanceSpan *nativeSpan = openSpans[[spanId stringByAppendingString:traceId]];
+        if (nativeSpan != nil) {
+            NSDate *nativeEndTime = [NSDate dateWithTimeIntervalSince1970: endTime / NSEC_PER_SEC];
+            [nativeSpan markEndTime:nativeEndTime];
+        }
     }
-
+    
     return nil;
 }
 
@@ -253,15 +269,34 @@ static uint64_t hexStringToUInt64(NSString *hexString) {
                 resolve:(RCTPromiseResolveBlock)resolve
                 reject:(RCTPromiseRejectBlock)reject) {
     NSString *spanKey = [spanId stringByAppendingString:traceId];
-    BugsnagPerformanceSpan *nativeSpan = openSpans[spanKey];    
-    if (nativeSpan != nil) {
-        [openSpans removeObjectForKey:spanKey];
-        [nativeSpan abortUnconditionally];
+    BugsnagPerformanceSpan *nativeSpan
+    @synchronized (openSpans) {
+        nativeSpan = openSpans[spanKey];    
+        if (nativeSpan != nil) {
+            [openSpans removeObjectForKey:spanKey];
+            [nativeSpan abortUnconditionally];
+        }
     }
 
     resolve(nil);
 }
 
+- (void)discardOldSpans {
+    NSDate *oneHourAgo = [NSDate dateWithTimeIntervalSinceNow:-hourInSeconds];
+    NSMutableArray *keysToRemove = [NSMutableArray new];
+
+    @synchronized (openSpans) {
+        for (NSString *key in openSpans) {
+            BugsnagPerformanceSpan *span = openSpans[key];
+            if ([span.startTime compare:oneHourAgo] == NSOrderedAscending) {
+                [keysToRemove addObject:key];
+            }
+        }
+
+        [openSpans removeObjectsForKeys:keysToRemove];
+    }
+}
+
 #ifdef RCT_NEW_ARCH_ENABLED
 - (std::shared_ptr<facebook::react::TurboModule>)getTurboModule:
     (const facebook::react::ObjCTurboModule::InitParams &)params

diff --git a/packages/platforms/react-native/lib/NativeBugsnagPerformance.ts b/packages/platforms/react-native/lib/NativeBugsnagPerformance.ts
@@ -42,7 +42,7 @@ export interface Spec extends TurboModule {
   requestEntropy: () => string
   requestEntropyAsync: () => Promise<string>
   isNativePerformanceAvailable: () => boolean
-  getNativeConfiguration: () => NativeConfiguration | null
+  attachToNativeSDK: () => NativeConfiguration | null
   startNativeSpan: (name: string, options: UnsafeObject) => NativeSpan
   endNativeSpan: (spanId: string, traceId: string, endTime: number, attributes: UnsafeObject) => Promise<void>
   markNativeSpanEndTime: (spanId: string, traceId: string, endTime: number) => void

diff --git a/packages/platforms/react-native/lib/platform-extensions.tsx b/packages/platforms/react-native/lib/platform-extensions.tsx
@@ -38,7 +38,7 @@ export const platformExtensions = (appStartTime: number, clock: Clock, schema: R
       return
     }
 
-    const nativeConfig = NativeBugsnagPerformance?.getNativeConfiguration()
+    const nativeConfig = NativeBugsnagPerformance?.attachToNativeSDK()
     if (!nativeConfig) {
       logger.warn(`Could not attach to native SDK. Bugsnag ${platform} Performance has not been started.`)
       return

diff --git a/packages/platforms/react-native/tests/client.test.ts b/packages/platforms/react-native/tests/client.test.ts
@@ -81,7 +81,7 @@ describe('React Native client tests', () => {
     })
 
     it('logs a warning and noops if native performance has not been started', () => {
-      turboModule.getNativeConfiguration = jest.fn().mockReturnValue(null)
+      turboModule.attachToNativeSDK = jest.fn().mockReturnValue(null)
 
       client = require('../lib/client').default
       const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {})
@@ -91,7 +91,7 @@ describe('React Native client tests', () => {
     })
 
     it('starts the client using the native configuration', () => {
-      const nativeConfig = turboModule.getNativeConfiguration()
+      const nativeConfig = turboModule.attachToNativeSDK()
       client = require('../lib/client').default
       const startSpy = jest.spyOn(client, 'start')
 
@@ -124,7 +124,7 @@ describe('React Native client tests', () => {
     })
 
     it('does not overwrite native configuration with JS values', () => {
-      const nativeConfig = turboModule.getNativeConfiguration()
+      const nativeConfig = turboModule.attachToNativeSDK()
       client = require('../lib/client').default
       const startSpy = jest.spyOn(client, 'start')