Skip to content

Commit

Permalink
update current state in page load and navigation
Browse files Browse the repository at this point in the history
  • Loading branch information
AnastasiiaSvietlova committed Dec 19, 2024
1 parent e1c4db0 commit 752f910
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { getPermittedAttributes } from '../send-page-attributes'
import type { WebVitals } from '../web-vitals'
import { instrumentPageLoadPhaseSpans } from './page-load-phase-spans'
import { defaultRouteResolver } from '../default-routing-provider'
import type { AppState } from '../../../../core/lib/core'

export class FullPageLoadPlugin implements Plugin<BrowserConfiguration> {
private readonly spanFactory: SpanFactory<BrowserConfiguration>
Expand All @@ -14,6 +15,7 @@ export class FullPageLoadPlugin implements Plugin<BrowserConfiguration> {
private readonly onSettle: OnSettle
private readonly webVitals: WebVitals
private readonly performance: PerformanceWithTiming
private readonly setAppState: (appState: AppState) => void

// if the page was backgrounded at any point in the loading process a page
// load span is invalidated as the browser will deprioritise the page
Expand All @@ -26,14 +28,16 @@ export class FullPageLoadPlugin implements Plugin<BrowserConfiguration> {
webVitals: WebVitals,
onSettle: OnSettle,
backgroundingListener: BackgroundingListener,
performance: PerformanceWithTiming
performance: PerformanceWithTiming,
setAppState: (appState: AppState) => void
) {
this.document = document
this.location = location
this.spanFactory = spanFactory
this.webVitals = webVitals
this.onSettle = onSettle
this.performance = performance
this.setAppState = setAppState

backgroundingListener.onStateChange(state => {
if (!this.wasBackgrounded && state === 'in-background') {
Expand Down Expand Up @@ -87,6 +91,7 @@ export class FullPageLoadPlugin implements Plugin<BrowserConfiguration> {

this.webVitals.attachTo(span)
this.spanFactory.endSpan(span, endTime)
this.setAppState("ready")

Check failure on line 94 in packages/platforms/browser/lib/auto-instrumentation/full-page-load-plugin.ts

View workflow job for this annotation

GitHub Actions / linting

Strings must use singlequote
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { BrowserConfiguration } from '../config'
import type { RouteChangeSpanEndOptions, RouteChangeSpanOptions } from '../routing-provider'
import { getPermittedAttributes } from '../send-page-attributes'
import { defaultRouteResolver } from '../default-routing-provider'
import type { AppState } from '../../../../core/lib/core'

// exclude isFirstClass from the route change option schema
const { startTime, parentContext, makeCurrentContext } = coreSpanOptionSchema
Expand All @@ -27,7 +28,8 @@ export class RouteChangePlugin implements Plugin<BrowserConfiguration> {
constructor (
private readonly spanFactory: SpanFactory<BrowserConfiguration>,
private readonly location: Location,
private readonly document: Document
private readonly document: Document,
private readonly setAppState: (appState: AppState) => void
) {}

configure (configuration: InternalConfiguration<BrowserConfiguration>) {
Expand Down Expand Up @@ -117,6 +119,7 @@ export class RouteChangePlugin implements Plugin<BrowserConfiguration> {
span.name = `[RouteChange]${route}`
span.setAttribute('bugsnag.browser.page.route', route)
previousRoute = route
this.setAppState('navigating')

// update the URL attribute as well
if (permittedAttributes.url) {
Expand All @@ -125,6 +128,7 @@ export class RouteChangePlugin implements Plugin<BrowserConfiguration> {
}

this.spanFactory.toPublicApi(span).end(options.endTime)
this.setAppState('ready')
}

} satisfies Span
Expand Down
16 changes: 11 additions & 5 deletions packages/platforms/browser/lib/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ import makeBrowserPersistence from './persistence'
import createResourceAttributesSource from './resource-attributes-source'
import createSpanAttributesSource from './span-attributes-source'
import { WebVitals } from './web-vitals'
import type { AppState } from '../../../core/lib/core'

export let onSettle: OnSettlePlugin
export let DefaultRoutingProvider: ReturnType<typeof createDefaultRoutingProvider>
let BugsnagPerformance: Client<BrowserConfiguration>
let setAppState: (appState: AppState) => void;

Check failure on line 23 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Extra semicolon

if (typeof window === 'undefined' || typeof document === 'undefined') {
onSettle = createNoopOnSettle()
Expand All @@ -40,7 +42,8 @@ if (typeof window === 'undefined' || typeof document === 'undefined') {
xhrRequestTracker,
performance
)
DefaultRoutingProvider = createDefaultRoutingProvider(onSettle, window.location)

Check failure on line 45 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Trailing spaces not allowed
DefaultRoutingProvider = createDefaultRoutingProvider(onSettle, window.location, setAppState)

BugsnagPerformance = createClient({
backgroundingListener,
Expand All @@ -50,7 +53,9 @@ if (typeof window === 'undefined' || typeof document === 'undefined') {
deliveryFactory: createFetchDeliveryFactory(window.fetch, clock, backgroundingListener),
idGenerator,
schema: createSchema(window.location.hostname, new DefaultRoutingProvider()),
plugins: (spanFactory, spanContextStorage) => [
plugins: (spanFactory, spanContextStorage, setAppState) => {
DefaultRoutingProvider.setAppState = setAppState('starting');

Check failure on line 57 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Extra semicolon
return [
onSettle,

Check failure on line 59 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Expected indentation of 8 spaces but found 6
new FullPageLoadPlugin(

Check failure on line 60 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Expected indentation of 8 spaces but found 6
document,

Check failure on line 61 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Expected indentation of 10 spaces but found 8
Expand All @@ -59,14 +64,15 @@ if (typeof window === 'undefined' || typeof document === 'undefined') {
webVitals,

Check failure on line 64 in packages/platforms/browser/lib/browser.ts

View workflow job for this annotation

GitHub Actions / linting

Expected indentation of 10 spaces but found 8
onSettle,
backgroundingListener,
performance
performance,
setAppState
),
// ResourceLoadPlugin should always come after FullPageLoad plugin, as it should use that
// span context as the parent of it's spans
new ResourceLoadPlugin(spanFactory, spanContextStorage, window.PerformanceObserver),
new NetworkRequestPlugin(spanFactory, spanContextStorage, fetchRequestTracker, xhrRequestTracker),
new RouteChangePlugin(spanFactory, window.location, document)
],
new RouteChangePlugin(spanFactory, window.location, document, setAppState)
]},
persistence,
retryQueueFactory: (delivery, retryQueueMaxSize) => new InMemoryQueue(delivery, retryQueueMaxSize)
})
Expand Down
11 changes: 9 additions & 2 deletions packages/platforms/browser/lib/default-routing-provider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { OnSettle } from './on-settle'
import { getAbsoluteUrl } from '@bugsnag/request-tracker-performance'
import type { RouteResolver, RoutingProvider, StartRouteChangeCallback } from './routing-provider'
import { AppState } from '@bugsnag/core-performance/dist/types/core'

export const defaultRouteResolver: RouteResolver = (url: URL) => url.pathname || '/'

Expand All @@ -16,21 +17,25 @@ export const createNoopRoutingProvider = () => {
}
}

export const createDefaultRoutingProvider = (onSettle: OnSettle, location: Location) => {
export const createDefaultRoutingProvider = (onSettle: OnSettle, location: Location, setAppState?: (appState: AppState) => void) => {
return class DefaultRoutingProvider implements RoutingProvider {
resolveRoute: RouteResolver

setAppState?: (appState: AppState) => void

constructor (resolveRoute = defaultRouteResolver) {
this.resolveRoute = resolveRoute
this.setAppState = setAppState
}

listenForRouteChanges (startRouteChangeSpan: StartRouteChangeCallback) {
addEventListener('popstate', (ev) => {
const url = new URL(location.href)
const span = startRouteChangeSpan(url, 'popstate')
this.setAppState?.('navigating')

onSettle((endTime) => {
span.end(endTime)
this.setAppState?.('ready')
})
})

Expand All @@ -41,9 +46,11 @@ export const createDefaultRoutingProvider = (onSettle: OnSettle, location: Locat
if (url) {
const absoluteURL = new URL(getAbsoluteUrl(url.toString(), document.baseURI))
const span = startRouteChangeSpan(absoluteURL, 'pushState')
// this.setAppState?.('navigating')

onSettle((endTime) => {
span.end(endTime)
// this.setAppState?.('ready')
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React from 'react'
import type { AppRegistry, WrapperComponentProvider } from 'react-native'
import type { ReactNativeConfiguration } from '../config'
import { createAppStartSpan } from '../create-app-start-span'
import type { AppState } from '../../../../core/lib/core'

interface WrapperProps {
children: ReactNode
Expand All @@ -22,17 +23,20 @@ export class AppStartPlugin implements Plugin<ReactNativeConfiguration> {
private readonly spanFactory: SpanFactory<ReactNativeConfiguration>
private readonly clock: Clock
private readonly appRegistry: typeof AppRegistry
private readonly setAppState: (appState: AppState) => void

constructor (
appStartTime: number,
spanFactory: SpanFactory<ReactNativeConfiguration>,
clock: Clock,
appRegistry: typeof AppRegistry
appRegistry: typeof AppRegistry,
setAppState: (appState: AppState) => void
) {
this.appStartTime = appStartTime
this.spanFactory = spanFactory
this.clock = clock
this.appRegistry = appRegistry
this.setAppState = setAppState
}

configure (configuration: InternalConfiguration<ReactNativeConfiguration>) {
Expand All @@ -44,6 +48,7 @@ export class AppStartPlugin implements Plugin<ReactNativeConfiguration> {
React.useEffect(() => {
if (appStartSpan.isValid()) {
this.spanFactory.endSpan(appStartSpan, this.clock.now())
this.setAppState('ready')
}
}, [])

Expand Down
4 changes: 2 additions & 2 deletions packages/platforms/react-native/lib/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ const BugsnagPerformance = createClient({
deliveryFactory,
idGenerator,
persistence,
plugins: (spanFactory, spanContextStorage) => [
new AppStartPlugin(appStartTime, spanFactory, clock, AppRegistry),
plugins: (spanFactory, spanContextStorage, setAppState) => [
new AppStartPlugin(appStartTime, spanFactory, clock, AppRegistry, setAppState),
new NetworkRequestPlugin(spanFactory, spanContextStorage, xhrRequestTracker)
],
resourceAttributesSource,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import createClock from '../../lib/clock'
import { AppStartPlugin } from '../../lib/auto-instrumentation/app-start-plugin'
import type { ReactNativeConfiguration } from '../../lib/config'
import type { AppRegistry } from 'react-native'
import type { AppState } from '../../../../core/lib/core'

describe('app start plugin', () => {
let spanFactory: MockSpanFactory<ReactNativeConfiguration>
let clock: Clock
let appRegistry: typeof AppRegistry
let setAppState: (appState: AppState) => void

beforeEach(() => {
spanFactory = new MockSpanFactory()
Expand All @@ -20,7 +22,7 @@ describe('app start plugin', () => {

it('starts an app start span when autoInstrumentAppStarts is true', () => {
const appStartTime = 1234
const plugin = new AppStartPlugin(appStartTime, spanFactory, clock, appRegistry)
const plugin = new AppStartPlugin(appStartTime, spanFactory, clock, appRegistry, setAppState)

plugin.configure(createConfiguration<ReactNativeConfiguration>({ autoInstrumentAppStarts: true }))

Expand All @@ -34,7 +36,7 @@ describe('app start plugin', () => {
})

it('does not start an app start span when autoInstrumentAppStarts is false', () => {
const plugin = new AppStartPlugin(1234, spanFactory, clock, appRegistry)
const plugin = new AppStartPlugin(1234, spanFactory, clock, appRegistry, setAppState)

plugin.configure(createConfiguration<ReactNativeConfiguration>({ autoInstrumentAppStarts: false }))

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Plugin, SpanFactory, SpanInternal } from '@bugsnag/core-performance'
import type { ReactNativeConfiguration } from '@bugsnag/react-native-performance'
import type { NavigationDelegate } from 'react-native-navigation/lib/dist/src/NavigationDelegate'
import type { AppState } from '../../core/lib/core'

import { createNavigationSpan } from '@bugsnag/react-native-performance'

Expand All @@ -18,9 +19,11 @@ class BugsnagPluginReactNativeNavigationPerformance implements Plugin<ReactNativ
private componentsWaiting = 0
private spanFactory?: SpanFactory<ReactNativeConfiguration>
private previousRoute?: string
private setAppState: (appState: AppState) => void

constructor (Navigation: NavigationDelegate) {
constructor (Navigation: NavigationDelegate, setAppState: (appState: AppState) => void) {
this.Navigation = Navigation
this.setAppState = setAppState
}

private clearActiveSpan () {
Expand All @@ -35,6 +38,7 @@ class BugsnagPluginReactNativeNavigationPerformance implements Plugin<ReactNativ
this.currentNavigationSpan.setAttribute('bugsnag.navigation.ended_by', endedBy)
this.spanFactory.endSpan(this.currentNavigationSpan, endTime)
this.clearActiveSpan()
this.setAppState('ready')
}
}

Expand Down Expand Up @@ -89,6 +93,7 @@ class BugsnagPluginReactNativeNavigationPerformance implements Plugin<ReactNativ

if (this.previousRoute) {
this.currentNavigationSpan.setAttribute('bugsnag.navigation.previous_route', this.previousRoute)
this.setAppState('navigating')
}

this.previousRoute = routeName
Expand Down
7 changes: 6 additions & 1 deletion packages/plugin-react-navigation/lib/navigation-context.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { SpanFactory, SpanInternal } from '@bugsnag/core-performance'
import type { ReactNativeConfiguration } from '@bugsnag/react-native-performance'
import type { PropsWithChildren } from 'react'
import type { AppState } from '../../core/lib/core'

import React from 'react'
import { createNavigationSpan } from '@bugsnag/react-native-performance'
Expand All @@ -14,6 +15,7 @@ export const NavigationContext = React.createContext({
interface Props extends PropsWithChildren {
currentRoute?: string
spanFactory: SpanFactory<ReactNativeConfiguration>
setAppState: (appState: AppState) => void
}

type EndCondition = 'condition' | 'mount' | 'unmount' | 'immediate'
Expand Down Expand Up @@ -54,7 +56,7 @@ export class NavigationContextProvider extends React.Component<Props> {
this.currentSpan.setAttribute('bugsnag.navigation.ended_by', this.endCondition)
const endTime = Math.max(this.lastRenderTime, triggerNavigationEndTime)
this.props.spanFactory.endSpan(this.currentSpan, endTime)

this.props.setAppState('ready')
this.currentSpan = undefined
this.lastRenderTime = 0
}
Expand All @@ -71,13 +73,15 @@ export class NavigationContextProvider extends React.Component<Props> {
// invalid time to cause it to be discarded from the context stack.
if (this.currentSpan) {
spanFactory.endSpan(this.currentSpan, DISCARDED)
this.props.setAppState('ready')
}

const span = createNavigationSpan(spanFactory, currentRoute, { startTime: updateTime })
span.setAttribute('bugsnag.navigation.triggered_by', '@bugsnag/plugin-react-navigation-performance')

if (this.previousRoute) {
span.setAttribute('bugsnag.navigation.previous_route', this.previousRoute)
this.props.setAppState('navigating')
}

this.currentSpan = span
Expand All @@ -86,6 +90,7 @@ export class NavigationContextProvider extends React.Component<Props> {

setTimeout(() => {
this.triggerNavigationEnd()
this.props.setAppState('ready')
})
}
}
Expand Down

0 comments on commit 752f910

Please sign in to comment.