diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts index 19646f0c39b89..8764eb434213e 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts @@ -6,10 +6,19 @@ */ import moment from 'moment'; -import { firstValueFrom } from 'rxjs'; import { fakeSchedulers } from 'rxjs-marbles/jest'; +import { firstValueFrom } from 'rxjs'; import { MetadataService } from './metadata_service'; +jest.mock('rxjs', () => { + const RxJs = jest.requireActual('rxjs'); + + return { + ...RxJs, + debounceTime: () => RxJs.identity, // Remove the delaying effect of debounceTime + }; +}); + describe('MetadataService', () => { jest.useFakeTimers(); @@ -26,9 +35,6 @@ describe('MetadataService', () => { describe('setup', () => { test('emits the initial metadata', async () => { - // Initially undefined - await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual(undefined); - const initialMetadata = { userId: 'fake-user-id', kibanaVersion: 'version' }; metadataService.setup(initialMetadata); await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual( @@ -39,11 +45,6 @@ describe('MetadataService', () => { test( 'emits in_trial when trial_end_date is provided', fakeSchedulers(async (advance) => { - // Initially undefined - await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual( - undefined - ); - const initialMetadata = { userId: 'fake-user-id', kibanaVersion: 'version', diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts index f4f918064d3bf..6691211b7b01c 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts @@ -5,7 +5,19 @@ * 2.0. */ -import { BehaviorSubject, exhaustMap, Subject, takeUntil, takeWhile, timer } from 'rxjs'; +import { + BehaviorSubject, + debounceTime, + distinct, + exhaustMap, + filter, + type Observable, + shareReplay, + Subject, + takeUntil, + takeWhile, + timer, +} from 'rxjs'; import { type Duration } from 'moment'; export interface MetadataServiceStartContract { @@ -28,13 +40,13 @@ export interface MetadataServiceConfig { } export class MetadataService { - public readonly userMetadata$ = new BehaviorSubject(undefined); + private readonly _userMetadata$ = new BehaviorSubject(undefined); private readonly stop$ = new Subject(); constructor(private readonly config: MetadataServiceConfig) {} public setup(initialUserMetadata: UserMetadata) { - this.userMetadata$.next(initialUserMetadata); + this._userMetadata$.next(initialUserMetadata); // Calculate `in_trial` based on the `trial_end_date`. // Elastic Cloud allows customers to end their trials earlier or even extend it in some cases, but this is a good compromise for now. @@ -48,9 +60,18 @@ export class MetadataService { } } + public get userMetadata$(): Observable { + return this._userMetadata$.pipe( + filter(Boolean), // Ensure we don't return undefined + debounceTime(100), // Swallows multiple emissions that may occur during bootstrap + distinct((meta) => [meta.in_trial, meta.has_data].join('-')), // Checks if any of the dynamic fields have changed + shareReplay(1) + ); + } + public start({ hasDataFetcher }: MetadataServiceStartContract) { // If no initial metadata (setup was not called) => it should not schedule any metadata extension - if (!this.userMetadata$.value) return; + if (!this._userMetadata$.value) return; this.scheduleUntil( async () => hasDataFetcher(), @@ -61,7 +82,7 @@ export class MetadataService { public stop() { this.stop$.next(); - this.userMetadata$.complete(); + this._userMetadata$.complete(); } /** @@ -78,13 +99,13 @@ export class MetadataService { .pipe( takeUntil(this.stop$), exhaustMap(async () => { - this.userMetadata$.next({ - ...this.userMetadata$.value!, // We are running the schedules after the initial user metadata is set + this._userMetadata$.next({ + ...this._userMetadata$.value!, // We are running the schedules after the initial user metadata is set ...(await fn()), }); }), takeWhile(() => { - return !untilFn(this.userMetadata$.value!); + return !untilFn(this._userMetadata$.value!); }) ) .subscribe(); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.ts index 2ce99449f0563..9c1e3d25537fd 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.ts @@ -13,8 +13,17 @@ import { CloudExperimentsPluginStart } from '../common'; import { FEATURE_FLAG_NAMES } from '../common/constants'; import { CloudExperimentsPlugin } from './plugin'; import { LaunchDarklyClient } from './launch_darkly_client'; +import { MetadataService } from '../common/metadata_service'; jest.mock('./launch_darkly_client'); +function getLaunchDarklyClientInstanceMock() { + const launchDarklyClientInstanceMock = ( + LaunchDarklyClient as jest.MockedClass + ).mock.instances[0] as jest.Mocked; + + return launchDarklyClientInstanceMock; +} + describe('Cloud Experiments public plugin', () => { jest.spyOn(console, 'debug').mockImplementation(); // silence console.debug logs @@ -33,6 +42,7 @@ describe('Cloud Experiments public plugin', () => { expect(plugin).toHaveProperty('stop'); expect(plugin).toHaveProperty('flagOverrides', undefined); expect(plugin).toHaveProperty('launchDarklyClient', undefined); + expect(plugin).toHaveProperty('metadataService', expect.any(MetadataService)); }); test('fails if launch_darkly is not provided in the config and it is a non-dev environment', () => { @@ -66,7 +76,7 @@ describe('Cloud Experiments public plugin', () => { describe('setup', () => { let plugin: CloudExperimentsPlugin; - let launchDarklyInstanceMock: jest.Mocked; + let metadataServiceSetupSpy: jest.SpyInstance; beforeEach(() => { const initializerContext = coreMock.createPluginInitializerContext({ @@ -75,8 +85,8 @@ describe('Cloud Experiments public plugin', () => { metadata_refresh_interval: duration(1, 'h'), }); plugin = new CloudExperimentsPlugin(initializerContext); - launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) - .mock.instances[0] as jest.Mocked; + // eslint-disable-next-line dot-notation + metadataServiceSetupSpy = jest.spyOn(plugin['metadataService'], 'setup'); }); afterEach(() => { @@ -109,16 +119,20 @@ describe('Cloud Experiments public plugin', () => { cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, }); - expect(launchDarklyInstanceMock.updateUserMetadata).not.toHaveBeenCalled(); + expect(metadataServiceSetupSpy).not.toHaveBeenCalled(); }); test('it initializes the LaunchDarkly client', async () => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); - // await the lazy import - await new Promise((resolve) => process.nextTick(resolve)); - expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledTimes(1); + + expect(metadataServiceSetupSpy).toHaveBeenCalledWith({ + is_elastic_staff_owned: true, + kibanaVersion: 'version', + trial_end_date: '2020-10-01T14:13:12.000Z', + userId: '1c2412b751f056aef6e340efa5637d137442d489a4b1e3117071e7c87f8523f2', + }); }); }); }); @@ -135,8 +149,7 @@ describe('Cloud Experiments public plugin', () => { flag_overrides: { [firstKnownFlag]: '1234' }, }); plugin = new CloudExperimentsPlugin(initializerContext); - launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) - .mock.instances[0] as jest.Mocked; + launchDarklyInstanceMock = getLaunchDarklyClientInstanceMock(); }); afterEach(() => { @@ -146,6 +159,7 @@ describe('Cloud Experiments public plugin', () => { test('returns the contract', () => { plugin.setup(coreMock.createSetup(), { cloud: cloudMock.createSetup() }); const startContract = plugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); expect(startContract).toStrictEqual( @@ -162,10 +176,10 @@ describe('Cloud Experiments public plugin', () => { }); const dataViews = dataViewPluginMocks.createStartContract(); - plugin.start(coreMock.createStart(), { dataViews }); + plugin.start(coreMock.createStart(), { cloud: cloudMock.createStart(), dataViews }); // After scheduler kicks in... - await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 200)); // Using a timeout of 0ms to let the `timer` kick in. // For some reason, fakeSchedulers is not working on browser-side tests :shrug: expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledWith( @@ -183,6 +197,7 @@ describe('Cloud Experiments public plugin', () => { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); startContract = plugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); }); @@ -221,6 +236,7 @@ describe('Cloud Experiments public plugin', () => { }); expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); startContract = customPlugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); }); @@ -252,6 +268,7 @@ describe('Cloud Experiments public plugin', () => { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); startContract = plugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); }); @@ -283,6 +300,7 @@ describe('Cloud Experiments public plugin', () => { }); expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); startContract = customPlugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); }); @@ -311,12 +329,12 @@ describe('Cloud Experiments public plugin', () => { metadata_refresh_interval: duration(1, 'h'), }); plugin = new CloudExperimentsPlugin(initializerContext); - launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) - .mock.instances[0] as jest.Mocked; + launchDarklyInstanceMock = getLaunchDarklyClientInstanceMock(); plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); plugin.start(coreMock.createStart(), { + cloud: cloudMock.createStart(), dataViews: dataViewPluginMocks.createStartContract(), }); }); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts index 4db1cbecca1fd..89a214000caba 100755 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts @@ -8,9 +8,9 @@ import type { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; import { get, has } from 'lodash'; import { duration } from 'moment'; -import { concatMap, filter } from 'rxjs'; +import { concatMap } from 'rxjs'; import { Sha256 } from '@kbn/crypto-browser'; -import type { CloudSetup } from '@kbn/cloud-plugin/public'; +import type { CloudSetup, CloudStart } from '@kbn/cloud-plugin/public'; import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public'; import { LaunchDarklyClient, type LaunchDarklyClientConfig } from './launch_darkly_client'; import type { @@ -26,6 +26,7 @@ interface CloudExperimentsPluginSetupDeps { } interface CloudExperimentsPluginStartDeps { + cloud: CloudStart; dataViews: DataViewsPublicPluginStart; } @@ -84,12 +85,26 @@ export class CloudExperimentsPlugin trial_end_date: deps.cloud.trialEndDate?.toISOString(), is_elastic_staff_owned: deps.cloud.isElasticStaffOwned, }); + } + } + + /** + * Returns the contract {@link CloudExperimentsPluginStart} + * @param core {@link CoreStart} + */ + public start( + core: CoreStart, + { cloud, dataViews }: CloudExperimentsPluginStartDeps + ): CloudExperimentsPluginStart { + if (cloud.isCloudEnabled) { + this.metadataService.start({ + hasDataFetcher: async () => ({ has_data: await dataViews.hasData.hasUserDataView() }), + }); // We only subscribe to the user metadata updates if Cloud is enabled. // This way, since the user is not identified, it cannot retrieve Feature Flags from LaunchDarkly when not running on Cloud. this.metadataService.userMetadata$ .pipe( - filter(Boolean), // Filter out undefined // Using concatMap to ensure we call the promised update in an orderly manner to avoid concurrency issues concatMap( async (userMetadata) => await this.launchDarklyClient!.updateUserMetadata(userMetadata) @@ -97,19 +112,6 @@ export class CloudExperimentsPlugin ) .subscribe(); // This subscription will stop on when the metadataService stops because it completes the Observable } - } - - /** - * Returns the contract {@link CloudExperimentsPluginStart} - * @param core {@link CoreStart} - */ - public start( - core: CoreStart, - { dataViews }: CloudExperimentsPluginStartDeps - ): CloudExperimentsPluginStart { - this.metadataService.start({ - hasDataFetcher: async () => ({ has_data: await dataViews.hasData.hasUserDataView() }), - }); return { getVariation: this.getVariation, reportMetric: this.reportMetric, diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.ts index e2f904aad2338..4d33e0ce65a7d 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.ts @@ -105,20 +105,24 @@ describe('Cloud Experiments server plugin', () => { expect(usageCollection.registerCollector).toHaveBeenCalledTimes(1); }); - test('updates the user metadata on setup', () => { - plugin.setup(coreMock.createSetup(), { - cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, - }); - const launchDarklyInstanceMock = ( - LaunchDarklyClient as jest.MockedClass - ).mock.instances[0]; - expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledWith({ - userId: '1c2412b751f056aef6e340efa5637d137442d489a4b1e3117071e7c87f8523f2', - kibanaVersion: coreMock.createPluginInitializerContext().env.packageInfo.version, - is_elastic_staff_owned: true, - trial_end_date: expect.any(String), - }); - }); + test( + 'updates the user metadata on setup', + fakeSchedulers((advance) => { + plugin.setup(coreMock.createSetup(), { + cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, + }); + const launchDarklyInstanceMock = ( + LaunchDarklyClient as jest.MockedClass + ).mock.instances[0]; + advance(100); // Remove the debounceTime effect + expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledWith({ + userId: '1c2412b751f056aef6e340efa5637d137442d489a4b1e3117071e7c87f8523f2', + kibanaVersion: coreMock.createPluginInitializerContext().env.packageInfo.version, + is_elastic_staff_owned: true, + trial_end_date: expect.any(String), + }); + }) + ); }); describe('start', () => { @@ -129,6 +133,7 @@ describe('Cloud Experiments server plugin', () => { const firstKnownFlag = Object.keys(FEATURE_FLAG_NAMES)[0] as keyof typeof FEATURE_FLAG_NAMES; beforeEach(() => { + jest.useRealTimers(); const initializerContext = coreMock.createPluginInitializerContext( config.schema.validate( { @@ -146,6 +151,7 @@ describe('Cloud Experiments server plugin', () => { afterEach(() => { plugin.stop(); + jest.useFakeTimers(); }); test('returns the contract', () => { @@ -159,26 +165,22 @@ describe('Cloud Experiments server plugin', () => { ); }); - test( - 'triggers a userMetadataUpdate for `has_data`', - fakeSchedulers(async (advance) => { - plugin.setup(coreMock.createSetup(), { - cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, - }); - dataViews.dataViewsServiceFactory.mockResolvedValue(dataViewsService); - dataViewsService.hasUserDataView.mockResolvedValue(true); - plugin.start(coreMock.createStart(), { dataViews }); - - // After scheduler kicks in... - advance(1); // The timer kicks in first on 0 (but let's give us 1ms so the trial is expired) - await new Promise((resolve) => process.nextTick(resolve)); // The timer triggers a promise, so we need to skip to the next tick - expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledWith( - expect.objectContaining({ - has_data: true, - }) - ); - }) - ); + test('triggers a userMetadataUpdate for `has_data`', async () => { + plugin.setup(coreMock.createSetup(), { + cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, + }); + dataViews.dataViewsServiceFactory.mockResolvedValue(dataViewsService); + dataViewsService.hasUserDataView.mockResolvedValue(true); + plugin.start(coreMock.createStart(), { dataViews }); + + // After scheduler kicks in... + await new Promise((resolve) => setTimeout(resolve, 200)); // Waiting for scheduler and debounceTime to complete (don't know why fakeScheduler didn't work here). + expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledWith( + expect.objectContaining({ + has_data: true, + }) + ); + }); describe('getVariation', () => { describe('with the client created', () => {