From cdeb1e984425e4ad40d5125e4d224e01f1a6936e Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 25 Nov 2024 10:51:05 -0700 Subject: [PATCH] [dashboard] fix time_to_data does not capture entire client-side rendering (#200640) Closes https://github.com/elastic/kibana/issues/194489 PR adds new `PublishesRendered` interface. Embeddables can implement this interface to provide feedback when rendering is complete. PR updates ReactEmbeddableRender phase tracking logic to include check for `PublishesRendered` value when interface is implemented. --------- Co-authored-by: Elastic Machine --- .../presentation_publishing/index.ts | 1 + .../interfaces/publishes_rendered.ts | 20 ++++ .../phase_tracker.test.ts | 100 ++++++++++++++++++ .../react_embeddable_system/phase_tracker.ts | 48 +++++++++ .../react_embeddable_renderer.tsx | 35 ++---- .../visualizations/public/embeddable/types.ts | 2 + .../embeddable/visualize_embeddable.tsx | 2 +- 7 files changed, 179 insertions(+), 29 deletions(-) create mode 100644 packages/presentation/presentation_publishing/interfaces/publishes_rendered.ts create mode 100644 src/plugins/embeddable/public/react_embeddable_system/phase_tracker.test.ts create mode 100644 src/plugins/embeddable/public/react_embeddable_system/phase_tracker.ts diff --git a/packages/presentation/presentation_publishing/index.ts b/packages/presentation/presentation_publishing/index.ts index 2b96c6d353eee..d3b9c44f6fd36 100644 --- a/packages/presentation/presentation_publishing/index.ts +++ b/packages/presentation/presentation_publishing/index.ts @@ -108,6 +108,7 @@ export { type PhaseEventType, type PublishesPhaseEvents, } from './interfaces/publishes_phase_events'; +export { apiPublishesRendered, type PublishesRendered } from './interfaces/publishes_rendered'; export { apiPublishesSavedObjectId, type PublishesSavedObjectId, diff --git a/packages/presentation/presentation_publishing/interfaces/publishes_rendered.ts b/packages/presentation/presentation_publishing/interfaces/publishes_rendered.ts new file mode 100644 index 0000000000000..9acbd8c3f258f --- /dev/null +++ b/packages/presentation/presentation_publishing/interfaces/publishes_rendered.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { PublishingSubject } from '../publishing_subject'; + +export interface PublishesRendered { + rendered$: PublishingSubject; +} + +export const apiPublishesRendered = ( + unknownApi: null | unknown +): unknownApi is PublishesRendered => { + return Boolean(unknownApi && (unknownApi as PublishesRendered)?.rendered$ !== undefined); +}; diff --git a/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.test.ts b/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.test.ts new file mode 100644 index 0000000000000..700c90f08ce5b --- /dev/null +++ b/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.test.ts @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { BehaviorSubject, skip } from 'rxjs'; +import { PhaseTracker } from './phase_tracker'; + +describe('PhaseTracker', () => { + describe('api does not implement PublishesDataLoading or PublishesRendered', () => { + test(`should emit 'rendered' event`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('rendered'); + done(); + }); + phaseTracker.trackPhaseEvents('1', {}); + }); + }); + + describe('api implements PublishesDataLoading', () => { + test(`should emit 'loading' event when dataLoading is true`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('loading'); + done(); + }); + phaseTracker.trackPhaseEvents('1', { dataLoading: new BehaviorSubject(true) }); + }); + + test(`should emit 'rendered' event when dataLoading is false`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('rendered'); + done(); + }); + phaseTracker.trackPhaseEvents('1', { dataLoading: new BehaviorSubject(false) }); + }); + }); + + describe('api implements PublishesDataLoading and PublishesRendered', () => { + test(`should emit 'loading' event when dataLoading is true`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('loading'); + done(); + }); + phaseTracker.trackPhaseEvents('1', { + dataLoading: new BehaviorSubject(true), + rendered$: new BehaviorSubject(false), + }); + }); + + test(`should emit 'loading' event when dataLoading is false but rendered is false`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('loading'); + done(); + }); + phaseTracker.trackPhaseEvents('1', { + dataLoading: new BehaviorSubject(false), + rendered$: new BehaviorSubject(false), + }); + }); + + test(`should emit 'rendered' event only when rendered is true`, (done) => { + const phaseTracker = new PhaseTracker(); + phaseTracker + .getPhase$() + .pipe(skip(1)) + .subscribe((phaseEvent) => { + expect(phaseEvent?.status).toBe('rendered'); + done(); + }); + phaseTracker.trackPhaseEvents('1', { + dataLoading: new BehaviorSubject(false), + rendered$: new BehaviorSubject(true), + }); + }); + }); +}); diff --git a/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.ts b/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.ts new file mode 100644 index 0000000000000..037599ab646cc --- /dev/null +++ b/src/plugins/embeddable/public/react_embeddable_system/phase_tracker.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import { + PhaseEvent, + apiPublishesDataLoading, + apiPublishesRendered, +} from '@kbn/presentation-publishing'; +import { BehaviorSubject, Subscription, combineLatest } from 'rxjs'; + +export class PhaseTracker { + private firstLoadCompleteTime: number | undefined; + private embeddableStartTime = performance.now(); + private subscriptions = new Subscription(); + private phase$ = new BehaviorSubject(undefined); + + getPhase$() { + return this.phase$; + } + + public trackPhaseEvents(uuid: string, api: unknown) { + const dataLoading$ = apiPublishesDataLoading(api) + ? api.dataLoading + : new BehaviorSubject(false); + const rendered$ = apiPublishesRendered(api) ? api.rendered$ : new BehaviorSubject(true); + + this.subscriptions.add( + combineLatest([dataLoading$, rendered$]).subscribe(([dataLoading, rendered]) => { + if (!this.firstLoadCompleteTime) { + this.firstLoadCompleteTime = performance.now(); + } + const duration = this.firstLoadCompleteTime - this.embeddableStartTime; + const status = dataLoading || !rendered ? 'loading' : 'rendered'; + this.phase$.next({ id: uuid, status, timeToEvent: duration }); + }) + ); + } + + public cleanup() { + this.subscriptions.unsubscribe(); + } +} diff --git a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx index c3dc06e198cd8..7cf9d9f203fc3 100644 --- a/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx +++ b/src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx @@ -16,12 +16,7 @@ import { SerializedPanelState, } from '@kbn/presentation-containers'; import { PresentationPanel, PresentationPanelProps } from '@kbn/presentation-panel-plugin/public'; -import { - apiPublishesDataLoading, - ComparatorDefinition, - PhaseEvent, - StateComparators, -} from '@kbn/presentation-publishing'; +import { ComparatorDefinition, StateComparators } from '@kbn/presentation-publishing'; import React, { useEffect, useImperativeHandle, useMemo, useRef } from 'react'; import { BehaviorSubject, combineLatest, debounceTime, skip, Subscription, switchMap } from 'rxjs'; import { v4 as generateId } from 'uuid'; @@ -31,6 +26,7 @@ import { DefaultEmbeddableApi, SetReactEmbeddableApiRegistration, } from './types'; +import { PhaseTracker } from './phase_tracker'; const ON_STATE_CHANGE_DEBOUNCE = 100; @@ -78,25 +74,12 @@ export const ReactEmbeddableRenderer = < onAnyStateChange?: (state: SerializedPanelState) => void; }) => { const cleanupFunction = useRef<(() => void) | null>(null); - const firstLoadCompleteTime = useRef(null); + const phaseTracker = useRef(new PhaseTracker()); const componentPromise = useMemo( () => { const uuid = maybeId ?? generateId(); - /** - * Phase tracking instrumentation for telemetry - */ - const phase$ = new BehaviorSubject(undefined); - const embeddableStartTime = performance.now(); - const reportPhaseChange = (loading: boolean) => { - if (firstLoadCompleteTime.current === null) { - firstLoadCompleteTime.current = performance.now(); - } - const duration = firstLoadCompleteTime.current - embeddableStartTime; - phase$.next({ id: uuid, status: loading ? 'loading' : 'rendered', timeToEvent: duration }); - }; - /** * Build the embeddable promise */ @@ -126,7 +109,7 @@ export const ReactEmbeddableRenderer = < return { ...apiRegistration, uuid, - phase$, + phase$: phaseTracker.current.getPhase$(), parentApi, hasLockedHoverActions$, lockHoverActions: (lock: boolean) => { @@ -186,6 +169,7 @@ export const ReactEmbeddableRenderer = < cleanupFunction.current = () => { subscriptions.unsubscribe(); + phaseTracker.current.cleanup(); unsavedChanges.cleanup(); }; return fullApi as Api & HasSnapshottableState; @@ -200,13 +184,8 @@ export const ReactEmbeddableRenderer = < lastSavedRuntimeState ); - if (apiPublishesDataLoading(api)) { - subscriptions.add( - api.dataLoading.subscribe((loading) => reportPhaseChange(Boolean(loading))) - ); - } else { - reportPhaseChange(false); - } + phaseTracker.current.trackPhaseEvents(uuid, api); + return { api, Component }; }; diff --git a/src/plugins/visualizations/public/embeddable/types.ts b/src/plugins/visualizations/public/embeddable/types.ts index f4215d923e1d5..80e7e2d9179e8 100644 --- a/src/plugins/visualizations/public/embeddable/types.ts +++ b/src/plugins/visualizations/public/embeddable/types.ts @@ -17,6 +17,7 @@ import { HasSupportedTriggers, PublishesDataLoading, PublishesDataViews, + PublishesRendered, PublishesTimeRange, SerializedTimeRange, SerializedTitles, @@ -92,6 +93,7 @@ export const isVisualizeRuntimeState = (state: unknown): state is VisualizeRunti export type VisualizeApi = Partial & PublishesDataViews & PublishesDataLoading & + PublishesRendered & HasVisualizeConfig & HasInspectorAdapters & HasSupportedTriggers & diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.tsx b/src/plugins/visualizations/public/embeddable/visualize_embeddable.tsx index 7b48521265d6f..4993c0168313a 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable.tsx +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable.tsx @@ -179,6 +179,7 @@ export const getVisualizeEmbeddableFactory: (deps: { defaultPanelTitle, dataLoading: dataLoading$, dataViews: new BehaviorSubject(initialDataViews), + rendered$: hasRendered$, supportedTriggers: () => [ ACTION_CONVERT_TO_LENS, APPLY_FILTER_TRIGGER, @@ -397,7 +398,6 @@ export const getVisualizeEmbeddableFactory: (deps: { if (hasRendered$.getValue() === true) return; hasRendered$.next(true); - hasRendered$.complete(); }, onEvent: async (event) => { // Visualize doesn't respond to sizing events, so ignore.