[Canvas] Reduce report generation time by re-using headless browser page in background (#63301) (#65147)

* Rough first pass at reusing page for multiple links in report generation * Some adjustments to handling the events coming from CDP * Add new data-share-page selector for jobs with multiple urls * Cleanup * PR feedback * Adding tests for Canvas export app and multi user observable jobs * Adding a short blurb describing the data-shared-page attribute requirement * PR feedback Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
2025-04-23 17:28:26 -04:00 · 2020-05-05 07:36:30 -05:00 · 2020-05-05 07:36:30 -05:00 · 4926fc6cbf
commit 4926fc6cbf
parent 6297981519
11 changed files with 355 additions and 113 deletions
--- a/docs/user/reporting/development/pdf-integration.asciidoc
+++ b/docs/user/reporting/development/pdf-integration.asciidoc
@ -63,3 +63,5 @@ If there are multiple visualizations, the `data-shared-items-count` attribute sh
 many Visualizations to look for. Reporting will look at every element with the `data-shared-item` attribute and use the corresponding
 `data-render-complete` attribute and `renderComplete` events to listen for rendering to complete. When rendering is complete for a visualization
 the `data-render-complete` attribute should be set to "true" and it should dispatch a custom DOM `renderComplete` event.
+
+If the reporting job uses multiple URLs, before looking for any of the `data-shared-item` or `data-shared-items-count` attributes, it waits for a `data-shared-page` attribute that specifies which page is being loaded.
--- a/x-pack/legacy/plugins/canvas/public/apps/export/export/tests/snapshots/export_app.test.tsx.snap
+++ b/x-pack/legacy/plugins/canvas/public/apps/export/export/tests/snapshots/export_app.test.tsx.snap
@ -0,0 +1,141 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<ExportApp /> renders as expected 1`] = `
+<ExportApp
+  initializeWorkpad={[Function]}
+  selectedPageIndex={0}
+  workpad={
+    Object {
+      "css": "",
+      "id": "my-workpad-abcd",
+      "pages": Array [
+        Object {
+          "elements": Array [
+            0,
+            1,
+            2,
+          ],
+        },
+        Object {
+          "elements": Array [
+            3,
+            4,
+            5,
+            6,
+          ],
+        },
+      ],
+    }
+  }
+>
+  <div
+    className="canvasExport"
+    data-shared-page={1}
+  >
+    <div
+      className="canvasExport__stage"
+    >
+      <div
+        className="canvasLayout__stageHeader"
+      >
+        <Link
+          name="loadWorkpad"
+          params={
+            Object {
+              "id": "my-workpad-abcd",
+            }
+          }
+        >
+          <div>
+            Link
+          </div>
+        </Link>
+      </div>
+      <div
+        className="canvasExport__stageContent"
+        data-shared-items-count={3}
+      >
+        <WorkpadPage
+          isSelected={true}
+          registerLayout={[Function]}
+          unregisterLayout={[Function]}
+        >
+          <div>
+            Page
+          </div>
+        </WorkpadPage>
+      </div>
+    </div>
+  </div>
+</ExportApp>
+`;
+
+exports[`<ExportApp /> renders as expected 2`] = `
+<ExportApp
+  initializeWorkpad={[Function]}
+  selectedPageIndex={1}
+  workpad={
+    Object {
+      "css": "",
+      "id": "my-workpad-abcd",
+      "pages": Array [
+        Object {
+          "elements": Array [
+            0,
+            1,
+            2,
+          ],
+        },
+        Object {
+          "elements": Array [
+            3,
+            4,
+            5,
+            6,
+          ],
+        },
+      ],
+    }
+  }
+>
+  <div
+    className="canvasExport"
+    data-shared-page={2}
+  >
+    <div
+      className="canvasExport__stage"
+    >
+      <div
+        className="canvasLayout__stageHeader"
+      >
+        <Link
+          name="loadWorkpad"
+          params={
+            Object {
+              "id": "my-workpad-abcd",
+            }
+          }
+        >
+          <div>
+            Link
+          </div>
+        </Link>
+      </div>
+      <div
+        className="canvasExport__stageContent"
+        data-shared-items-count={4}
+      >
+        <WorkpadPage
+          isSelected={true}
+          registerLayout={[Function]}
+          unregisterLayout={[Function]}
+        >
+          <div>
+            Page
+          </div>
+        </WorkpadPage>
+      </div>
+    </div>
+  </div>
+</ExportApp>
+`;
--- a/x-pack/legacy/plugins/canvas/public/apps/export/export/tests/export_app.test.tsx
+++ b/x-pack/legacy/plugins/canvas/public/apps/export/export/tests/export_app.test.tsx
@ -0,0 +1,49 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { mount } from 'enzyme';
+// @ts-ignore untyped local
+import { ExportApp } from '../export_app';
+
+jest.mock('style-it', () => ({
+  it: (css: string, Component: any) => Component,
+}));
+
+jest.mock('../../../../components/workpad_page', () => ({
+  WorkpadPage: (props: any) => <div>Page</div>,
+}));
+
+jest.mock('../../../../components/link', () => ({
+  Link: (props: any) => <div>Link</div>,
+}));
+
+describe('<ExportApp />', () => {
+  test('renders as expected', () => {
+    const sampleWorkpad = {
+      id: 'my-workpad-abcd',
+      css: '',
+      pages: [
+        {
+          elements: [0, 1, 2],
+        },
+        {
+          elements: [3, 4, 5, 6],
+        },
+      ],
+    };
+
+    const page1 = mount(
+      <ExportApp workpad={sampleWorkpad} selectedPageIndex={0} initializeWorkpad={() => {}} />
+    );
+    expect(page1).toMatchSnapshot();
+
+    const page2 = mount(
+      <ExportApp workpad={sampleWorkpad} selectedPageIndex={1} initializeWorkpad={() => {}} />
+    );
+    expect(page2).toMatchSnapshot();
+  });
+});
--- a/x-pack/legacy/plugins/canvas/public/apps/export/export/export_app.js
+++ b/x-pack/legacy/plugins/canvas/public/apps/export/export/export_app.js
@ -16,7 +16,7 @@ export class ExportApp extends React.PureComponent {
      id: PropTypes.string.isRequired,
      pages: PropTypes.array.isRequired,
    }).isRequired,
-    selectedPageId: PropTypes.string.isRequired,
+    selectedPageIndex: PropTypes.number.isRequired,
    initializeWorkpad: PropTypes.func.isRequired,
  };

@ -25,13 +25,13 @@ export class ExportApp extends React.PureComponent {
  }

  render() {
-    const { workpad, selectedPageId } = this.props;
+    const { workpad, selectedPageIndex } = this.props;
    const { pages, height, width } = workpad;
-    const activePage = pages.find(page => page.id === selectedPageId);
+    const activePage = pages[selectedPageIndex];
    const pageElementCount = activePage.elements.length;

    return (
-      <div className="canvasExport">
+      <div className="canvasExport" data-shared-page={selectedPageIndex + 1}>
        <div className="canvasExport__stage">
          <div className="canvasLayout__stageHeader">
            <Link name="loadWorkpad" params={{ id: this.props.workpad.id }}>
--- a/x-pack/legacy/plugins/canvas/public/apps/export/export/index.js
+++ b/x-pack/legacy/plugins/canvas/public/apps/export/export/index.js
@ -7,13 +7,13 @@
 import { connect } from 'react-redux';
 import { compose, branch, renderComponent } from 'recompose';
 import { initializeWorkpad } from '../../../state/actions/workpad';
-import { getWorkpad, getSelectedPage } from '../../../state/selectors/workpad';
+import { getWorkpad, getSelectedPageIndex } from '../../../state/selectors/workpad';
 import { LoadWorkpad } from './load_workpad';
 import { ExportApp as Component } from './export_app';

 const mapStateToProps = state => ({
  workpad: getWorkpad(state),
-  selectedPageId: getSelectedPage(state),
+  selectedPageIndex: getSelectedPageIndex(state),
 });

 const mapDispatchToProps = dispatch => ({
--- a/x-pack/legacy/plugins/reporting/export_types/common/constants.ts
+++ b/x-pack/legacy/plugins/reporting/export_types/common/constants.ts
@ -9,4 +9,4 @@ export const LayoutTypes = {
  PRINT: 'print',
 };

-export const PAGELOAD_SELECTOR = '.application';
+export const DEFAULT_PAGELOAD_SELECTOR = '.application';
--- a/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/observable.test.ts
+++ b/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/observable.test.ts
@ -98,9 +98,12 @@ describe('Screenshot Observable Pipeline', () => {
      return Promise.resolve(`allyourBase64 screenshots`);
    });

+    const mockOpen = jest.fn();
+
    // mocks
    mockBrowserDriverFactory = await createMockBrowserDriverFactory(logger, {
      screenshot: mockScreenshot,
+      open: mockOpen,
    });

    // test
@ -179,6 +182,15 @@ describe('Screenshot Observable Pipeline', () => {
        },
      ]
    `);
