From b1d7a598a897b3d332d4e9171dae697d64c9656d Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Mon, 6 Jan 2025 16:10:35 +0000 Subject: [PATCH 1/2] refactor(react-native): move navigation span creation logic to ReactNativeSpanFactory --- .../lib/create-navigation-span.ts | 16 -------- packages/platforms/react-native/lib/index.ts | 1 - .../react-native/lib/platform-extensions.tsx | 4 +- .../react-native/lib/span-factory.ts | 16 +++++++- .../tests/create-navigation-span.test.ts | 37 ------------------- .../tests/platform-extensions.test.ts | 5 ++- .../react-native/tests/span-factory.test.ts | 30 +++++++++++++++ .../@bugsnag/react-native-performance.ts | 1 - .../react-native-navigation-plugin.test.ts | 8 ++-- .../lib/react-native-navigation-plugin.ts | 6 +-- .../@bugsnag/react-native-performance.ts | 1 - .../lib/navigation-context.tsx | 3 +- .../test/create-navigation-container.test.tsx | 11 +++--- .../test/navigation-context.test.tsx | 12 +++--- packages/test-utilities/lib/index.ts | 2 +- .../test-utilities/lib/mock-span-factory.ts | 37 ++++++++++++++++++- 16 files changed, 104 insertions(+), 86 deletions(-) delete mode 100644 packages/platforms/react-native/lib/create-navigation-span.ts delete mode 100644 packages/platforms/react-native/tests/create-navigation-span.test.ts diff --git a/packages/platforms/react-native/lib/create-navigation-span.ts b/packages/platforms/react-native/lib/create-navigation-span.ts deleted file mode 100644 index e65379f3a..000000000 --- a/packages/platforms/react-native/lib/create-navigation-span.ts +++ /dev/null @@ -1,16 +0,0 @@ -import type { SpanOptions } from '@bugsnag/core-performance' -import type { ReactNativeSpanFactory } from './span-factory' - -export function createNavigationSpan (spanFactory: ReactNativeSpanFactory, routeName: string, spanOptions: SpanOptions) { - // Navigation spans are always first class - spanOptions.isFirstClass = true - - const spanName = '[Navigation]' + routeName - const span = spanFactory.startSpan(spanName, spanOptions) - - // Default navigation span attributes - span.setAttribute('bugsnag.span.category', 'navigation') - span.setAttribute('bugsnag.navigation.route', routeName) - - return span -} diff --git a/packages/platforms/react-native/lib/index.ts b/packages/platforms/react-native/lib/index.ts index 3b929333d..f57768d23 100644 --- a/packages/platforms/react-native/lib/index.ts +++ b/packages/platforms/react-native/lib/index.ts @@ -5,5 +5,4 @@ export type { PlatformExtensions } from './platform-extensions' export default BugsnagPerformance -export { createNavigationSpan } from './create-navigation-span' export * from './span-factory' diff --git a/packages/platforms/react-native/lib/platform-extensions.tsx b/packages/platforms/react-native/lib/platform-extensions.tsx index f9e1bc829..a1573da9c 100644 --- a/packages/platforms/react-native/lib/platform-extensions.tsx +++ b/packages/platforms/react-native/lib/platform-extensions.tsx @@ -4,7 +4,6 @@ import { Platform } from 'react-native' import NativeBugsnagPerformance from './native' import type { ReactNativeAttachConfiguration, ReactNativeConfiguration, ReactNativeSchema } from './config' import { createAppStartSpan } from './create-app-start-span' -import { createNavigationSpan } from './create-navigation-span' import type { ReactNativeSpanFactory } from './span-factory' type NavigationSpanOptions = Omit @@ -12,8 +11,7 @@ type NavigationSpanOptions = Omit export const platformExtensions = (appStartTime: number, clock: Clock, schema: ReactNativeSchema, spanFactory: ReactNativeSpanFactory, spanContextStorage: SpanContextStorage) => ({ startNavigationSpan: function (routeName: string, spanOptions?: NavigationSpanOptions) { const cleanOptions = spanFactory.validateSpanOptions(routeName, spanOptions) - cleanOptions.options.isFirstClass = true - const span = createNavigationSpan(spanFactory, cleanOptions.name, cleanOptions.options) + const span = spanFactory.startNavigationSpan(cleanOptions.name, cleanOptions.options) return spanFactory.toPublicApi(span) }, withInstrumentedAppStarts: function (App: React.FC) { diff --git a/packages/platforms/react-native/lib/span-factory.ts b/packages/platforms/react-native/lib/span-factory.ts index 3cd8a2161..1fe6e8c59 100644 --- a/packages/platforms/react-native/lib/span-factory.ts +++ b/packages/platforms/react-native/lib/span-factory.ts @@ -1,5 +1,5 @@ import { runSpanEndCallbacks, SpanFactory, SpanInternal } from '@bugsnag/core-performance' -import type { SpanAttributes, ParentContext } from '@bugsnag/core-performance' +import type { SpanAttributes, ParentContext, SpanOptions } from '@bugsnag/core-performance' import type { ReactNativeConfiguration } from './config' import NativeBugsnagPerformance from './native' import type { ReactNativeClock } from './clock' @@ -51,4 +51,18 @@ export class ReactNativeSpanFactory extends SpanFactory { - it('sets the span name to the route prefixed with [Navigation]', () => { - const spanFactory = new MockSpanFactory() - const span = createNavigationSpan(spanFactory as unknown as ReactNativeSpanFactory, 'testRoute', { isFirstClass: false }) - const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) - - expect(endedSpan.name).toBe('[Navigation]testRoute') - }) - - it('always sets the span as first class', () => { - const spanFactory = new MockSpanFactory() - const span = createNavigationSpan(spanFactory as unknown as ReactNativeSpanFactory, 'testRoute', { isFirstClass: false }) - const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) - - expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.span.first_class', value: { boolValue: true } }) - }) - - it('includes navigation category attribute', () => { - const spanFactory = new MockSpanFactory() - const span = createNavigationSpan(spanFactory as unknown as ReactNativeSpanFactory, 'testRoute', { isFirstClass: true }) - const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) - - expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.span.category', value: { stringValue: 'navigation' } }) - }) - - it('includes the route attribute', () => { - const spanFactory = new MockSpanFactory() - const span = createNavigationSpan(spanFactory as unknown as ReactNativeSpanFactory, 'testRoute', { isFirstClass: true }) - const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) - - expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.navigation.route', value: { stringValue: 'testRoute' } }) - }) -}) diff --git a/packages/platforms/react-native/tests/platform-extensions.test.ts b/packages/platforms/react-native/tests/platform-extensions.test.ts index 393f854a8..8806dfc51 100644 --- a/packages/platforms/react-native/tests/platform-extensions.test.ts +++ b/packages/platforms/react-native/tests/platform-extensions.test.ts @@ -1,4 +1,4 @@ -import type { ReactNativeSpanFactory } from '../lib' +import { ReactNativeSpanFactory } from '../lib/span-factory' import createSchema from '../lib/config' import { platformExtensions } from '../lib/platform-extensions' import { createTestClient, IncrementingClock, InMemoryDelivery, VALID_API_KEY } from '@bugsnag/js-performance-test-utilities' @@ -11,7 +11,8 @@ describe('startNavigationSpan', () => { const clock = new IncrementingClock() const testClient = createTestClient({ deliveryFactory: () => delivery, - platformExtensions: (spanFactory, spanContextStorage) => platformExtensions(0, clock, createSchema(), spanFactory as unknown as ReactNativeSpanFactory, spanContextStorage) + spanFactory: ReactNativeSpanFactory, + platformExtensions: (spanFactory, spanContextStorage) => platformExtensions(0, clock, createSchema(), spanFactory as ReactNativeSpanFactory, spanContextStorage) }) testClient.start({ apiKey: VALID_API_KEY }) diff --git a/packages/platforms/react-native/tests/span-factory.test.ts b/packages/platforms/react-native/tests/span-factory.test.ts index cffd13334..6dc390faf 100644 --- a/packages/platforms/react-native/tests/span-factory.test.ts +++ b/packages/platforms/react-native/tests/span-factory.test.ts @@ -194,4 +194,34 @@ describe('ReactNativeSpanFactory', () => { expect(processor.spans.length).toBe(0) }) }) + + describe('startNavigationSpan', () => { + it('sets the span name to the route prefixed with [Navigation]', () => { + const span = spanFactory.startNavigationSpan('testRoute', {}) + const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) + + expect(endedSpan.name).toBe('[Navigation]testRoute') + }) + + it('always sets the span as first class', () => { + const span = spanFactory.startNavigationSpan('testRoute', { isFirstClass: false }) + const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) + + expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.span.first_class', value: { boolValue: true } }) + }) + + it('includes navigation category attribute', () => { + const span = spanFactory.startNavigationSpan('testRoute', {}) + const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) + + expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.span.category', value: { stringValue: 'navigation' } }) + }) + + it('includes the route attribute', () => { + const span = spanFactory.startNavigationSpan('testRoute', {}) + const endedSpan = span.end(12345, spanFactory.sampler.spanProbability) + + expect(endedSpan.attributes.toJson()).toContainEqual({ key: 'bugsnag.navigation.route', value: { stringValue: 'testRoute' } }) + }) + }) }) diff --git a/packages/plugin-react-native-navigation/__mocks__/@bugsnag/react-native-performance.ts b/packages/plugin-react-native-navigation/__mocks__/@bugsnag/react-native-performance.ts index 7f41d90a4..3fde35c7b 100644 --- a/packages/plugin-react-native-navigation/__mocks__/@bugsnag/react-native-performance.ts +++ b/packages/plugin-react-native-navigation/__mocks__/@bugsnag/react-native-performance.ts @@ -1,6 +1,5 @@ import type { Configuration, Plugin } from '@bugsnag/core-performance' import type { ReactNativeConfiguration } from '@bugsnag/react-native-performance' -export { createNavigationSpan } from '@bugsnag/react-native-performance/lib/create-navigation-span' const plugins: Array> = [] diff --git a/packages/plugin-react-native-navigation/__tests__/react-native-navigation-plugin.test.ts b/packages/plugin-react-native-navigation/__tests__/react-native-navigation-plugin.test.ts index 0e1f4da35..d53b1a6de 100644 --- a/packages/plugin-react-native-navigation/__tests__/react-native-navigation-plugin.test.ts +++ b/packages/plugin-react-native-navigation/__tests__/react-native-navigation-plugin.test.ts @@ -1,4 +1,4 @@ -import { MockSpanFactory, createConfiguration } from '@bugsnag/js-performance-test-utilities' +import { MockReactNativeSpanFactory, createConfiguration } from '@bugsnag/js-performance-test-utilities' import type { ReactNativeConfiguration } from '@bugsnag/react-native-performance' import { Navigation } from 'react-native-navigation' import ReactNativeNavigationPlugin from '../lib/react-native-navigation-plugin' @@ -17,7 +17,7 @@ describe('ReactNativeNavigationPlugin', () => { it('creates a navigation span when the route changes', () => { const plugin = new ReactNativeNavigationPlugin(Navigation) const configuration = createConfiguration() - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() plugin.configure(configuration, spanFactory) // Simulate a route change @@ -49,7 +49,7 @@ describe('ReactNativeNavigationPlugin', () => { it('does not end the current navigation while there are components waiting', () => { const plugin = new ReactNativeNavigationPlugin(Navigation) const configuration = createConfiguration() - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() plugin.configure(configuration, spanFactory) // Simulate a route change @@ -98,7 +98,7 @@ describe('ReactNativeNavigationPlugin', () => { it('discards the active navigation span when the route changes', () => { const plugin = new ReactNativeNavigationPlugin(Navigation) const configuration = createConfiguration() - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() plugin.configure(configuration, spanFactory) // Simulate a route change diff --git a/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts b/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts index f3f228afd..c45e1c3dd 100644 --- a/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts +++ b/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts @@ -2,8 +2,6 @@ import type { Plugin, SpanFactory, SpanInternal } from '@bugsnag/core-performanc import type { ReactNativeConfiguration, ReactNativeSpanFactory } from '@bugsnag/react-native-performance' import type { NavigationDelegate } from 'react-native-navigation/lib/dist/src/NavigationDelegate' -import { createNavigationSpan } from '@bugsnag/react-native-performance' - type Reason = 'immediate' | 'mount' | 'unmount' | 'condition' const NAVIGATION_START_TIMEOUT = 1000 @@ -80,11 +78,11 @@ class BugsnagPluginReactNativeNavigationPerformance implements Plugin { - if (typeof this.startTime === 'number') { + if (this.spanFactory && typeof this.startTime === 'number') { clearTimeout(this.startTimeout) const routeName = event.componentName - this.currentNavigationSpan = createNavigationSpan(spanFactory as ReactNativeSpanFactory, routeName, { startTime: this.startTime }) + this.currentNavigationSpan = this.spanFactory.startNavigationSpan(routeName, { startTime: this.startTime }) this.currentNavigationSpan.setAttribute('bugsnag.navigation.triggered_by', '@bugsnag/plugin-react-native-navigation-performance') if (this.previousRoute) { diff --git a/packages/plugin-react-navigation/__mocks__/@bugsnag/react-native-performance.ts b/packages/plugin-react-navigation/__mocks__/@bugsnag/react-native-performance.ts index 7f41d90a4..3fde35c7b 100644 --- a/packages/plugin-react-navigation/__mocks__/@bugsnag/react-native-performance.ts +++ b/packages/plugin-react-navigation/__mocks__/@bugsnag/react-native-performance.ts @@ -1,6 +1,5 @@ import type { Configuration, Plugin } from '@bugsnag/core-performance' import type { ReactNativeConfiguration } from '@bugsnag/react-native-performance' -export { createNavigationSpan } from '@bugsnag/react-native-performance/lib/create-navigation-span' const plugins: Array> = [] diff --git a/packages/plugin-react-navigation/lib/navigation-context.tsx b/packages/plugin-react-navigation/lib/navigation-context.tsx index abd19d8e2..349911258 100644 --- a/packages/plugin-react-navigation/lib/navigation-context.tsx +++ b/packages/plugin-react-navigation/lib/navigation-context.tsx @@ -3,7 +3,6 @@ import type { ReactNativeSpanFactory } from '@bugsnag/react-native-performance' import type { PropsWithChildren } from 'react' import React from 'react' -import { createNavigationSpan } from '@bugsnag/react-native-performance' export const NavigationContext = React.createContext({ blockNavigationEnd: () => {}, @@ -73,7 +72,7 @@ export class NavigationContextProvider extends React.Component { spanFactory.endSpan(this.currentSpan, DISCARDED) } - const span = createNavigationSpan(spanFactory, currentRoute, { startTime: updateTime }) + const span = spanFactory.startNavigationSpan(currentRoute, { startTime: updateTime }) span.setAttribute('bugsnag.navigation.triggered_by', '@bugsnag/plugin-react-navigation-performance') if (this.previousRoute) { diff --git a/packages/plugin-react-navigation/test/create-navigation-container.test.tsx b/packages/plugin-react-navigation/test/create-navigation-container.test.tsx index 8e9d5f347..3ee532297 100644 --- a/packages/plugin-react-navigation/test/create-navigation-container.test.tsx +++ b/packages/plugin-react-navigation/test/create-navigation-container.test.tsx @@ -1,5 +1,4 @@ -import { MockSpanFactory } from '@bugsnag/js-performance-test-utilities' -import type { ReactNativeConfiguration, ReactNativeSpanFactory } from '@bugsnag/react-native-performance' +import { MockReactNativeSpanFactory } from '@bugsnag/js-performance-test-utilities' import { NavigationContainer, createNavigationContainerRef } from '@react-navigation/native' import type { ParamListBase } from '@react-navigation/native' import { createNativeStackNavigator } from '@react-navigation/native-stack' @@ -19,8 +18,8 @@ afterEach(() => { describe('createNavigationContainer', () => { it('creates a navigation span when the route changes', () => { - const spanFactory = new MockSpanFactory() - const BugsnagNavigationContainer = createNavigationContainer(NavigationContainer, spanFactory as unknown as ReactNativeSpanFactory) + const spanFactory = new MockReactNativeSpanFactory() + const BugsnagNavigationContainer = createNavigationContainer(NavigationContainer, spanFactory) render( @@ -43,8 +42,8 @@ describe('createNavigationContainer', () => { it('forwards the provided ref to the NavigationContainer', () => { const navigationRef = createNavigationContainerRef() - const spanFactory = new MockSpanFactory() - const BugsnagNavigationContainer = createNavigationContainer(NavigationContainer, spanFactory as unknown as ReactNativeSpanFactory) + const spanFactory = new MockReactNativeSpanFactory() + const BugsnagNavigationContainer = createNavigationContainer(NavigationContainer, spanFactory) render( diff --git a/packages/plugin-react-navigation/test/navigation-context.test.tsx b/packages/plugin-react-navigation/test/navigation-context.test.tsx index 950b4ed9c..da7226978 100644 --- a/packages/plugin-react-navigation/test/navigation-context.test.tsx +++ b/packages/plugin-react-navigation/test/navigation-context.test.tsx @@ -1,5 +1,5 @@ import type { SpanFactory } from '@bugsnag/core-performance' -import { MockSpanFactory } from '@bugsnag/js-performance-test-utilities' +import { MockReactNativeSpanFactory } from '@bugsnag/js-performance-test-utilities' import type { ReactNativeConfiguration, ReactNativeSpanFactory } from '@bugsnag/react-native-performance' import { fireEvent, render, screen } from '@testing-library/react-native' import React, { useContext } from 'react' @@ -16,7 +16,7 @@ afterEach(() => { describe('NavigationContextProvider', () => { it('Creates a navigation span when the currentRoute changes', () => { - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() render() // Initial route should not create a span @@ -43,7 +43,7 @@ describe('NavigationContextProvider', () => { }) it('Discards the active navigation span when the route changes', () => { - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() render() // Change to a new route but block the navigation span from ending @@ -79,7 +79,7 @@ describe('NavigationContextProvider', () => { }) it('Prevents a navigation span from ending when navigation is blocked', () => { - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() render() // Start a navigation @@ -106,7 +106,7 @@ describe('NavigationContextProvider', () => { }) it('Does not end a navigation span while multiple components are blocking', () => { - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() render() // Start a navigation @@ -133,7 +133,7 @@ describe('NavigationContextProvider', () => { it('resets the lastRenderTime when a navigation span ends', () => { jest.useFakeTimers() - const spanFactory = new MockSpanFactory() + const spanFactory = new MockReactNativeSpanFactory() render() // start navigation diff --git a/packages/test-utilities/lib/index.ts b/packages/test-utilities/lib/index.ts index 3683e3b74..a44026c4b 100644 --- a/packages/test-utilities/lib/index.ts +++ b/packages/test-utilities/lib/index.ts @@ -9,7 +9,7 @@ export { default as InMemoryDelivery } from './in-memory-delivery' export { default as InMemoryProcessor } from './in-memory-processor' export { default as IncrementingClock } from './incrementing-clock' export { default as IncrementingIdGenerator } from './incrementing-id-generator' -export { default as MockSpanFactory } from './mock-span-factory' +export * from './mock-span-factory' export { default as resourceAttributesSource } from './resource-attributes-source' export { default as createSpanAttributes } from './create-span-attributes' export { default as spanAttributesSource } from './span-attributes-source' diff --git a/packages/test-utilities/lib/mock-span-factory.ts b/packages/test-utilities/lib/mock-span-factory.ts index 7e2c10e4f..2f6b89e59 100644 --- a/packages/test-utilities/lib/mock-span-factory.ts +++ b/packages/test-utilities/lib/mock-span-factory.ts @@ -9,6 +9,7 @@ import InMemoryProcessor from './in-memory-processor' import IncrementingClock from './incrementing-clock' import spanAttributesSource from './span-attributes-source' import StableIdGenerator from './stable-id-generator' +import { ReactNativeSpanFactory } from '@bugsnag/react-native-performance/lib/span-factory' const jestLogger = { debug: jest.fn(), @@ -47,4 +48,38 @@ class MockSpanFactory extends SpanFactory { }) } -export default MockSpanFactory +class MockReactNativeSpanFactory extends ReactNativeSpanFactory { + public createdSpans: SpanEnded[] + + constructor () { + const processor = new InMemoryProcessor() + const backgroundingListener = new ControllableBackgroundingListener() + + super( + processor, + new Sampler(1.0), + new StableIdGenerator(), + spanAttributesSource, + new IncrementingClock(), + backgroundingListener, + jestLogger, + new DefaultSpanContextStorage(backgroundingListener) + ) + + this.createdSpans = processor.spans + } + + startSpan = jest.fn((name: string, options: SpanOptions) => { + return super.startSpan(name, options) + }) + + endSpan = jest.fn((span: SpanInternal, endTime: number, additionalAttributes?: Record) => { + super.endSpan(span, endTime, additionalAttributes) + }) + + startNavigationSpan = jest.fn((name: string, options: SpanOptions) => { + return super.startNavigationSpan(name, options) + }) +} + +export { MockSpanFactory, MockReactNativeSpanFactory } From 27d161b69d39b813674eeed42d04659ce8e6b722 Mon Sep 17 00:00:00 2001 From: Yousif Ahmed Date: Wed, 8 Jan 2025 15:07:47 +0000 Subject: [PATCH 2/2] feat(react-native): do not delegate automatic navigation spans to native SDK --- packages/core/lib/span-factory.ts | 19 ++++++------- .../react-native/lib/span-factory.ts | 27 ++++++++++++------- .../react-native/tests/span-factory.test.ts | 9 +++++++ .../lib/react-native-navigation-plugin.ts | 2 +- .../lib/navigation-context.tsx | 2 +- .../test/create-navigation-container.test.tsx | 12 ++++----- .../test/navigation-context.test.tsx | 20 +++++++------- 7 files changed, 53 insertions(+), 38 deletions(-) diff --git a/packages/core/lib/span-factory.ts b/packages/core/lib/span-factory.ts index a5c99b434..1f5e9ed3c 100644 --- a/packages/core/lib/span-factory.ts +++ b/packages/core/lib/span-factory.ts @@ -8,7 +8,7 @@ import type { IdGenerator } from './id-generator' import type { NetworkSpanOptions } from './network-span' import type { BufferingProcessor, Processor } from './processor' import type { ReadonlySampler } from './sampler' -import type { InternalSpanOptions, ParentContext, Span, SpanOptionSchema, SpanOptions } from './span' +import type { InternalSpanOptions, Span, SpanOptionSchema, SpanOptions } from './span' import { SpanInternal, coreSpanOptionSchema } from './span' import type { SpanContextStorage } from './span-context' import { timeToNumber } from './time' @@ -64,12 +64,10 @@ export class SpanFactory { } startSpan (name: string, options: SpanOptions) { - const safeStartTime = timeToNumber(this.clock, options.startTime) - // if the parentContext option is not set use the current context // if parentContext is explicitly null, or there is no current context, // we are starting a new root span - const parentContext = isParentContext(options.parentContext) || options.parentContext === null + options.parentContext = isParentContext(options.parentContext) || options.parentContext === null ? options.parentContext : this.spanContextStorage.current @@ -78,7 +76,7 @@ export class SpanFactory { attributes.set('bugsnag.span.first_class', options.isFirstClass) } - const span = this.createSpanInternal(name, safeStartTime, parentContext, options.isFirstClass, attributes) + const span = this.createSpanInternal(name, options, attributes) // don't track spans that are started while the app is backgrounded if (this.isInForeground) { @@ -94,15 +92,14 @@ export class SpanFactory { protected createSpanInternal ( name: string, - startTime: number, - parentContext: ParentContext | null | undefined, - isFirstClass: boolean | undefined, + options: SpanOptions, attributes: SpanAttributes) { + const safeStartTime = timeToNumber(this.clock, options.startTime) const spanId = this.idGenerator.generate(64) - const parentSpanId = parentContext ? parentContext.id : undefined - const traceId = parentContext ? parentContext.traceId : this.idGenerator.generate(128) + const parentSpanId = options.parentContext ? options.parentContext.id : undefined + const traceId = options.parentContext ? options.parentContext.traceId : this.idGenerator.generate(128) - return new SpanInternal(spanId, traceId, name, startTime, attributes, this.clock, parentSpanId) + return new SpanInternal(spanId, traceId, name, safeStartTime, attributes, this.clock, parentSpanId) } startNetworkSpan (options: NetworkSpanOptions) { diff --git a/packages/platforms/react-native/lib/span-factory.ts b/packages/platforms/react-native/lib/span-factory.ts index 1fe6e8c59..7ce7902cd 100644 --- a/packages/platforms/react-native/lib/span-factory.ts +++ b/packages/platforms/react-native/lib/span-factory.ts @@ -1,5 +1,5 @@ -import { runSpanEndCallbacks, SpanFactory, SpanInternal } from '@bugsnag/core-performance' -import type { SpanAttributes, ParentContext, SpanOptions } from '@bugsnag/core-performance' +import { runSpanEndCallbacks, SpanFactory, SpanInternal, timeToNumber } from '@bugsnag/core-performance' +import type { SpanAttributes, SpanOptions } from '@bugsnag/core-performance' import type { ReactNativeConfiguration } from './config' import NativeBugsnagPerformance from './native' import type { ReactNativeClock } from './clock' @@ -8,6 +8,10 @@ class NativeSpanInternal extends SpanInternal { public readonly isNativeSpan: boolean = true } +interface ReactNativeSpanOptions extends SpanOptions { + doNotDelegateToNativeSDK?: boolean +} + export class ReactNativeSpanFactory extends SpanFactory { private attachedToNative = false @@ -15,15 +19,20 @@ export class ReactNativeSpanFactory extends SpanFactory { expect(contextStorage.current).toBe(span) }) + it('does not start a native span when doNotDelegateToNativeSDK is true', () => { + spanFactory.onAttach() + + const startTime = clock.now() + const span = spanFactory.startSpan('first class', { startTime, isFirstClass: true, doNotDelegateToNativeSDK: true }) + expect(NativeBugsnagPerformance!.startNativeSpan).not.toHaveBeenCalled() + expect(contextStorage.current).toBe(span) + }) + it('does not start a native span when not attached to native', () => { const startTime = clock.now() const span = spanFactory.startSpan('first class', { startTime, isFirstClass: true }) diff --git a/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts b/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts index c45e1c3dd..8b2e45164 100644 --- a/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts +++ b/packages/plugin-react-native-navigation/lib/react-native-navigation-plugin.ts @@ -82,7 +82,7 @@ class BugsnagPluginReactNativeNavigationPerformance implements Plugin { spanFactory.endSpan(this.currentSpan, DISCARDED) } - const span = spanFactory.startNavigationSpan(currentRoute, { startTime: updateTime }) + const span = spanFactory.startNavigationSpan(currentRoute, { startTime: updateTime, doNotDelegateToNativeSDK: true }) span.setAttribute('bugsnag.navigation.triggered_by', '@bugsnag/plugin-react-navigation-performance') if (this.previousRoute) { diff --git a/packages/plugin-react-navigation/test/create-navigation-container.test.tsx b/packages/plugin-react-navigation/test/create-navigation-container.test.tsx index 3ee532297..adb19d2e4 100644 --- a/packages/plugin-react-navigation/test/create-navigation-container.test.tsx +++ b/packages/plugin-react-navigation/test/create-navigation-container.test.tsx @@ -30,14 +30,14 @@ describe('createNavigationContainer', () => { fireEvent.press(screen.getByText('Go to route 2')) expect(screen.getByText('Route 2')).toBeOnTheScreen() - expect(spanFactory.startSpan).toHaveBeenCalledTimes(1) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]Route 2', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledTimes(1) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('Route 2', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) fireEvent.press(screen.getByText('Go back')) expect(screen.getByText('Route 1')).toBeOnTheScreen() - expect(spanFactory.startSpan).toHaveBeenCalledTimes(2) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]Route 1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledTimes(2) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('Route 1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) }) it('forwards the provided ref to the NavigationContainer', () => { @@ -56,8 +56,8 @@ describe('createNavigationContainer', () => { navigationRef.navigate('Route 2') }) - expect(spanFactory.startSpan).toHaveBeenCalledTimes(1) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]Route 2', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledTimes(1) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('Route 2', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) }) }) diff --git a/packages/plugin-react-navigation/test/navigation-context.test.tsx b/packages/plugin-react-navigation/test/navigation-context.test.tsx index da7226978..d6d4b0365 100644 --- a/packages/plugin-react-navigation/test/navigation-context.test.tsx +++ b/packages/plugin-react-navigation/test/navigation-context.test.tsx @@ -20,11 +20,11 @@ describe('NavigationContextProvider', () => { render() // Initial route should not create a span - expect(spanFactory.startSpan).not.toHaveBeenCalled() + expect(spanFactory.startNavigationSpan).not.toHaveBeenCalled() // Route change should create a navigation span fireEvent.press(screen.getByText('Change to route 1')) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) // Await the navigation span to end jest.advanceTimersByTime(100) @@ -49,8 +49,8 @@ describe('NavigationContextProvider', () => { // Change to a new route but block the navigation span from ending fireEvent.press(screen.getByText('Change to route 1')) fireEvent.press(screen.getByText('Block Navigation')) - expect(spanFactory.startSpan).toHaveBeenCalledTimes(1) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledTimes(1) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) // Navigation span should not end after timeout jest.advanceTimersByTime(100) @@ -58,8 +58,8 @@ describe('NavigationContextProvider', () => { // Change to a second route while the first navigation span is still open fireEvent.press(screen.getByText('Change to route 2')) - expect(spanFactory.startSpan).toHaveBeenCalledTimes(2) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-2', { isFirstClass: true, startTime: 100 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledTimes(2) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-2', { isFirstClass: true, startTime: 100, doNotDelegateToNativeSDK: true }) // End the navigation fireEvent.press(screen.getByText('Unblock Navigation')) @@ -84,7 +84,7 @@ describe('NavigationContextProvider', () => { // Start a navigation fireEvent.press(screen.getByText('Change to route 1')) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) // Prevent navigation from ending fireEvent.press(screen.getByText('Block Navigation')) @@ -111,7 +111,7 @@ describe('NavigationContextProvider', () => { // Start a navigation fireEvent.press(screen.getByText('Change to route 1')) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) // Block navigation from completing fireEvent.press(screen.getByText('Block Navigation')) @@ -138,7 +138,7 @@ describe('NavigationContextProvider', () => { // start navigation fireEvent.press(screen.getByText('Change to route 1')) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-1', { isFirstClass: true, startTime: 0 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-1', { isFirstClass: true, startTime: 0, doNotDelegateToNativeSDK: true }) // lastRenderTime should be set to 2 @@ -158,7 +158,7 @@ describe('NavigationContextProvider', () => { // start second navigation fireEvent.press(screen.getByText('Change to route 2')) - expect(spanFactory.startSpan).toHaveBeenCalledWith('[Navigation]route-2', { isFirstClass: true, startTime: 200 }) + expect(spanFactory.startNavigationSpan).toHaveBeenCalledWith('route-2', { isFirstClass: true, startTime: 200, doNotDelegateToNativeSDK: true }) jest.advanceTimersByTime(100) // check delivered span