[journeys] restart ES for each journey, fix flakiness (#141530)

.buildkite/pipelines/performance/daily.yml

@@ -20,6 +20,12 @@ steps:
     depends_on: build
     key: tests
     timeout_in_minutes: 60
+    retry:
+      automatic:
+        - exit_status: '-1'
+          limit: 3
+        - exit_status: '*'
+          limit: 1
 
   - label: '🚢 Performance Tests dataset extraction for scalability benchmarking'
     command: .buildkite/scripts/steps/functional/scalability_dataset_extraction.sh

.buildkite/scripts/steps/functional/performance_playwright.sh

@@ -11,26 +11,9 @@ is_test_execution_step
 rm -rf "$KIBANA_BUILD_LOCATION"
 .buildkite/scripts/download_build_artifacts.sh
 
-echo "--- 🦺 Starting Elasticsearch"
-
-node scripts/es snapshot&
-export esPid=$!
-trap 'kill ${esPid}' EXIT
-
-export TEST_ES_URL=http://elastic:changeme@localhost:9200
-export TEST_ES_DISABLE_STARTUP=true
-
-# Pings the es server every second for up to 2 minutes until it is green
-curl \
-  --fail \
-  --silent \
-  --retry 120 \
-  --retry-delay 1 \
-  --retry-connrefused \
-  -XGET "${TEST_ES_URL}/_cluster/health?wait_for_nodes=>=1&wait_for_status=yellow" \
-  > /dev/null
-
-echo "✅ ES is ready and will continue to run in the background"
+function is_running {
+  kill -0 "$1" &>/dev/null
+}
 
 # unset env vars defined in other parts of CI for automatic APM collection of
 # Kibana. We manage APM config in our FTR config and performance service, and
@@ -46,29 +29,100 @@ unset ELASTIC_APM_SERVER_URL
 unset ELASTIC_APM_SECRET_TOKEN
 unset ELASTIC_APM_GLOBAL_LABELS
 
-journeys=("login" "ecommerce_dashboard" "flight_dashboard" "web_logs_dashboard" "promotion_tracking_dashboard" "many_fields_discover" "data_stress_test_lens")
+# `kill $esPid` doesn't work, seems that kbn-es doesn't listen to signals correctly, this does work
+trap 'killall node -q' EXIT
 
-for journey in "${journeys[@]}"; do
-  set +e
+export TEST_ES_URL=http://elastic:changeme@localhost:9200
+export TEST_ES_DISABLE_STARTUP=true
+
+echo "--- determining which journeys to run"
+
+journeys=$(buildkite-agent meta-data get "failed-journeys" --default '')
+if [ "$journeys" != "" ]; then
+  echo "re-running failed journeys:${journeys}"
+else
+  paths=()
+  for path in x-pack/performance/journeys/*; do
+    paths+=("$path")
+  done
+  journeys=$(printf "%s\n" "${paths[@]}")
+  echo "running discovered journeys:${journeys}"
+fi
+
+# track failed journeys here which might get written to metadata
+failedJourneys=()
+
+while read -r journey; do
+  if [ "$journey" == "" ]; then
+    continue;
+  fi
+
+  echo "--- $journey - 🔎 Start es"
+
+  node scripts/es snapshot&
+  export esPid=$!
+
+  # Pings the es server every second for up to 2 minutes until it is green
+  curl \
+    --fail \
+    --silent \
+    --retry 120 \
+    --retry-delay 1 \
+    --retry-connrefused \
+    -XGET "${TEST_ES_URL}/_cluster/health?wait_for_nodes=>=1&wait_for_status=yellow" \
+    > /dev/null
+
+  echo "✅ ES is ready and will run in the background"
 
   phases=("WARMUP" "TEST")
+  status=0
   for phase in "${phases[@]}"; do
     echo "--- $journey - $phase"
 
     export TEST_PERFORMANCE_PHASE="$phase"
+
+    set +e
     node scripts/functional_tests \
-      --config "x-pack/performance/journeys/$journey.ts" \
+      --config "$journey" \
       --kibana-install-dir "$KIBANA_BUILD_LOCATION" \
       --debug \
       --bail
-
     status=$?
+    set -e
+
     if [ $status -ne 0 ]; then
+      failedJourneys+=("$journey")
       echo "^^^ +++"
       echo "❌ FTR failed with status code: $status"
-      exit 1
+      break
     fi
   done
 
-  set -e
-done
+  # remove trap, we're manually shutting down
+  trap - EXIT;
+
+  echo "--- $journey - 🔎 Shutdown ES"
+  killall node
+  echo "waiting for $esPid to exit gracefully";
+
+  timeout=30 #seconds
+  dur=0
+  while is_running $esPid; do
+    sleep 1;
+    ((dur=dur+1))
+    if [ $dur -ge $timeout ]; then
+      echo "es still running after $dur seconds, killing ES and node forcefully";
+      killall -SIGKILL java
+      killall -SIGKILL node
+      sleep 5;
+    fi
+  done
+done <<< "$journeys"
+
+echo "--- report/record failed journeys"
+if [ "${failedJourneys[*]}" != "" ]; then
+  buildkite-agent meta-data set "failed-journeys" "$(printf "%s\n" "${failedJourneys[@]}")"
+
+  echo "failed journeys: ${failedJourneys[*]}"
+  exit 1
+fi

.buildkite/scripts/steps/functional/scalability_dataset_extraction.sh

@@ -46,8 +46,13 @@ cd "${OUTPUT_DIR}/.."
 gsutil -m cp -r "${BUILD_ID}" "${GCS_BUCKET}"
 cd -
 
-echo "--- Promoting '${BUILD_ID}' dataset to LATEST"
-cd "${OUTPUT_DIR}/.."
-echo "${BUILD_ID}" > latest
-gsutil cp latest "${GCS_BUCKET}"
-cd -
+if [ "$BUILDKITE_PIPELINE_SLUG" == "kibana-single-user-performance" ]; then
+  echo "--- Promoting '${BUILD_ID}' dataset to LATEST"
+  cd "${OUTPUT_DIR}/.."
+  echo "${BUILD_ID}" > latest
+  gsutil cp latest "${GCS_BUCKET}"
+  cd -
+else
+  echo "--- Skipping promotion of dataset to LATEST"
+  echo "$BUILDKITE_PIPELINE_SLUG is not 'kibana-single-user-performance', so skipping"
+fi

packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failures_to_file.ts

@@ -44,12 +44,26 @@ async function getJourneySnapshotHtml(log: ToolingLog, journeyMeta: JourneyMeta)
   return [
     '<section>',
     '<h5>Steps</h5>',
-    ...screenshots.get().flatMap(({ title, path }) => {
+    ...screenshots.get().flatMap(({ title, path, fullscreenPath }) => {
       const base64 = Fs.readFileSync(path, 'base64');
+      const fullscreenBase64 = Fs.readFileSync(fullscreenPath, 'base64');
 
       return [
         `<p><strong>${escape(title)}</strong></p>`,
-        `<img class="screenshot img-fluid img-thumbnail" src="data:image/png;base64,${base64}" />`,
+        `<div class="screenshotContainer">
+          <img class="screenshot img-fluid img-thumbnail" src="data:image/png;base64,${base64}" />
+          <img class="screenshot img-fluid img-thumbnail fs" src="data:image/png;base64,${fullscreenBase64}" />
+          <button type="button" class="toggleFs on" title="Expand screenshot to full page">
+            <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-arrows-expand" viewBox="0 0 16 16">
+              <path fill-rule="evenodd" d="M1 8a.5.5 0 0 1 .5-.5h13a.5.5 0 0 1 0 1h-13A.5.5 0 0 1 1 8zM7.646.146a.5.5 0 0 1 .708 0l2 2a.5.5 0 0 1-.708.708L8.5 1.707V5.5a.5.5 0 0 1-1 0V1.707L6.354 2.854a.5.5 0 1 1-.708-.708l2-2zM8 10a.5.5 0 0 1 .5.5v3.793l1.146-1.147a.5.5 0 0 1 .708.708l-2 2a.5.5 0 0 1-.708 0l-2-2a.5.5 0 0 1 .708-.708L7.5 14.293V10.5A.5.5 0 0 1 8 10z"/>
+            </svg>
+          </button>
+          <button type="button" class="toggleFs off" title="Restrict screenshot to content visible in the viewport">
+            <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-arrows-collapse" viewBox="0 0 16 16">
+              <path fill-rule="evenodd" d="M1 8a.5.5 0 0 1 .5-.5h13a.5.5 0 0 1 0 1h-13A.5.5 0 0 1 1 8zm7-8a.5.5 0 0 1 .5.5v3.793l1.146-1.147a.5.5 0 0 1 .708.708l-2 2a.5.5 0 0 1-.708 0l-2-2a.5.5 0 1 1 .708-.708L7.5 4.293V.5A.5.5 0 0 1 8 0zm-.5 11.707-1.146 1.147a.5.5 0 0 1-.708-.708l2-2a.5.5 0 0 1 .708 0l2 2a.5.5 0 0 1-.708.708L8.5 11.707V15.5a.5.5 0 0 1-1 0v-3.793z"/>
+            </svg>
+          </button>
+        </div>`,
       ];
     }),
     '</section>',
@@ -88,7 +102,11 @@ function getFtrScreenshotHtml(log: ToolingLog, failureName: string) {
     .filter((s) => s.name.startsWith(FtrScreenshotFilename.create(failureName, { ext: false })))
     .map((s) => {
       const base64 = Fs.readFileSync(s.path).toString('base64');
-      return `<img class="screenshot img-fluid img-thumbnail" src="data:image/png;base64,${base64}" />`;
+      return `
+        <div class="screenshotContainer">
+          <img class="screenshot img-fluid img-thumbnail" src="data:image/png;base64,${base64}" />
+        </div>
+      `;
     })
     .join('\n');
 }

packages/kbn-failed-test-reporter-cli/failed_tests_reporter/report_failures_to_file_html_template.html

@@ -16,12 +16,24 @@
 
       img.screenshot {
         cursor: pointer;
-        height: 200px;
         margin: 5px 0;
       }
+      .screenshotContainer:not(.expanded) img.screenshot {
+        height: 200px;
+      }
 
-      img.screenshot.expanded {
-        height: auto;
+      .screenshotContainer:not(.fs) img.screenshot.fs,
+      .screenshotContainer:not(.fs) button.toggleFs.off,
+      .screenshotContainer.fs img.screenshot:not(.fs),
+      .screenshotContainer.fs button.toggleFs.on {
+        display: none;
+      }
+
+      .screenshotContainer .toggleFs {
+        background: none;
+        border: none;
+        margin: 0 0 0 5px;
+        vertical-align: top;
       }
     </style>
     <title>$TITLE</title>
@@ -31,11 +43,46 @@
       <main>$MAIN</main>
     </div>
     <script type="text/javascript">
-      for (const img of document.getElementsByTagName('img')) {
-        img.addEventListener('click', () => {
-          img.classList.toggle('expanded');
-        });
+      /**
+       * @param {HTMLElement} el
+       * @param {(el: HTMLElement) => boolean} className
+       */
+      function findParent(el, test) {
+        while (el) {
+          if (test(el)) {
+            return el
+          }
+
+          // stop if we iterate all the way up to the document body
+          if (el.parentElement === document.body) {
+            break
+          }
+
+          el = el.parentElement
+        }
+
+        return null
       }
+
+      function isContainer(el) {
+        return el.classList.contains('screenshotContainer')
+      }
+
+      function isButtonOrImg(el) {
+        return el instanceof HTMLImageElement || el instanceof HTMLButtonElement
+      }
+
+      document.addEventListener('click', (event) => {
+        const el = findParent(event.target, isButtonOrImg)
+
+        if (el instanceof HTMLImageElement && el.classList.contains('screenshot')) {
+          findParent(el, isContainer)?.classList.toggle('expanded')
+        }
+
+        if (el instanceof HTMLButtonElement && el.classList.contains('toggleFs')) {
+          findParent(el, isContainer)?.classList.toggle('fs')
+        }
+      })
     </script>
   </body>
 </html>

packages/kbn-journeys/journey/journey_ftr_config.ts

@@ -82,7 +82,7 @@ export function makeFtrConfigProvider(
       kbnTestServer: {
         ...baseConfig.kbnTestServer,
         // delay shutdown by 15 seconds to ensure that APM can report the data it collects during test execution
-        delayShutdown: 15_000,
+        delayShutdown: process.env.TEST_PERFORMANCE_PHASE === 'TEST' ? 15_000 : 0,
 
         serverArgs: [
           ...baseConfig.kbnTestServer.serverArgs,

packages/kbn-journeys/journey/journey_ftr_harness.ts

@@ -198,7 +198,12 @@ export class JourneyFtrHarness {
       return;
     }
 
-    await this.screenshots.addSuccess(step, await this.page.screenshot());
+    const [screenshot, fs] = await Promise.all([
+      this.page.screenshot(),
+      this.page.screenshot({ fullPage: true }),
+    ]);
+
+    await this.screenshots.addSuccess(step, screenshot, fs);
   }
 
   private async onStepError(step: AnyStep, err: Error) {
@@ -208,7 +213,12 @@ export class JourneyFtrHarness {
     }
 
     if (this.page) {
-      await this.screenshots.addError(step, await this.page.screenshot());
+      const [screenshot, fs] = await Promise.all([
+        this.page.screenshot(),
+        this.page.screenshot({ fullPage: true }),
+      ]);
+
+      await this.screenshots.addError(step, screenshot, fs);
     }
   }
 

packages/kbn-journeys/journey/journey_screenshots.ts

@@ -19,6 +19,7 @@ interface StepShot {
   type: 'success' | 'failure';
   title: string;
   filename: string;
+  fullscreenFilename: string;
 }
 
 interface Manifest {
@@ -87,34 +88,44 @@ export class JourneyScreenshots {
     }
   }
 
-  async addError(step: AnyStep, screenshot: Buffer) {
+  async addError(step: AnyStep, screenshot: Buffer, fullscreenScreenshot: Buffer) {
     await this.lock(async () => {
       const filename = FtrScreenshotFilename.create(`${step.index}-${step.name}-failure`);
+      const fullscreenFilename = FtrScreenshotFilename.create(
+        `${step.index}-${step.name}-failure-fullscreen`
+      );
       this.#manifest.steps.push({
         type: 'failure',
         title: `Step #${step.index + 1}: ${step.name} - FAILED`,
         filename,
+        fullscreenFilename,
       });
 
       await Promise.all([
         write(Path.resolve(this.#dir, 'manifest.json'), JSON.stringify(this.#manifest)),
         write(Path.resolve(this.#dir, filename), screenshot),
+        write(Path.resolve(this.#dir, fullscreenFilename), fullscreenScreenshot),
       ]);
     });
   }
 
-  async addSuccess(step: AnyStep, screenshot: Buffer) {
+  async addSuccess(step: AnyStep, screenshot: Buffer, fullscreenScreenshot: Buffer) {
     await this.lock(async () => {
       const filename = FtrScreenshotFilename.create(`${step.index}-${step.name}`);
+      const fullscreenFilename = FtrScreenshotFilename.create(
+        `${step.index}-${step.name}-fullscreen`
+      );
       this.#manifest.steps.push({
         type: 'success',
         title: `Step #${step.index + 1}: ${step.name} - DONE`,
         filename,
+        fullscreenFilename,
       });
 
       await Promise.all([
         write(Path.resolve(this.#dir, 'manifest.json'), JSON.stringify(this.#manifest)),
         write(Path.resolve(this.#dir, filename), screenshot),
+        write(Path.resolve(this.#dir, fullscreenFilename), fullscreenScreenshot),
       ]);
     });
   }
@@ -123,6 +134,7 @@ export class JourneyScreenshots {
     return this.#manifest.steps.map((stepShot) => ({
       ...stepShot,
       path: Path.resolve(this.#dir, stepShot.filename),
+      fullscreenPath: Path.resolve(this.#dir, stepShot.fullscreenFilename),
     }));
   }
 }

x-pack/performance/jest.config.js

@@ -0,0 +1,12 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+module.exports = {
+  preset: '@kbn/test/jest_node',
+  rootDir: '../..',
+  roots: ['<rootDir>/x-pack/performance'],
+};

x-pack/performance/services/lib/time.test.ts

@@ -0,0 +1,26 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { ms, toMs } from './time';
+
+describe('ms()', () => {
+  it('converts simple timestrings to milliseconds', () => {
+    expect(ms('1s')).toMatchInlineSnapshot(`1000`);
+    expect(ms('10s')).toMatchInlineSnapshot(`10000`);
+    expect(ms('1m')).toMatchInlineSnapshot(`60000`);
+    expect(ms('10m')).toMatchInlineSnapshot(`600000`);
+    expect(ms('0.5s')).toMatchInlineSnapshot(`500`);
+    expect(ms('0.5m')).toMatchInlineSnapshot(`30000`);
+  });
+});
+
+describe('toMs()', () => {
+  it('converts strings to ms, returns number directly', () => {
+    expect(toMs(1000)).toBe(1000);
+    expect(toMs('1s')).toBe(1000);
+  });
+});

x-pack/performance/services/lib/time.ts

@@ -0,0 +1,41 @@
+/*
+ * 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; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export const SECOND = 1000;
+export const MINUTE = 60 * SECOND;
+
+const TIME_STR_RE = /^((?:\d+)(?:\.\d+)?)(m|s)$/i;
+
+/**
+ * Either a number of milliseconds or a simple time string (eg. 2m or 30s)
+ */
+export type TimeOrMilliseconds = number | string;
+
+export function toMs(timeOrMs: TimeOrMilliseconds) {
+  return typeof timeOrMs === 'number' ? timeOrMs : ms(timeOrMs);
+}
+
+/**
+ * Convert a basic time string into milliseconds. The string can end with
+ * `m` (for minutes) or `s` (for seconds) and have any number before it.
+ */
+export function ms(time: string) {
+  const match = time.match(TIME_STR_RE);
+  if (!match) {
+    throw new Error('invalid time string, expected a number followed by "m" or "s"');
+  }
+
+  const [, num, unit] = match;
+  switch (unit.toLowerCase()) {
+    case 's':
+      return Number.parseFloat(num) * SECOND;
+    case 'm':
+      return Number.parseFloat(num) * MINUTE;
+    default:
+      throw new Error(`unexpected timestring unit [time=${time}] [unit=${unit}]`);
+  }
+}

x-pack/performance/services/toasts.ts

@@ -9,6 +9,8 @@ import { ToolingLog } from '@kbn/tooling-log';
 import { subj } from '@kbn/test-subj-selector';
 import { Page } from 'playwright';
 
+import { toMs, type TimeOrMilliseconds } from './lib/time';
+
 export class ToastsService {
   constructor(private readonly log: ToolingLog, private readonly page: Page) {}
 
@@ -16,13 +18,18 @@ export class ToastsService {
    * Wait for a toast with some bit of text matching the provided `textSnipped`, then clear
    * it and resolve the promise.
    */
-  async waitForAndClear(textSnippet: string) {
+  async waitForAndClear(
+    textSnippet: string,
+    options?: {
+      /** How long should we wait for the toast to show up? */
+      timeout?: TimeOrMilliseconds;
+    }
+  ) {
     const txt = JSON.stringify(textSnippet);
     this.log.info(`waiting for toast that has the text ${txt}`);
-    const toastSel = `.euiToast:has-text(${txt})`;
 
-    const toast = this.page.locator(toastSel);
-    await toast.waitFor();
+    const toast = this.page.locator(`.euiToast:has-text(${txt})`);
+    await toast.waitFor({ timeout: toMs(options?.timeout ?? '2m') });
 
     this.log.info('toast found, closing');
 

x-pack/performance/tsconfig.json

@@ -5,7 +5,7 @@
     "emitDeclarationOnly": true,
     "declaration": true,
     "declarationMap": true,
-    "types": ["node", "mocha"]
+    "types": ["node", "jest"]
   },
   "include": ["**/*.ts"],
 }