[dashboard] fix time_to_data does not capture entire client-side rend…

…ering (elastic#200640) Closes elastic#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 <[email protected]>
nreese · Nov 25, 2024 · cdeb1e9 · cdeb1e9
1 parent b6586a9
commit cdeb1e9
Show file tree

Hide file tree

Showing 7 changed files with 179 additions and 29 deletions.
diff --git 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
@@ -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<boolean>;
+}
+
+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
@@ -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
@@ -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<PhaseEvent | undefined>(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
@@ -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<SerializedState>) => void;
 }) => {
   const cleanupFunction = useRef<(() => void) | null>(null);
-  const firstLoadCompleteTime = useRef<number | null>(null);
+  const phaseTracker = useRef(new PhaseTracker());
 
   const componentPromise = useMemo(
     () => {
       const uuid = maybeId ?? generateId();
 
-      /**
-       * Phase tracking instrumentation for telemetry
-       */
-      const phase$ = new BehaviorSubject<PhaseEvent | undefined>(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<RuntimeState>;
@@ -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
@@ -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<HasEditCapabilities> &
   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
@@ -179,6 +179,7 @@ export const getVisualizeEmbeddableFactory: (deps: {
         defaultPanelTitle,
         dataLoading: dataLoading$,
         dataViews: new BehaviorSubject<DataView[] | undefined>(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.