Merge branch 'master' into fix-agent-flow-tests-75241

.github/PULL_REQUEST_TEMPLATE.md

@@ -9,8 +9,9 @@ Delete any items that are not applicable to this PR.
 - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
 - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
 - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
-- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)
-- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)
+- [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
+- [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
+- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
 - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
 
 ### For maintainers

src/plugins/dashboard/public/index.ts

@@ -31,7 +31,12 @@ export {
 } from './application';
 export { DashboardConstants, createDashboardEditUrl } from './dashboard_constants';
 
-export { DashboardStart, DashboardUrlGenerator, DashboardFeatureFlagConfig } from './plugin';
+export {
+  DashboardSetup,
+  DashboardStart,
+  DashboardUrlGenerator,
+  DashboardFeatureFlagConfig,
+} from './plugin';
 export {
   DASHBOARD_APP_URL_GENERATOR,
   createDashboardUrlGenerator,

src/plugins/dashboard/public/plugin.tsx

@@ -145,7 +145,7 @@ interface StartDependencies {
   savedObjects: SavedObjectsStart;
 }
 
-export type Setup = void;
+export type DashboardSetup = void;
 
 export interface DashboardStart {
   getSavedDashboardLoader: () => SavedObjectLoader;
@@ -180,7 +180,7 @@ declare module '../../../plugins/ui_actions/public' {
 }
 
 export class DashboardPlugin
-  implements Plugin<Setup, DashboardStart, SetupDependencies, StartDependencies> {
+  implements Plugin<DashboardSetup, DashboardStart, SetupDependencies, StartDependencies> {
   constructor(private initializerContext: PluginInitializerContext) {}
 
   private appStateUpdater = new BehaviorSubject<AppUpdater>(() => ({}));
@@ -193,17 +193,8 @@ export class DashboardPlugin
 
   public setup(
     core: CoreSetup<StartDependencies, DashboardStart>,
-    {
-      share,
-      uiActions,
-      embeddable,
-      home,
-      kibanaLegacy,
-      urlForwarding,
-      data,
-      usageCollection,
-    }: SetupDependencies
-  ): Setup {
+    { share, uiActions, embeddable, home, urlForwarding, data, usageCollection }: SetupDependencies
+  ): DashboardSetup {
     this.dashboardFeatureFlagConfig = this.initializerContext.config.get<
       DashboardFeatureFlagConfig
     >();

src/plugins/es_ui_shared/static/forms/hook_form_lib/hooks/use_field.ts

@@ -464,24 +464,58 @@ export const useField = <T>(
     [errors]
   );
 
+  /**
+   * Handler to update the state and make sure the component is still mounted.
+   * When resetting the form, some field might get unmounted (e.g. a toggle on "true" becomes "false" and now certain fields should not be in the DOM).
+   * In that scenario there is a race condition in the "reset" method below, because the useState() hook is not synchronous.
+   *
+   * A better approach would be to have the state in a reducer and being able to update all values in a single dispatch action.
+   */
+  const updateStateIfMounted = useCallback(
+    (
+      state: 'isPristine' | 'isValidating' | 'isChangingValue' | 'isValidated' | 'errors' | 'value',
+      nextValue: any
+    ) => {
+      if (isMounted.current === false) {
+        return;
+      }
+
+      switch (state) {
+        case 'value':
+          return setValue(nextValue);
+        case 'errors':
+          return setErrors(nextValue);
+        case 'isChangingValue':
+          return setIsChangingValue(nextValue);
+        case 'isPristine':
+          return setPristine(nextValue);
+        case 'isValidated':
+          return setIsValidated(nextValue);
+        case 'isValidating':
+          return setValidating(nextValue);
+      }
+    },
+    [setValue]
+  );
+
   const reset: FieldHook<T>['reset'] = useCallback(
     (resetOptions = { resetValue: true }) => {
       const { resetValue = true, defaultValue: updatedDefaultValue } = resetOptions;
 
-      setPristine(true);
-      setValidating(false);
-      setIsChangingValue(false);
-      setIsValidated(false);
-      setErrors([]);
+      updateStateIfMounted('isPristine', true);
+      updateStateIfMounted('isValidating', false);
+      updateStateIfMounted('isChangingValue', false);
+      updateStateIfMounted('isValidated', false);
+      updateStateIfMounted('errors', []);
 
       if (resetValue) {
         hasBeenReset.current = true;
         const newValue = deserializeValue(updatedDefaultValue ?? defaultValue);
-        setValue(newValue);
+        updateStateIfMounted('value', newValue);
         return newValue;
       }
     },
-    [setValue, deserializeValue, defaultValue]
+    [updateStateIfMounted, deserializeValue, defaultValue]
   );
 
   // Don't take into account non blocker validation. Some are just warning (like trying to add a wrong ComboBox item)

src/plugins/share/public/index.ts

@@ -40,4 +40,6 @@ export {
 
 import { SharePlugin } from './plugin';
 
+export { KibanaURL } from './kibana_url';
+
 export const plugin = () => new SharePlugin();

src/plugins/share/public/kibana_url.ts

@@ -0,0 +1,44 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+// TODO: Replace this logic with KibanaURL once it is available.
+// https://github.com/elastic/kibana/issues/64497
+export class KibanaURL {
+  public readonly path: string;
+  public readonly appName: string;
+  public readonly appPath: string;
+
+  constructor(path: string) {
+    const match = path.match(/^.*\/app\/([^\/#]+)(.+)$/);
+
+    if (!match) {
+      throw new Error('Unexpected URL path.');
+    }
+
+    const [, appName, appPath] = match;
+
+    if (!appName || !appPath) {
+      throw new Error('Could not parse URL path.');
+    }
+
+    this.path = path;
+    this.appName = appName;
+    this.appPath = appPath;
+  }
+}

src/plugins/telemetry/server/fetcher.test.ts

@@ -23,19 +23,93 @@ import { coreMock } from '../../../core/server/mocks';
 
 describe('FetcherTask', () => {
   describe('sendIfDue', () => {
-    it('returns undefined and warns when it fails to get telemetry configs', async () => {
+    it('stops when it fails to get telemetry configs', async () => {
       const initializerContext = coreMock.createPluginInitializerContext({});
       const fetcherTask = new FetcherTask(initializerContext);
       const mockError = new Error('Some message.');
-      fetcherTask['getCurrentConfigs'] = async () => {
-        throw mockError;
-      };
+      const getCurrentConfigs = jest.fn().mockRejectedValue(mockError);
+      const fetchTelemetry = jest.fn();
+      const sendTelemetry = jest.fn();
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        fetchTelemetry,
+        sendTelemetry,
+      });
       const result = await fetcherTask['sendIfDue']();
       expect(result).toBe(undefined);
+      expect(getCurrentConfigs).toBeCalledTimes(1);
+      expect(fetchTelemetry).toBeCalledTimes(0);
+      expect(sendTelemetry).toBeCalledTimes(0);
       expect(fetcherTask['logger'].warn).toBeCalledTimes(1);
       expect(fetcherTask['logger'].warn).toHaveBeenCalledWith(
-        `Error fetching telemetry configs: ${mockError}`
+        `Error getting telemetry configs. (${mockError})`
       );
     });
+
+    it('stops when all collectors are not ready', async () => {
+      const initializerContext = coreMock.createPluginInitializerContext({});
+      const fetcherTask = new FetcherTask(initializerContext);
+      const getCurrentConfigs = jest.fn().mockResolvedValue({});
+      const areAllCollectorsReady = jest.fn().mockResolvedValue(false);
+      const shouldSendReport = jest.fn().mockReturnValue(true);
+      const fetchTelemetry = jest.fn();
+      const sendTelemetry = jest.fn();
+      const updateReportFailure = jest.fn();
+
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        areAllCollectorsReady,
+        shouldSendReport,
+        fetchTelemetry,
+        updateReportFailure,
+        sendTelemetry,
+      });
+
+      await fetcherTask['sendIfDue']();
+
+      expect(fetchTelemetry).toBeCalledTimes(0);
+      expect(sendTelemetry).toBeCalledTimes(0);
+
+      expect(areAllCollectorsReady).toBeCalledTimes(1);
+      expect(updateReportFailure).toBeCalledTimes(0);
+      expect(fetcherTask['logger'].warn).toBeCalledTimes(1);
+      expect(fetcherTask['logger'].warn).toHaveBeenCalledWith(
+        `Error fetching usage. (Error: Not all collectors are ready.)`
+      );
+    });
+
+    it('fetches usage and send telemetry', async () => {
+      const initializerContext = coreMock.createPluginInitializerContext({});
+      const fetcherTask = new FetcherTask(initializerContext);
+      const mockTelemetryUrl = 'mock_telemetry_url';
+      const mockClusters = ['cluster_1', 'cluster_2'];
+      const getCurrentConfigs = jest.fn().mockResolvedValue({
+        telemetryUrl: mockTelemetryUrl,
+      });
+      const areAllCollectorsReady = jest.fn().mockResolvedValue(true);
+      const shouldSendReport = jest.fn().mockReturnValue(true);
+
+      const fetchTelemetry = jest.fn().mockResolvedValue(mockClusters);
+      const sendTelemetry = jest.fn();
+      const updateReportFailure = jest.fn();
+
+      Object.assign(fetcherTask, {
+        getCurrentConfigs,
+        areAllCollectorsReady,
+        shouldSendReport,
+        fetchTelemetry,
+        updateReportFailure,
+        sendTelemetry,
+      });
+
+      await fetcherTask['sendIfDue']();
+
+      expect(areAllCollectorsReady).toBeCalledTimes(1);
+      expect(fetchTelemetry).toBeCalledTimes(1);
+      expect(sendTelemetry).toBeCalledTimes(2);
+      expect(sendTelemetry).toHaveBeenNthCalledWith(1, mockTelemetryUrl, mockClusters[0]);
+      expect(sendTelemetry).toHaveBeenNthCalledWith(2, mockTelemetryUrl, mockClusters[1]);
+      expect(updateReportFailure).toBeCalledTimes(0);
+    });
   });
 });

src/plugins/telemetry/server/fetcher.ts

@@ -22,7 +22,10 @@ import { Observable } from 'rxjs';
 import { take } from 'rxjs/operators';
 // @ts-ignore
 import fetch from 'node-fetch';
-import { TelemetryCollectionManagerPluginStart } from 'src/plugins/telemetry_collection_manager/server';
+import {
+  TelemetryCollectionManagerPluginStart,
+  UsageStatsPayload,
+} from 'src/plugins/telemetry_collection_manager/server';
 import {
   PluginInitializerContext,
   Logger,
@@ -94,6 +97,10 @@ export class FetcherTask {
     }
   }
 
+  private async areAllCollectorsReady() {
+    return (await this.telemetryCollectionManager?.areAllCollectorsReady()) ?? false;
+  }
+
   private async sendIfDue() {
     if (this.isSending) {
       return;
@@ -103,17 +110,30 @@ export class FetcherTask {
     try {
       telemetryConfig = await this.getCurrentConfigs();
     } catch (err) {
-      this.logger.warn(`Error fetching telemetry configs: ${err}`);
+      this.logger.warn(`Error getting telemetry configs. (${err})`);
       return;
     }
 
     if (!telemetryConfig || !this.shouldSendReport(telemetryConfig)) {
       return;
     }
 
+    let clusters: Array<UsageStatsPayload | string> = [];
+    this.isSending = true;
+
+    try {
+      const allCollectorsReady = await this.areAllCollectorsReady();
+      if (!allCollectorsReady) {
+        throw new Error('Not all collectors are ready.');
+      }
+      clusters = await this.fetchTelemetry();
+    } catch (err) {
+      this.logger.warn(`Error fetching usage. (${err})`);
+      this.isSending = false;
+      return;
+    }
+
     try {
-      this.isSending = true;
-      const clusters = await this.fetchTelemetry();
       const { telemetryUrl } = telemetryConfig;
       for (const cluster of clusters) {
         await this.sendTelemetry(telemetryUrl, cluster);
@@ -123,7 +143,7 @@ export class FetcherTask {
     } catch (err) {
       await this.updateReportFailure(telemetryConfig);
 
-      this.logger.warn(`Error sending telemetry usage data: ${err}`);
+      this.logger.warn(`Error sending telemetry usage data. (${err})`);
     }
     this.isSending = false;
   }

src/plugins/telemetry_collection_manager/server/index.ts

@@ -38,4 +38,5 @@ export {
   ClusterDetails,
   ClusterDetailsGetter,
   LicenseGetter,
+  UsageStatsPayload,
 } from './types';

src/plugins/telemetry_collection_manager/server/plugin.ts

@@ -67,6 +67,7 @@ export class TelemetryCollectionManagerPlugin
       setCollection: this.setCollection.bind(this),
       getOptInStats: this.getOptInStats.bind(this),
       getStats: this.getStats.bind(this),
+      areAllCollectorsReady: this.areAllCollectorsReady.bind(this),
     };
   }
 
@@ -75,6 +76,7 @@ export class TelemetryCollectionManagerPlugin
       setCollection: this.setCollection.bind(this),
       getOptInStats: this.getOptInStats.bind(this),
       getStats: this.getStats.bind(this),
+      areAllCollectorsReady: this.areAllCollectorsReady.bind(this),
     };
   }
 
@@ -185,6 +187,10 @@ export class TelemetryCollectionManagerPlugin
     return [];
   }
 
+  private areAllCollectorsReady = async () => {
+    return await this.usageCollection?.areAllCollectorsReady();
+  };
+
   private getOptInStatsForCollection = async (
     collection: Collection,
     optInStatus: boolean,

src/plugins/telemetry_collection_manager/server/types.ts

@@ -34,6 +34,7 @@ export interface TelemetryCollectionManagerPluginSetup {
   ) => void;
   getOptInStats: TelemetryCollectionManagerPlugin['getOptInStats'];
   getStats: TelemetryCollectionManagerPlugin['getStats'];
+  areAllCollectorsReady: TelemetryCollectionManagerPlugin['areAllCollectorsReady'];
 }
 
 export interface TelemetryCollectionManagerPluginStart {
@@ -42,6 +43,7 @@ export interface TelemetryCollectionManagerPluginStart {
   ) => void;
   getOptInStats: TelemetryCollectionManagerPlugin['getOptInStats'];
   getStats: TelemetryCollectionManagerPlugin['getStats'];
+  areAllCollectorsReady: TelemetryCollectionManagerPlugin['areAllCollectorsReady'];
 }
 
 export interface TelemetryOptInStats {