+
+    // ensures the correct selectors are waited on for multi URL jobs
+    expect(mockOpen.mock.calls.length).toBe(2);
+
+    const firstSelector = mockOpen.mock.calls[0][1].waitForSelector;
+    expect(firstSelector).toBe('.application');
+
+    const secondSelector = mockOpen.mock.calls[1][1].waitForSelector;
+    expect(secondSelector).toBe('[data-shared-page="2"]');
  });

  describe('error handling', () => {
--- a/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/observable.ts
+++ b/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/observable.ts
@ -7,6 +7,7 @@
 import * as Rx from 'rxjs';
 import { catchError, concatMap, first, mergeMap, take, takeUntil, toArray } from 'rxjs/operators';
 import { CaptureConfig } from '../../../../server/types';
+import { DEFAULT_PAGELOAD_SELECTOR } from '../../constants';
 import { HeadlessChromiumDriverFactory } from '../../../../types';
 import { getElementPositionAndAttributes } from './get_element_position_data';
 import { getNumberOfItems } from './get_number_of_items';
@ -44,13 +45,29 @@ export function screenshotsObservableFactory(
      { viewport: layout.getBrowserViewport(), browserTimezone },
      logger
    );
-    return Rx.from(urls).pipe(
-      concatMap(url => {
-        return create$.pipe(
-          mergeMap(({ driver, exit$ }) => {
+
+    return create$.pipe(
+      mergeMap(({ driver, exit$ }) => {
+        return Rx.from(urls).pipe(
+          concatMap((url, index) => {
            const setup$: Rx.Observable<ScreenSetupData> = Rx.of(1).pipe(
              takeUntil(exit$),
-              mergeMap(() => openUrl(captureConfig, driver, url, conditionalHeaders, logger)),
+              mergeMap(() => {
+                // If we're moving to another page in the app, we'll want to wait for the app to tell us
+                // it's loaded the next page.
+                const page = index + 1;
+                const pageLoadSelector =
+                  page > 1 ? `[data-shared-page="${page}"]` : DEFAULT_PAGELOAD_SELECTOR;
+
+                return openUrl(
+                  captureConfig,
+                  driver,
+                  url,
+                  pageLoadSelector,
+                  conditionalHeaders,
+                  logger
+                );
+              }),
              mergeMap(() => getNumberOfItems(captureConfig, driver, layout, logger)),
              mergeMap(async itemsCount => {
                const viewport = layout.getViewport(itemsCount) || getDefaultViewPort();
@ -104,11 +121,11 @@ export function screenshotsObservableFactory(
              )
            );
          }),
-          first()
+          take(urls.length),
+          toArray()
        );
      }),
-      take(urls.length),
-      toArray()
+      first()
    );
  };
 }
--- a/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/open_url.ts
+++ b/x-pack/legacy/plugins/reporting/export_types/common/lib/screenshots/open_url.ts
@ -9,12 +9,12 @@ import { HeadlessChromiumDriver as HeadlessBrowser } from '../../../../server/br
 import { LevelLogger } from '../../../../server/lib';
 import { CaptureConfig } from '../../../../server/types';
 import { ConditionalHeaders } from '../../../../types';
-import { PAGELOAD_SELECTOR } from '../../constants';

 export const openUrl = async (
  captureConfig: CaptureConfig,
  browser: HeadlessBrowser,
  url: string,
+  pageLoadSelector: string,
  conditionalHeaders: ConditionalHeaders,
  logger: LevelLogger
 ): Promise<void> => {
@ -23,7 +23,7 @@ export const openUrl = async (
      url,
      {
        conditionalHeaders,
-        waitForSelector: PAGELOAD_SELECTOR,
+        waitForSelector: pageLoadSelector,
        timeout: captureConfig.timeouts.openUrl,
      },
      logger
--- a/x-pack/legacy/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts
+++ b/x-pack/legacy/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts
@ -7,7 +7,7 @@
 import { i18n } from '@kbn/i18n';
 import { map, trunc } from 'lodash';
 import open from 'opn';
-import { ElementHandle, EvaluateFn, Page, SerializableOrJSHandle } from 'puppeteer';
+import { ElementHandle, EvaluateFn, Page, SerializableOrJSHandle, Response } from 'puppeteer';
 import { parse as parseUrl } from 'url';
 import { ViewZoomWidthHeight } from '../../../../export_types/common/layouts/layout';
 import { LevelLogger } from '../../../../server/lib';
@ -45,6 +45,9 @@ export class HeadlessChromiumDriver {
  private readonly inspect: boolean;
  private readonly networkPolicy: NetworkPolicy;

+  private listenersAttached = false;
+  private interceptedCount = 0;
+
  constructor(page: Page, { inspect, networkPolicy }: ChromiumDriverOptions) {
    this.page = page;
    this.inspect = inspect;
@ -76,103 +79,13 @@ export class HeadlessChromiumDriver {
    logger: LevelLogger
  ): Promise<void> {
    logger.info(`opening url ${url}`);
-    // @ts-ignore
-    const client = this.page._client;
-    let interceptedCount = 0;
+
+    // Reset intercepted request count
+    this.interceptedCount = 0;

    await this.page.setRequestInterception(true);

-    // We have to reach into the Chrome Devtools Protocol to apply headers as using
-    // puppeteer's API will cause map tile requests to hang indefinitely:
-    //    https://github.com/puppeteer/puppeteer/issues/5003
-    // Docs on this client/protocol can be found here:
-    //    https://chromedevtools.github.io/devtools-protocol/tot/Fetch
-    client.on('Fetch.requestPaused', async (interceptedRequest: InterceptedRequest) => {
-      const {
-        requestId,
-        request: { url: interceptedUrl },
-      } = interceptedRequest;
-      const allowed = !interceptedUrl.startsWith('file://');
-      const isData = interceptedUrl.startsWith('data:');
-
-      // We should never ever let file protocol requests go through
-      if (!allowed || !this.allowRequest(interceptedUrl)) {
-        logger.error(`Got bad URL: "${interceptedUrl}", closing browser.`);
-        await client.send('Fetch.failRequest', {
-          errorReason: 'Aborted',
-          requestId,
-        });
-        this.page.browser().close();
-        throw new Error(
-          i18n.translate('xpack.reporting.chromiumDriver.disallowedOutgoingUrl', {
-            defaultMessage: `Received disallowed outgoing URL: "{interceptedUrl}", exiting`,
-            values: { interceptedUrl },
-          })
-        );
-      }
-
-      if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedUrl)) {
-        logger.debug(`Using custom headers for ${interceptedUrl}`);
-        const headers = map(
-          {
-            ...interceptedRequest.request.headers,
-            ...conditionalHeaders.headers,
-          },
-          (value, name) => ({
-            name,
-            value,
-          })
-        );
-
-        try {
-          await client.send('Fetch.continueRequest', {
-            requestId,
-            headers,
-          });
-        } catch (err) {
-          logger.error(
-            i18n.translate('xpack.reporting.chromiumDriver.failedToCompleteRequestUsingHeaders', {
-              defaultMessage: 'Failed to complete a request using headers: {error}',
-              values: { error: err },
-            })
-          );
-        }
-      } else {
-        const loggedUrl = isData ? this.truncateUrl(interceptedUrl) : interceptedUrl;
-        logger.debug(`No custom headers for ${loggedUrl}`);
-        try {
-          await client.send('Fetch.continueRequest', { requestId });
-        } catch (err) {
-          logger.error(
-            i18n.translate('xpack.reporting.chromiumDriver.failedToCompleteRequest', {
-              defaultMessage: 'Failed to complete a request: {error}',
-              values: { error: err },
-            })
-          );
-        }
-      }
-      interceptedCount = interceptedCount + (isData ? 0 : 1);
-    });
-
-    // Even though 3xx redirects go through our request
-    // handler, we should probably inspect responses just to
-    // avoid being bamboozled by some malicious request
-    this.page.on('response', interceptedResponse => {
-      const interceptedUrl = interceptedResponse.url();
-      const allowed = !interceptedUrl.startsWith('file://');
-
-      if (!interceptedResponse.ok()) {
-        logger.warn(
-          `Chromium received a non-OK response (${interceptedResponse.status()}) for request ${interceptedUrl}`
-        );
-      }
-
-      if (!allowed || !this.allowRequest(interceptedUrl)) {
-        logger.error(`Got disallowed URL "${interceptedUrl}", closing browser.`);
-        this.page.browser().close();
-        throw new Error(`Received disallowed URL in response: ${interceptedUrl}`);
-      }
-    });
+    this.registerListeners(conditionalHeaders, logger);

    await this.page.goto(url, { waitUntil: 'domcontentloaded' });

@ -186,7 +99,7 @@ export class HeadlessChromiumDriver {
      { context: 'waiting for page load selector' },
      logger
    );
-    logger.info(`handled ${interceptedCount} page requests`);
+    logger.info(`handled ${this.interceptedCount} page requests`);
  }

  public async screenshot(elementPosition: ElementPosition): Promise<string> {
@ -272,6 +185,111 @@ export class HeadlessChromiumDriver {
    });
  }

+  private registerListeners(conditionalHeaders: ConditionalHeaders, logger: LevelLogger) {
+    if (this.listenersAttached) {
+      return;
+    }
+
+    // @ts-ignore
+    const client = this.page._client;
+
+    // We have to reach into the Chrome Devtools Protocol to apply headers as using
+    // puppeteer's API will cause map tile requests to hang indefinitely:
+    //    https://github.com/puppeteer/puppeteer/issues/5003
+    // Docs on this client/protocol can be found here:
+    //    https://chromedevtools.github.io/devtools-protocol/tot/Fetch
+    client.on('Fetch.requestPaused', async (interceptedRequest: InterceptedRequest) => {
+      const {
+        requestId,
+        request: { url: interceptedUrl },
+      } = interceptedRequest;
+
+      const allowed = !interceptedUrl.startsWith('file://');
+      const isData = interceptedUrl.startsWith('data:');
+
+      // We should never ever let file protocol requests go through
+      if (!allowed || !this.allowRequest(interceptedUrl)) {
+        logger.error(`Got bad URL: "${interceptedUrl}", closing browser.`);
+        await client.send('Fetch.failRequest', {
+          errorReason: 'Aborted',
+          requestId,
+        });
+        this.page.browser().close();
+        throw new Error(
+          i18n.translate('xpack.reporting.chromiumDriver.disallowedOutgoingUrl', {
+            defaultMessage: `Received disallowed outgoing URL: "{interceptedUrl}", exiting`,
+            values: { interceptedUrl },
+          })
+        );
+      }
+
+      if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedUrl)) {
+        logger.debug(`Using custom headers for ${interceptedUrl}`);
+        const headers = map(
+          {
+            ...interceptedRequest.request.headers,
+            ...conditionalHeaders.headers,
+          },
+          (value, name) => ({
+            name,
+            value,
+          })
+        );
+
+        try {
+          await client.send('Fetch.continueRequest', {
+            requestId,
+            headers,
+          });
+        } catch (err) {
+          logger.error(
+            i18n.translate('xpack.reporting.chromiumDriver.failedToCompleteRequestUsingHeaders', {
+              defaultMessage: 'Failed to complete a request using headers: {error}',
+              values: { error: err },
+            })
+          );
+        }
+      } else {
+        const loggedUrl = isData ? this.truncateUrl(interceptedUrl) : interceptedUrl;
+        logger.debug(`No custom headers for ${loggedUrl}`);
+        try {
+          await client.send('Fetch.continueRequest', { requestId });
+        } catch (err) {
+          logger.error(
+            i18n.translate('xpack.reporting.chromiumDriver.failedToCompleteRequest', {
+              defaultMessage: 'Failed to complete a request: {error}',
+              values: { error: err },
+            })
+          );
+        }
+      }
+
+      this.interceptedCount = this.interceptedCount + (isData ? 0 : 1);
+    });
+
+    // Even though 3xx redirects go through our request
+    // handler, we should probably inspect responses just to
+    // avoid being bamboozled by some malicious request
+    this.page.on('response', (interceptedResponse: Response) => {
+      const interceptedUrl = interceptedResponse.url();
+      const allowed = !interceptedUrl.startsWith('file://');
+
+      if (!interceptedResponse.ok()) {
+        logger.warn(
+          `Chromium received a non-OK response (${interceptedResponse.status()}) for request ${interceptedUrl}`
+        );
+      }
+
+      if (!allowed || !this.allowRequest(interceptedUrl)) {
+        logger.error(`Got disallowed URL "${interceptedUrl}", closing browser.`);
+        this.page.browser().close();
+        throw new Error(`Received disallowed URL in response: ${interceptedUrl}`);
+      }
+    });
+
+    this.listenersAttached = true;
+  }
+
  private async launchDebugger() {
    // In order to pause on execution we have to reach more deeply into Chromiums Devtools Protocol,
    // and more specifically, for the page being used. _client is per-page, and puppeteer doesn't expose
--- a/x-pack/legacy/plugins/reporting/test_helpers/create_mock_browserdriverfactory.ts
+++ b/x-pack/legacy/plugins/reporting/test_helpers/create_mock_browserdriverfactory.ts
@ -17,6 +17,7 @@ interface CreateMockBrowserDriverFactoryOpts {
  evaluate: jest.Mock<Promise<any>, any[]>;
  waitForSelector: jest.Mock<Promise<any>, any[]>;
  screenshot: jest.Mock<Promise<any>, any[]>;
+  open: jest.Mock<Promise<any>, any[]>;
  getCreatePage: (driver: HeadlessChromiumDriver) => jest.Mock<any, any>;
 }

@ -87,6 +88,7 @@ const defaultOpts: CreateMockBrowserDriverFactoryOpts = {
  evaluate: mockBrowserEvaluate,
  waitForSelector: mockWaitForSelector,
  screenshot: mockScreenshot,
+  open: jest.fn(),
  getCreatePage,
 };

@ -124,6 +126,7 @@ export const createMockBrowserDriverFactory = async (
  mockBrowserDriver.waitForSelector = opts.waitForSelector ? opts.waitForSelector : defaultOpts.waitForSelector; // prettier-ignore
  mockBrowserDriver.evaluate = opts.evaluate ? opts.evaluate : defaultOpts.evaluate;
  mockBrowserDriver.screenshot = opts.screenshot ? opts.screenshot : defaultOpts.screenshot;
+  mockBrowserDriver.open = opts.open ? opts.open : defaultOpts.open;

  mockBrowserDriverFactory.createPage = opts.getCreatePage
    ? opts.getCreatePage(mockBrowserDriver)