Delay client-side feature usage registration until start (#79365)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

test/scripts/jenkins_xpack_build_plugins.sh

@@ -10,5 +10,6 @@ node scripts/build_kibana_platform_plugins \
   --scan-dir "$XPACK_DIR/test/alerting_api_integration/plugins" \
   --scan-dir "$XPACK_DIR/test/plugin_api_integration/plugins" \
   --scan-dir "$XPACK_DIR/test/plugin_api_perf/plugins" \
+  --scan-dir "$XPACK_DIR/test/licensing_plugin/plugins" \
   --workers 12 \
   --verbose

x-pack/plugins/licensing/public/plugin.ts

@@ -117,7 +117,7 @@ export class LicensingPlugin implements Plugin<LicensingPluginSetup, LicensingPl
     return {
       refresh: refreshManually,
       license$,
-      featureUsage: this.featureUsage.setup({ http: core.http }),
+      featureUsage: this.featureUsage.setup(),
     };
   }
 

x-pack/plugins/licensing/public/services/feature_usage_service.test.ts

@@ -18,22 +18,33 @@ describe('FeatureUsageService', () => {
 
   describe('#setup', () => {
     describe('#register', () => {
-      it('calls the endpoint with the correct parameters', async () => {
-        const setup = service.setup({ http });
-        await setup.register('my-feature', 'platinum');
+      it('calls the endpoint on start with the correct parameters', async () => {
+        const setup = service.setup();
+        setup.register('my-feature', 'platinum');
+        setup.register('my-other-feature', 'gold');
+        expect(http.post).not.toHaveBeenCalled();
+
+        service.start({ http });
         expect(http.post).toHaveBeenCalledTimes(1);
         expect(http.post).toHaveBeenCalledWith('/internal/licensing/feature_usage/register', {
-          body: JSON.stringify({
-            featureName: 'my-feature',
-            licenseType: 'platinum',
-          }),
+          body: JSON.stringify([
+            { featureName: 'my-feature', licenseType: 'platinum' },
+            { featureName: 'my-other-feature', licenseType: 'gold' },
+          ]),
         });
       });
 
-      it('does not call endpoint if on anonymous path', async () => {
+      it('does not call endpoint on start if no registrations', async () => {
+        service.setup();
+        service.start({ http });
+        expect(http.post).not.toHaveBeenCalled();
+      });
+
+      it('does not call endpoint on start if on anonymous path', async () => {
         http.anonymousPaths.isAnonymous.mockReturnValue(true);
-        const setup = service.setup({ http });
-        await setup.register('my-feature', 'platinum');
+        const setup = service.setup();
+        setup.register('my-feature', 'platinum');
+        service.start({ http });
         expect(http.post).not.toHaveBeenCalled();
       });
     });
@@ -42,7 +53,7 @@ describe('FeatureUsageService', () => {
   describe('#start', () => {
     describe('#notifyUsage', () => {
       it('calls the endpoint with the correct parameters', async () => {
-        service.setup({ http });
+        service.setup();
         const start = service.start({ http });
         await start.notifyUsage('my-feature', 42);
 
@@ -56,7 +67,7 @@ describe('FeatureUsageService', () => {
       });
 
       it('correctly convert dates', async () => {
-        service.setup({ http });
+        service.setup();
         const start = service.start({ http });
 
         const now = new Date();
@@ -74,7 +85,7 @@ describe('FeatureUsageService', () => {
 
       it('does not call endpoint if on anonymous path', async () => {
         http.anonymousPaths.isAnonymous.mockReturnValue(true);
-        service.setup({ http });
+        service.setup();
         const start = service.start({ http });
         await start.notifyUsage('my-feature', 42);
         expect(http.post).not.toHaveBeenCalled();

x-pack/plugins/licensing/public/services/feature_usage_service.ts

@@ -5,15 +5,15 @@
  */
 
 import { isDate } from 'lodash';
-import type { HttpSetup, HttpStart } from 'src/core/public';
+import type { HttpStart } from 'src/core/public';
 import { LicenseType } from '../../common/types';
 
 /** @public */
 export interface FeatureUsageServiceSetup {
   /**
    * Register a feature to be able to notify of it's usages using the {@link FeatureUsageServiceStart | service start contract}.
    */
-  register(featureName: string, licenseType: LicenseType): Promise<void>;
+  register(featureName: string, licenseType: LicenseType): void;
 }
 
 /** @public */
@@ -27,10 +27,6 @@ export interface FeatureUsageServiceStart {
   notifyUsage(featureName: string, usedAt?: Date | number): Promise<void>;
 }
 
-interface SetupDeps {
-  http: HttpSetup;
-}
-
 interface StartDeps {
   http: HttpStart;
 }
@@ -39,29 +35,33 @@ interface StartDeps {
  * @internal
  */
 export class FeatureUsageService {
-  public setup({ http }: SetupDeps): FeatureUsageServiceSetup {
+  private readonly registrations: Array<{ featureName: string; licenseType: LicenseType }> = [];
+
+  public setup(): FeatureUsageServiceSetup {
     return {
       register: async (featureName, licenseType) => {
-        // Skip registration if on logged-out page
-        // NOTE: this only works because the login page does a full-page refresh after logging in
-        // If this is ever changed, this code will need to buffer registrations and call them after the user logs in.
-        if (http.anonymousPaths.isAnonymous(window.location.pathname)) return;
-
-        await http.post('/internal/licensing/feature_usage/register', {
-          body: JSON.stringify({
-            featureName,
-            licenseType,
-          }),
-        });
+        this.registrations.push({ featureName, licenseType });
       },
     };
   }
 
   public start({ http }: StartDeps): FeatureUsageServiceStart {
+    // Skip registration if on logged-out page
+    // NOTE: this only works because the login page does a full-page refresh after logging in
+    // If this is ever changed, this code will need to buffer registrations and call them after the user logs in.
+    const registrationPromise =
+      http.anonymousPaths.isAnonymous(window.location.pathname) || this.registrations.length === 0
+        ? Promise.resolve()
+        : http.post('/internal/licensing/feature_usage/register', {
+            body: JSON.stringify(this.registrations),
+          });
+
     return {
       notifyUsage: async (featureName, usedAt = Date.now()) => {
         // Skip notification if on logged-out page
         if (http.anonymousPaths.isAnonymous(window.location.pathname)) return;
+        // Wait for registrations to complete
+        await registrationPromise;
 
         const lastUsed = isDate(usedAt) ? usedAt.getTime() : usedAt;
         await http.post('/internal/licensing/feature_usage/notify', {

x-pack/plugins/licensing/server/routes/internal/register_feature.ts

@@ -16,22 +16,26 @@ export function registerRegisterFeatureRoute(
     {
       path: '/internal/licensing/feature_usage/register',
       validate: {
-        body: schema.object({
-          featureName: schema.string(),
-          licenseType: schema.string({
-            validate: (value) => {
-              if (!(value in LICENSE_TYPE)) {
-                return `Invalid license type: ${value}`;
-              }
-            },
-          }),
-        }),
+        body: schema.arrayOf(
+          schema.object({
+            featureName: schema.string(),
+            licenseType: schema.string({
+              validate: (value) => {
+                if (!(value in LICENSE_TYPE)) {
+                  return `Invalid license type: ${value}`;
+                }
+              },
+            }),
+          })
+        ),
       },
     },
     async (context, request, response) => {
-      const { featureName, licenseType } = request.body;
+      const registrations = request.body;
 
-      featureUsageSetup.register(featureName, licenseType as LicenseType);
+      registrations.forEach(({ featureName, licenseType }) => {
+        featureUsageSetup.register(featureName, licenseType as LicenseType);
+      });
 
       return response.ok({
         body: {

x-pack/plugins/ui_actions_enhanced/public/services/ui_actions_service_enhancements.ts

@@ -158,17 +158,7 @@ export class UiActionsServiceEnhancements
 
   private registerFeatureUsage = (definition: ActionFactoryDefinition<any, any, any>): void => {
     if (!definition.minimalLicense || !definition.licenseFeatureName) return;
-
-    // Intentionally don't wait for response because
-    // happens in setup phase and has to be sync
-    this.deps.featureUsageSetup
-      .register(definition.licenseFeatureName, definition.minimalLicense)
-      .catch(() => {
-        // eslint-disable-next-line no-console
-        console.warn(
-          `ActionFactory [actionFactory.id = ${definition.id}] fail to register feature for featureUsage.`
-        );
-      });
+    this.deps.featureUsageSetup.register(definition.licenseFeatureName, definition.minimalLicense);
   };
 
   public readonly telemetry = (state: DynamicActionsState, telemetry: Record<string, any> = {}) => {

x-pack/test/licensing_plugin/config.public.ts

@@ -23,6 +23,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
           KIBANA_ROOT,
           'test/plugin_functional/plugins/core_provider_plugin'
         )}`,
+        `--plugin-path=${path.resolve(__dirname, 'plugins/test_feature_usage')}`,
       ],
     },
   };

x-pack/test/licensing_plugin/plugins/test_feature_usage/kibana.json

@@ -0,0 +1,7 @@
+{
+  "id": "testFeatureUsage",
+  "version": "kibana",
+  "server": false,
+  "ui": true,
+  "requiredPlugins": ["licensing"]
+}

x-pack/test/licensing_plugin/plugins/test_feature_usage/public/index.ts

@@ -0,0 +1,9 @@
+/*
+ * 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 { TestFeatureUsagePlugin } from './plugin';
+
+export const plugin = () => new TestFeatureUsagePlugin();

x-pack/test/licensing_plugin/plugins/test_feature_usage/public/plugin.ts

@@ -0,0 +1,22 @@
+/*
+ * 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 { CoreSetup } from 'src/core/server';
+import { LicensingPluginSetup } from '../../../../../plugins/licensing/public';
+
+interface SetupPlugins {
+  licensing: LicensingPluginSetup;
+}
+
+export class TestFeatureUsagePlugin {
+  public setup(core: CoreSetup, plugins: SetupPlugins) {
+    plugins.licensing.featureUsage.register('test-client-A', 'gold');
+    plugins.licensing.featureUsage.register('test-client-B', 'enterprise');
+  }
+
+  public start() {}
+  public stop() {}
+}

x-pack/test/licensing_plugin/public/feature_usage.ts

@@ -5,11 +5,7 @@
  */
 import expect from '@kbn/expect';
 import { FtrProviderContext } from '../services';
-import {
-  LicensingPluginSetup,
-  LicensingPluginStart,
-  LicenseType,
-} from '../../../plugins/licensing/public';
+import { LicensingPluginStart, LicenseType } from '../../../plugins/licensing/public';
 import '../../../../test/plugin_functional/plugins/core_provider_plugin/types';
 
 interface FeatureUsage {
@@ -25,19 +21,6 @@ export default function (ftrContext: FtrProviderContext) {
   const browser = getService('browser');
   const PageObjects = getPageObjects(['common', 'security']);
 
-  const registerFeature = async (featureName: string, licenseType: LicenseType) => {
-    await browser.executeAsync(
-      async (feature, type, cb) => {
-        const { setup } = window._coreProvider;
-        const licensing: LicensingPluginSetup = setup.plugins.licensing;
-        await licensing.featureUsage.register(feature, type);
-        cb();
-      },
-      featureName,
-      licenseType
-    );
-  };
-
   const notifyFeatureUsage = async (featureName: string, lastUsed: number) => {
     await browser.executeAsync(
       async (feature, time, cb) => {
@@ -57,12 +40,10 @@ export default function (ftrContext: FtrProviderContext) {
     });
 
     it('allows to register features to the server', async () => {
-      await registerFeature('test-client-A', 'gold');
-      await registerFeature('test-client-B', 'enterprise');
-
       const response = await supertest.get('/api/licensing/feature_usage').expect(200);
       const features = response.body.features.map(({ name }: FeatureUsage) => name);
 
+      // These were registered by the plugin in ../plugins/test_feature_usage
       expect(features).to.contain('test-client-A');
       expect(features).to.contain('test-client-B');
     });