From 4aa4a9584322b8398259d1089affdc39aa0fea6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Thu, 6 Oct 2022 17:47:19 +0200 Subject: [PATCH 1/4] [LaunchDarkly] Add Deployment Metadata --- .../test_suites/core_plugins/rendering.ts | 4 + x-pack/plugins/cloud/README.md | 12 +- ...ud_deployment_id_analytics_context.test.ts | 10 +- ...r_cloud_deployment_id_analytics_context.ts | 33 ++- x-pack/plugins/cloud/public/mocks.tsx | 2 + x-pack/plugins/cloud/public/plugin.tsx | 57 ++++- .../collectors/cloud_usage_collector.test.ts | 48 ++-- .../collectors/cloud_usage_collector.ts | 13 +- x-pack/plugins/cloud/server/config.ts | 4 + x-pack/plugins/cloud/server/mocks.ts | 2 + x-pack/plugins/cloud/server/plugin.ts | 38 +++- .../common/metadata_service/index.ts | 8 + .../metadata_service/metadata_service.test.ts | 96 ++++++++ .../metadata_service/metadata_service.ts | 92 ++++++++ .../cloud_experiments/kibana.json | 2 +- .../public/launch_darkly_client/index.ts | 12 + .../launch_darkly_client.test.mock.ts} | 11 +- .../launch_darkly_client.test.ts | 168 ++++++++++++++ .../launch_darkly_client.ts | 79 +++++++ .../cloud_experiments/public/plugin.test.ts | 149 +++++++++---- .../cloud_experiments/public/plugin.ts | 88 +++++--- .../cloud_experiments/server/config.test.ts | 30 ++- .../cloud_experiments/server/config.ts | 3 + .../server/launch_darkly_client/index.ts | 8 + .../launch_darkly_client.test.mock.ts} | 1 - .../launch_darkly_client.test.ts | 211 ++++++++++++++++++ .../launch_darkly_client.ts | 104 +++++++++ .../server/launch_darkly_client/mocks.ts | 26 +++ .../cloud_experiments/server/plugin.test.ts | 201 +++++++++++------ .../cloud_experiments/server/plugin.ts | 90 +++++--- .../usage/register_usage_collector.test.ts | 41 +--- .../server/usage/register_usage_collector.ts | 19 +- .../cloud_experiments/tsconfig.json | 1 + .../schema/xpack_plugins.json | 9 + 34 files changed, 1405 insertions(+), 267 deletions(-) create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/index.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/index.ts rename x-pack/plugins/cloud_integrations/cloud_experiments/public/{plugin.test.mock.ts => launch_darkly_client/launch_darkly_client.test.mock.ts} (80%) create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/index.ts rename x-pack/plugins/cloud_integrations/cloud_experiments/server/{plugin.test.mock.ts => launch_darkly_client/launch_darkly_client.test.mock.ts} (97%) create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.ts create mode 100644 x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/mocks.ts diff --git a/test/plugin_functional/test_suites/core_plugins/rendering.ts b/test/plugin_functional/test_suites/core_plugins/rendering.ts index a3968330e1425..6477d54124e87 100644 --- a/test/plugin_functional/test_suites/core_plugins/rendering.ts +++ b/test/plugin_functional/test_suites/core_plugins/rendering.ts @@ -167,12 +167,16 @@ export default function ({ getService }: PluginFunctionalProviderContext) { 'xpack.cloud.base_url (string)', 'xpack.cloud.cname (string)', 'xpack.cloud.deployment_url (string)', + 'xpack.cloud.is_elastic_staff_owned (boolean)', + 'xpack.cloud.trial_end_date (string)', 'xpack.cloud_integrations.chat.chatURL (string)', // No PII. This is an escape patch to override LaunchDarkly's flag resolution mechanism for testing or quick fix. 'xpack.cloud_integrations.experiments.flag_overrides (record)', // Commented because it's inside a schema conditional, and the test is not able to resolve it. But it's shared. // Added here for documentation purposes. // 'xpack.cloud_integrations.experiments.launch_darkly.client_id (string)', + // 'xpack.cloud_integrations.experiments.launch_darkly.client_log_level (string)', + 'xpack.cloud_integrations.experiments.metadata_refresh_interval (duration)', 'xpack.cloud_integrations.full_story.org_id (any)', // No PII. Just the list of event types we want to forward to FullStory. 'xpack.cloud_integrations.full_story.eventTypesAllowlist (array)', diff --git a/x-pack/plugins/cloud/README.md b/x-pack/plugins/cloud/README.md index 77f73a3eaea01..0b9a75de7e030 100644 --- a/x-pack/plugins/cloud/README.md +++ b/x-pack/plugins/cloud/README.md @@ -52,4 +52,14 @@ This is the path to the Cloud Account and Billing page. The value is already pre This value is the same as `baseUrl` on ESS but can be customized on ECE. -**Example:** `cloud.elastic.co` (on ESS) \ No newline at end of file +**Example:** `cloud.elastic.co` (on ESS) + +### `trial_end_date` + +The end date for the Elastic Cloud trial. Only available on Elastic Cloud. + +**Example:** `2020-10-14T10:40:22Z` + +### `is_elastic_staff_owned` + +`true` if the deployment is owned by an Elastician. Only available on Elastic Cloud. \ No newline at end of file diff --git a/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.test.ts b/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.test.ts index a6dc1f59b00e3..4793bb1ac6af9 100644 --- a/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.test.ts +++ b/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.test.ts @@ -6,7 +6,7 @@ */ import { firstValueFrom } from 'rxjs'; -import { registerCloudDeploymentIdAnalyticsContext } from './register_cloud_deployment_id_analytics_context'; +import { registerCloudDeploymentMetadataAnalyticsContext } from './register_cloud_deployment_id_analytics_context'; describe('registerCloudDeploymentIdAnalyticsContext', () => { let analytics: { registerContextProvider: jest.Mock }; @@ -17,14 +17,16 @@ describe('registerCloudDeploymentIdAnalyticsContext', () => { }); test('it does not register the context provider if cloudId not provided', () => { - registerCloudDeploymentIdAnalyticsContext(analytics); + registerCloudDeploymentMetadataAnalyticsContext(analytics, {}); expect(analytics.registerContextProvider).not.toHaveBeenCalled(); }); test('it registers the context provider and emits the cloudId', async () => { - registerCloudDeploymentIdAnalyticsContext(analytics, 'cloud_id'); + registerCloudDeploymentMetadataAnalyticsContext(analytics, { id: 'cloud_id' }); expect(analytics.registerContextProvider).toHaveBeenCalledTimes(1); const [{ context$ }] = analytics.registerContextProvider.mock.calls[0]; - await expect(firstValueFrom(context$)).resolves.toEqual({ cloudId: 'cloud_id' }); + await expect(firstValueFrom(context$)).resolves.toEqual({ + cloudId: 'cloud_id', + }); }); }); diff --git a/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.ts b/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.ts index e8bdc6b37b50c..68130cdcda799 100644 --- a/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.ts +++ b/x-pack/plugins/cloud/common/register_cloud_deployment_id_analytics_context.ts @@ -8,21 +8,44 @@ import type { AnalyticsClient } from '@kbn/analytics-client'; import { of } from 'rxjs'; -export function registerCloudDeploymentIdAnalyticsContext( +export interface CloudDeploymentMetadata { + id?: string; + trial_end_date?: string; + is_elastic_staff_owned?: boolean; +} + +export function registerCloudDeploymentMetadataAnalyticsContext( analytics: Pick, - cloudId?: string + cloudMetadata: CloudDeploymentMetadata ) { - if (!cloudId) { + if (!cloudMetadata.id) { return; } + const { + id: cloudId, + trial_end_date: cloudTrialEndDate, + is_elastic_staff_owned: cloudIsElasticStaffOwned, + } = cloudMetadata; + analytics.registerContextProvider({ - name: 'Cloud Deployment ID', - context$: of({ cloudId }), + name: 'Cloud Deployment Metadata', + context$: of({ cloudId, cloudTrialEndDate, cloudIsElasticStaffOwned }), schema: { cloudId: { type: 'keyword', _meta: { description: 'The Cloud Deployment ID' }, }, + cloudTrialEndDate: { + type: 'date', + _meta: { description: 'When the Elastic Cloud trial ends/ended', optional: true }, + }, + cloudIsElasticStaffOwned: { + type: 'boolean', + _meta: { + description: '`true` if the owner of the deployment is an Elastician', + optional: true, + }, + }, }, }); } diff --git a/x-pack/plugins/cloud/public/mocks.tsx b/x-pack/plugins/cloud/public/mocks.tsx index 608e826657b73..fb1b66adcec98 100644 --- a/x-pack/plugins/cloud/public/mocks.tsx +++ b/x-pack/plugins/cloud/public/mocks.tsx @@ -18,6 +18,8 @@ function createSetupMock() { deploymentUrl: 'deployment-url', profileUrl: 'profile-url', organizationUrl: 'organization-url', + isElasticStaffOwned: true, + trialEndDate: new Date('2020-10-01T14:13:12Z'), registerCloudService: jest.fn(), }; } diff --git a/x-pack/plugins/cloud/public/plugin.tsx b/x-pack/plugins/cloud/public/plugin.tsx index f50f41f3c79cd..ded7924b50631 100644 --- a/x-pack/plugins/cloud/public/plugin.tsx +++ b/x-pack/plugins/cloud/public/plugin.tsx @@ -8,7 +8,7 @@ import React, { FC } from 'react'; import type { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; -import { registerCloudDeploymentIdAnalyticsContext } from '../common/register_cloud_deployment_id_analytics_context'; +import { registerCloudDeploymentMetadataAnalyticsContext } from '../common/register_cloud_deployment_id_analytics_context'; import { getIsCloudEnabled } from '../common/is_cloud_enabled'; import { ELASTIC_SUPPORT_LINK, CLOUD_SNAPSHOTS_PATH } from '../common/constants'; import { getFullCloudUrl } from './utils'; @@ -20,11 +20,8 @@ export interface CloudConfigType { profile_url?: string; deployment_url?: string; organization_url?: string; - full_story: { - enabled: boolean; - org_id?: string; - eventTypesAllowlist?: string[]; - }; + trial_end_date?: string; + is_elastic_staff_owned?: boolean; } export interface CloudStart { @@ -55,14 +52,50 @@ export interface CloudStart { } export interface CloudSetup { + /** + * Cloud ID. Undefined if not running on Cloud. + */ cloudId?: string; + /** + * This value is the same as `baseUrl` on ESS but can be customized on ECE. + */ cname?: string; + /** + * This is the URL of the Cloud interface. + */ baseUrl?: string; + /** + * The full URL to the deployment management page on Elastic Cloud. Undefined if not running on Cloud. + */ deploymentUrl?: string; + /** + * The full URL to the user profile page on Elastic Cloud. Undefined if not running on Cloud. + */ profileUrl?: string; + /** + * The full URL to the organization management page on Elastic Cloud. Undefined if not running on Cloud. + */ organizationUrl?: string; + /** + * This is the path to the Snapshots page for the deployment to which the Kibana instance belongs. The value is already prepended with `deploymentUrl`. + */ snapshotsUrl?: string; + /** + * `true` when Kibana is running on Elastic Cloud. + */ isCloudEnabled: boolean; + /** + * When the Cloud Trial ends/ended for the organization that owns this deployment. Only available when running on Elastic Cloud. + */ + trialEndDate?: Date; + /** + * `true` if the Elastic Cloud organization that owns this deployment is owned by an Elastician. Only available when running on Elastic Cloud. + */ + isElasticStaffOwned?: boolean; + /** + * Registers CloudServiceProviders so start's `CloudContextProvider` hooks them. + * @param contextProvider The React component from the Service Provider. + */ registerCloudService: (contextProvider: FC) => void; } @@ -84,15 +117,23 @@ export class CloudPlugin implements Plugin { } public setup(core: CoreSetup): CloudSetup { - registerCloudDeploymentIdAnalyticsContext(core.analytics, this.config.id); + registerCloudDeploymentMetadataAnalyticsContext(core.analytics, this.config); - const { id, cname, base_url: baseUrl } = this.config; + const { + id, + cname, + base_url: baseUrl, + trial_end_date: trialEndDate, + is_elastic_staff_owned: isElasticStaffOwned, + } = this.config; return { cloudId: id, cname, baseUrl, ...this.getCloudUrls(), + trialEndDate: trialEndDate ? new Date(trialEndDate) : undefined, + isElasticStaffOwned, isCloudEnabled: this.isCloudEnabled, registerCloudService: (contextProvider) => { this.contextProviders.push(contextProvider); diff --git a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.test.ts b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.test.ts index f9fdef5319d59..612b75e5d68d3 100644 --- a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.test.ts +++ b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.test.ts @@ -5,31 +5,51 @@ * 2.0. */ +import { + createCollectorFetchContextMock, + usageCollectionPluginMock, +} from '@kbn/usage-collection-plugin/server/mocks'; +import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; import { createCloudUsageCollector } from './cloud_usage_collector'; -import { createCollectorFetchContextMock } from '@kbn/usage-collection-plugin/server/mocks'; +import { CollectorFetchContext } from '@kbn/usage-collection-plugin/server'; -const mockUsageCollection = () => ({ - makeUsageCollector: jest.fn().mockImplementation((args: any) => ({ ...args })), -}); +describe('createCloudUsageCollector', () => { + let usageCollection: UsageCollectionSetup; + let collectorFetchContext: jest.Mocked; -const getMockConfigs = (isCloudEnabled: boolean) => ({ isCloudEnabled }); + beforeEach(() => { + usageCollection = usageCollectionPluginMock.createSetupContract(); + collectorFetchContext = createCollectorFetchContextMock(); + }); -describe('createCloudUsageCollector', () => { it('calls `makeUsageCollector`', () => { - const mockConfigs = getMockConfigs(false); - const usageCollection = mockUsageCollection(); - createCloudUsageCollector(usageCollection as any, mockConfigs); + createCloudUsageCollector(usageCollection, { isCloudEnabled: false }); expect(usageCollection.makeUsageCollector).toBeCalledTimes(1); }); describe('Fetched Usage data', () => { it('return isCloudEnabled boolean', async () => { - const mockConfigs = getMockConfigs(true); - const usageCollection = mockUsageCollection() as any; - const collector = createCloudUsageCollector(usageCollection, mockConfigs); - const collectorFetchContext = createCollectorFetchContextMock(); + const collector = createCloudUsageCollector(usageCollection, { isCloudEnabled: true }); + + expect(await collector.fetch(collectorFetchContext)).toStrictEqual({ + isCloudEnabled: true, + isElasticStaffOwned: undefined, + trialEndDate: undefined, + }); + }); + + it('return inTrial boolean if trialEndDateIsProvided', async () => { + const collector = createCloudUsageCollector(usageCollection, { + isCloudEnabled: true, + trialEndDate: '2020-10-01T14:30:16Z', + }); - expect((await collector.fetch(collectorFetchContext)).isCloudEnabled).toBe(true); // Adding the await because the fetch can be a Promise or a synchronous method and TS complains in the test if not awaited + expect(await collector.fetch(collectorFetchContext)).toStrictEqual({ + isCloudEnabled: true, + isElasticStaffOwned: undefined, + trialEndDate: '2020-10-01T14:30:16Z', + inTrial: false, + }); }); }); }); diff --git a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts index 28f6e8c94d0b8..b107099133e20 100644 --- a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts +++ b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts @@ -9,23 +9,34 @@ import { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; interface Config { isCloudEnabled: boolean; + trialEndDate?: string; + isElasticStaffOwned?: boolean; } interface CloudUsage { isCloudEnabled: boolean; + trialEndDate?: string; + inTrial?: boolean; + isElasticStaffOwned?: boolean; } export function createCloudUsageCollector(usageCollection: UsageCollectionSetup, config: Config) { - const { isCloudEnabled } = config; + const { isCloudEnabled, trialEndDate, isElasticStaffOwned } = config; return usageCollection.makeUsageCollector({ type: 'cloud', isReady: () => true, schema: { isCloudEnabled: { type: 'boolean' }, + trialEndDate: { type: 'date' }, + inTrial: { type: 'boolean' }, + isElasticStaffOwned: { type: 'boolean' }, }, fetch: () => { return { isCloudEnabled, + isElasticStaffOwned, + trialEndDate, + ...(trialEndDate ? { inTrial: Date.now() <= new Date(trialEndDate).getTime() } : {}), }; }, }); diff --git a/x-pack/plugins/cloud/server/config.ts b/x-pack/plugins/cloud/server/config.ts index 512542c756798..028298c2da331 100644 --- a/x-pack/plugins/cloud/server/config.ts +++ b/x-pack/plugins/cloud/server/config.ts @@ -26,6 +26,8 @@ const configSchema = schema.object({ id: schema.maybe(schema.string()), organization_url: schema.maybe(schema.string()), profile_url: schema.maybe(schema.string()), + trial_end_date: schema.maybe(schema.string()), + is_elastic_staff_owned: schema.maybe(schema.boolean()), }); export type CloudConfigType = TypeOf; @@ -38,6 +40,8 @@ export const config: PluginConfigDescriptor = { id: true, organization_url: true, profile_url: true, + trial_end_date: true, + is_elastic_staff_owned: true, }, schema: configSchema, }; diff --git a/x-pack/plugins/cloud/server/mocks.ts b/x-pack/plugins/cloud/server/mocks.ts index 557e64edf6cc1..ad64768951450 100644 --- a/x-pack/plugins/cloud/server/mocks.ts +++ b/x-pack/plugins/cloud/server/mocks.ts @@ -13,6 +13,8 @@ function createSetupMock(): jest.Mocked { instanceSizeMb: 1234, deploymentId: 'deployment-id', isCloudEnabled: true, + isElasticStaffOwned: true, + trialEndDate: new Date('2020-10-01T14:13:12Z'), apm: { url: undefined, secretToken: undefined, diff --git a/x-pack/plugins/cloud/server/plugin.ts b/x-pack/plugins/cloud/server/plugin.ts index 9cf1a308800a0..4e54c2b9b7f40 100644 --- a/x-pack/plugins/cloud/server/plugin.ts +++ b/x-pack/plugins/cloud/server/plugin.ts @@ -7,7 +7,7 @@ import type { CoreSetup, Plugin, PluginInitializerContext } from '@kbn/core/server'; import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; -import { registerCloudDeploymentIdAnalyticsContext } from '../common/register_cloud_deployment_id_analytics_context'; +import { registerCloudDeploymentMetadataAnalyticsContext } from '../common/register_cloud_deployment_id_analytics_context'; import type { CloudConfigType } from './config'; import { registerCloudUsageCollector } from './collectors'; import { getIsCloudEnabled } from '../common/is_cloud_enabled'; @@ -18,11 +18,37 @@ interface PluginsSetup { usageCollection?: UsageCollectionSetup; } +/** + * Setup contract + */ export interface CloudSetup { + /** + * The deployment's Cloud ID. Only available when running on Elastic Cloud. + */ cloudId?: string; + /** + * The deployment's ID. Only available when running on Elastic Cloud. + */ deploymentId?: string; + /** + * `true` when running on Elastic Cloud. + */ isCloudEnabled: boolean; + /** + * The size of the instance in which Kibana is running. Only available when running on Elastic Cloud. + */ instanceSizeMb?: number; + /** + * When the Cloud Trial ends/ended for the organization that owns this deployment. Only available when running on Elastic Cloud. + */ + trialEndDate?: Date; + /** + * `true` if the Elastic Cloud organization that owns this deployment is owned by an Elastician. Only available when running on Elastic Cloud. + */ + isElasticStaffOwned?: boolean; + /** + * APM configuration keys. + */ apm: { url?: string; secretToken?: string; @@ -38,14 +64,20 @@ export class CloudPlugin implements Plugin { public setup(core: CoreSetup, { usageCollection }: PluginsSetup): CloudSetup { const isCloudEnabled = getIsCloudEnabled(this.config.id); - registerCloudDeploymentIdAnalyticsContext(core.analytics, this.config.id); - registerCloudUsageCollector(usageCollection, { isCloudEnabled }); + registerCloudDeploymentMetadataAnalyticsContext(core.analytics, this.config); + registerCloudUsageCollector(usageCollection, { + isCloudEnabled, + trialEndDate: this.config.trial_end_date, + isElasticStaffOwned: this.config.is_elastic_staff_owned, + }); return { cloudId: this.config.id, instanceSizeMb: readInstanceSizeMb(), deploymentId: parseDeploymentIdFromDeploymentUrl(this.config.deployment_url), isCloudEnabled, + trialEndDate: this.config.trial_end_date ? new Date(this.config.trial_end_date) : undefined, + isElasticStaffOwned: this.config.is_elastic_staff_owned, apm: { url: this.config.apm?.url, secretToken: this.config.apm?.secret_token, diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/index.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/index.ts new file mode 100644 index 0000000000000..74e2655e8302f --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/index.ts @@ -0,0 +1,8 @@ +/* + * 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 { MetadataService } from './metadata_service'; 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 new file mode 100644 index 0000000000000..19646f0c39b89 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts @@ -0,0 +1,96 @@ +/* + * 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 moment from 'moment'; +import { firstValueFrom } from 'rxjs'; +import { fakeSchedulers } from 'rxjs-marbles/jest'; +import { MetadataService } from './metadata_service'; + +describe('MetadataService', () => { + jest.useFakeTimers(); + + let metadataService: MetadataService; + + beforeEach(() => { + metadataService = new MetadataService({ metadata_refresh_interval: moment.duration(1, 's') }); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.clearAllMocks(); + }); + + 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( + initialMetadata + ); + }); + + 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', + trial_end_date: new Date(0).toISOString(), + }; + metadataService.setup(initialMetadata); + + // Still equals initialMetadata + await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual( + initialMetadata + ); + + // 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 + await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual({ + ...initialMetadata, + in_trial: false, + }); + }) + ); + }); + + describe('start', () => { + const initialMetadata = { userId: 'fake-user-id', kibanaVersion: 'version' }; + beforeEach(() => { + metadataService.setup(initialMetadata); + }); + + test( + 'emits has_data after resolving the `hasUserDataView`', + fakeSchedulers(async (advance) => { + metadataService.start({ hasDataFetcher: async () => ({ has_data: true }) }); + + // Still equals initialMetadata + await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual( + initialMetadata + ); + + // 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 + await expect(firstValueFrom(metadataService.userMetadata$)).resolves.toStrictEqual({ + ...initialMetadata, + has_data: true, + }); + }) + ); + }); +}); 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 new file mode 100644 index 0000000000000..f4f918064d3bf --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts @@ -0,0 +1,92 @@ +/* + * 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 { BehaviorSubject, exhaustMap, Subject, takeUntil, takeWhile, timer } from 'rxjs'; +import { type Duration } from 'moment'; + +export interface MetadataServiceStartContract { + hasDataFetcher: () => Promise<{ has_data: boolean }>; +} + +export interface UserMetadata extends Record { + // Static values + userId: string; + kibanaVersion: string; + trial_end_date?: string; + is_elastic_staff_owned?: boolean; + // Dynamic/calculated values + in_trial?: boolean; + has_data?: boolean; +} + +export interface MetadataServiceConfig { + metadata_refresh_interval: Duration; +} + +export class MetadataService { + public readonly userMetadata$ = new BehaviorSubject(undefined); + private readonly stop$ = new Subject(); + + constructor(private readonly config: MetadataServiceConfig) {} + + public setup(initialUserMetadata: UserMetadata) { + 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. + const trialEndDate = initialUserMetadata.trial_end_date; + if (trialEndDate) { + this.scheduleUntil( + () => ({ in_trial: Date.now() <= new Date(trialEndDate).getTime() }), + // Stop recalculating in_trial when the user is no-longer in trial + (metadata) => metadata.in_trial === false + ); + } + } + + public start({ hasDataFetcher }: MetadataServiceStartContract) { + // If no initial metadata (setup was not called) => it should not schedule any metadata extension + if (!this.userMetadata$.value) return; + + this.scheduleUntil( + async () => hasDataFetcher(), + // Stop checking the moment the user has any data + (metadata) => metadata.has_data === true + ); + } + + public stop() { + this.stop$.next(); + this.userMetadata$.complete(); + } + + /** + * Schedules a timer that calls `fn` to update the {@link UserMetadata} until `untilFn` returns true. + * @param fn Method to calculate the dynamic metadata. + * @param untilFn Method that returns true when the scheduler should stop calling fn (potentially because the dynamic value is not expected to change anymore). + * @private + */ + private scheduleUntil( + fn: () => Partial | Promise>, + untilFn: (value: UserMetadata) => boolean + ) { + timer(0, this.config.metadata_refresh_interval.asMilliseconds()) + .pipe( + takeUntil(this.stop$), + exhaustMap(async () => { + 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!); + }) + ) + .subscribe(); + } +} diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/kibana.json b/x-pack/plugins/cloud_integrations/cloud_experiments/kibana.json index 6bbb41f796f94..6dc3e8fe34c87 100755 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/kibana.json +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/kibana.json @@ -10,6 +10,6 @@ "server": true, "ui": true, "configPath": ["xpack", "cloud_integrations", "experiments"], - "requiredPlugins": ["cloud"], + "requiredPlugins": ["cloud", "dataViews"], "optionalPlugins": ["usageCollection"] } diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/index.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/index.ts new file mode 100644 index 0000000000000..ac961286b7043 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/index.ts @@ -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. + */ + +export { + LaunchDarklyClient, + type LaunchDarklyUserMetadata, + type LaunchDarklyClientConfig, +} from './launch_darkly_client'; diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.mock.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.mock.ts similarity index 80% rename from x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.mock.ts rename to x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.mock.ts index d2bfb5b54213d..b6a43a7d0715b 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.mock.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.mock.ts @@ -9,16 +9,19 @@ import type { LDClient } from 'launchdarkly-js-client-sdk'; export function createLaunchDarklyClientMock(): jest.Mocked { return { + identify: jest.fn(), waitForInitialization: jest.fn(), variation: jest.fn(), track: jest.fn(), - identify: jest.fn(), flush: jest.fn(), } as unknown as jest.Mocked; // Using casting because we only use these APIs. No need to declare everything. } export const ldClientMock = createLaunchDarklyClientMock(); -jest.doMock('launchdarkly-js-client-sdk', () => ({ - initialize: () => ldClientMock, -})); +export const launchDarklyLibraryMock = { + initialize: jest.fn(), + basicLogger: jest.fn(), +}; + +jest.doMock('launchdarkly-js-client-sdk', () => launchDarklyLibraryMock); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.ts new file mode 100644 index 0000000000000..8f4b0d63c9947 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.test.ts @@ -0,0 +1,168 @@ +/* + * 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 { ldClientMock, launchDarklyLibraryMock } from './launch_darkly_client.test.mock'; +import { LaunchDarklyClient, type LaunchDarklyClientConfig } from './launch_darkly_client'; + +describe('LaunchDarklyClient - browser', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const config: LaunchDarklyClientConfig = { + client_id: 'fake-client-id', + client_log_level: 'debug', + }; + + describe('Public APIs', () => { + let client: LaunchDarklyClient; + const testUserMetadata = { userId: 'fake-user-id', kibanaVersion: 'version' }; + + beforeEach(() => { + client = new LaunchDarklyClient(config, 'version'); + }); + + describe('updateUserMetadata', () => { + test("calls the client's initialize method with all the possible values", async () => { + expect(client).toHaveProperty('launchDarklyClient', undefined); + + launchDarklyLibraryMock.initialize.mockReturnValue(ldClientMock); + + const topFields = { + name: 'First Last', + firstName: 'First', + lastName: 'Last', + email: 'first.last@boring.co', + avatar: 'fake-blue-avatar', + ip: 'my-weird-ip', + country: 'distributed', + }; + + const extraFields = { + other_field: 'my other custom field', + kibanaVersion: 'version', + }; + + await client.updateUserMetadata({ userId: 'fake-user-id', ...topFields, ...extraFields }); + + expect(launchDarklyLibraryMock.initialize).toHaveBeenCalledWith( + 'fake-client-id', + { + key: 'fake-user-id', + ...topFields, + custom: extraFields, + }, + { + application: { id: 'kibana-browser', version: 'version' }, + logger: undefined, + } + ); + + expect(client).toHaveProperty('launchDarklyClient', ldClientMock); + }); + + test('sets a minimum amount of info', async () => { + expect(client).toHaveProperty('launchDarklyClient', undefined); + + await client.updateUserMetadata({ userId: 'fake-user-id', kibanaVersion: 'version' }); + + expect(launchDarklyLibraryMock.initialize).toHaveBeenCalledWith( + 'fake-client-id', + { + key: 'fake-user-id', + custom: { kibanaVersion: 'version' }, + }, + { + application: { id: 'kibana-browser', version: 'version' }, + logger: undefined, + } + ); + }); + + test('calls identify if an update comes after initializing the client', async () => { + expect(client).toHaveProperty('launchDarklyClient', undefined); + + launchDarklyLibraryMock.initialize.mockReturnValue(ldClientMock); + await client.updateUserMetadata({ userId: 'fake-user-id', kibanaVersion: 'version' }); + + expect(launchDarklyLibraryMock.initialize).toHaveBeenCalledWith( + 'fake-client-id', + { + key: 'fake-user-id', + custom: { kibanaVersion: 'version' }, + }, + { + application: { id: 'kibana-browser', version: 'version' }, + logger: undefined, + } + ); + expect(ldClientMock.identify).not.toHaveBeenCalled(); + + expect(client).toHaveProperty('launchDarklyClient', ldClientMock); + + // Update user metadata a 2nd time + await client.updateUserMetadata({ userId: 'fake-user-id', kibanaVersion: 'version' }); + expect(ldClientMock.identify).toHaveBeenCalledWith({ + key: 'fake-user-id', + custom: { kibanaVersion: 'version' }, + }); + }); + }); + + describe('getVariation', () => { + test('returns the default value if the user has not been defined', async () => { + await expect(client.getVariation('my-feature-flag', 123)).resolves.toStrictEqual(123); + expect(ldClientMock.variation).toHaveBeenCalledTimes(0); + }); + + test('calls the LaunchDarkly client when the user has been defined', async () => { + ldClientMock.variation.mockResolvedValue(1234); + await client.updateUserMetadata(testUserMetadata); + await expect(client.getVariation('my-feature-flag', 123)).resolves.toStrictEqual(1234); + expect(ldClientMock.variation).toHaveBeenCalledTimes(1); + expect(ldClientMock.variation).toHaveBeenCalledWith('my-feature-flag', 123); + }); + }); + + describe('reportMetric', () => { + test('does not call track if the user has not been defined', () => { + client.reportMetric('my-feature-flag', {}, 123); + expect(ldClientMock.track).toHaveBeenCalledTimes(0); + }); + + test('calls the LaunchDarkly client when the user has been defined', async () => { + await client.updateUserMetadata(testUserMetadata); + client.reportMetric('my-feature-flag', {}, 123); + expect(ldClientMock.track).toHaveBeenCalledTimes(1); + expect(ldClientMock.track).toHaveBeenCalledWith('my-feature-flag', {}, 123); + }); + }); + + describe('stop', () => { + test('flushes the events', async () => { + await client.updateUserMetadata(testUserMetadata); + + ldClientMock.flush.mockResolvedValue(); + expect(() => client.stop()).not.toThrow(); + expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the flush resolution + }); + + test('handles errors when flushing events', async () => { + await client.updateUserMetadata(testUserMetadata); + + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + const err = new Error('Something went terribly wrong'); + ldClientMock.flush.mockRejectedValue(err); + expect(() => client.stop()).not.toThrow(); + expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the flush resolution + expect(consoleWarnSpy).toHaveBeenCalledWith(err); + }); + }); + }); +}); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.ts new file mode 100644 index 0000000000000..f78286f0fa8ca --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/launch_darkly_client/launch_darkly_client.ts @@ -0,0 +1,79 @@ +/* + * 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 { type LDClient, type LDUser, type LDLogLevel } from 'launchdarkly-js-client-sdk'; + +export interface LaunchDarklyClientConfig { + client_id: string; + client_log_level: LDLogLevel; +} + +export interface LaunchDarklyUserMetadata + extends Record { + userId: string; + // We are not collecting any of the above, but this is to match the LDUser first-level definition + name?: string; + firstName?: string; + lastName?: string; + email?: string; + avatar?: string; + ip?: string; + country?: string; +} + +export class LaunchDarklyClient { + private launchDarklyClient?: LDClient; + + constructor( + private readonly ldConfig: LaunchDarklyClientConfig, + private readonly kibanaVersion: string + ) {} + + public async updateUserMetadata(userMetadata: LaunchDarklyUserMetadata) { + const { userId, name, firstName, lastName, email, avatar, ip, country, ...custom } = + userMetadata; + const launchDarklyUser: LDUser = { + key: userId, + name, + firstName, + lastName, + email, + avatar, + ip, + country, + // This casting is needed because LDUser does not allow `Record` + custom: custom as Record, + }; + if (this.launchDarklyClient) { + await this.launchDarklyClient.identify(launchDarklyUser); + } else { + const { initialize, basicLogger } = await import('launchdarkly-js-client-sdk'); + this.launchDarklyClient = initialize(this.ldConfig.client_id, launchDarklyUser, { + application: { id: 'kibana-browser', version: this.kibanaVersion }, + logger: basicLogger({ level: this.ldConfig.client_log_level }), + }); + } + } + + public async getVariation(configKey: string, defaultValue: Data): Promise { + if (!this.launchDarklyClient) return defaultValue; // Skip any action if no LD User is defined + await this.launchDarklyClient.waitForInitialization(); + return await this.launchDarklyClient.variation(configKey, defaultValue); + } + + public reportMetric(metricName: string, meta?: unknown, value?: number): void { + if (!this.launchDarklyClient) return; // Skip any action if no LD User is defined + this.launchDarklyClient.track(metricName, meta, value); + } + + public stop() { + this.launchDarklyClient + ?.flush() + // eslint-disable-next-line no-console + .catch((err) => console.warn(err)); + } +} 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 3c6396d686796..2ce99449f0563 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 @@ -5,17 +5,21 @@ * 2.0. */ +import { duration } from 'moment'; import { coreMock } from '@kbn/core/public/mocks'; import { cloudMock } from '@kbn/cloud-plugin/public/mocks'; -import { ldClientMock } from './plugin.test.mock'; -import { CloudExperimentsPlugin } from './plugin'; +import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks'; +import { CloudExperimentsPluginStart } from '../common'; import { FEATURE_FLAG_NAMES } from '../common/constants'; +import { CloudExperimentsPlugin } from './plugin'; +import { LaunchDarklyClient } from './launch_darkly_client'; +jest.mock('./launch_darkly_client'); describe('Cloud Experiments public plugin', () => { jest.spyOn(console, 'debug').mockImplementation(); // silence console.debug logs - beforeEach(() => { - jest.resetAllMocks(); + afterEach(() => { + jest.clearAllMocks(); }); describe('constructor', () => { @@ -49,17 +53,34 @@ describe('Cloud Experiments public plugin', () => { const plugin = new CloudExperimentsPlugin(initializerContext); expect(plugin).toHaveProperty('flagOverrides', { my_flag: '1234' }); }); + + test('it initializes the LaunchDarkly client', () => { + const initializerContext = coreMock.createPluginInitializerContext({ + launch_darkly: { client_id: 'sdk-1234' }, + }); + const plugin = new CloudExperimentsPlugin(initializerContext); + expect(LaunchDarklyClient).toHaveBeenCalledTimes(1); + expect(plugin).toHaveProperty('launchDarklyClient', expect.any(LaunchDarklyClient)); + }); }); describe('setup', () => { let plugin: CloudExperimentsPlugin; + let launchDarklyInstanceMock: jest.Mocked; beforeEach(() => { const initializerContext = coreMock.createPluginInitializerContext({ launch_darkly: { client_id: '1234' }, flag_overrides: { my_flag: '1234' }, + metadata_refresh_interval: duration(1, 'h'), }); plugin = new CloudExperimentsPlugin(initializerContext); + launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) + .mock.instances[0] as jest.Mocked; + }); + + afterEach(() => { + plugin.stop(); }); test('returns no contract', () => { @@ -74,37 +95,37 @@ describe('Cloud Experiments public plugin', () => { test('it skips creating the client if no client id provided in the config', () => { const initializerContext = coreMock.createPluginInitializerContext({ flag_overrides: { my_flag: '1234' }, + metadata_refresh_interval: duration(1, 'h'), }); const customPlugin = new CloudExperimentsPlugin(initializerContext); - expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); customPlugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); }); - test('it skips creating the client if cloud is not enabled', () => { - expect(plugin).toHaveProperty('launchDarklyClient', undefined); + test('it skips identifying the user if cloud is not enabled', () => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, }); - expect(plugin).toHaveProperty('launchDarklyClient', undefined); + + expect(launchDarklyInstanceMock.updateUserMetadata).not.toHaveBeenCalled(); }); test('it initializes the LaunchDarkly client', async () => { - expect(plugin).toHaveProperty('launchDarklyClient', undefined); plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); // await the lazy import await new Promise((resolve) => process.nextTick(resolve)); - expect(plugin).toHaveProperty('launchDarklyClient', ldClientMock); + expect(launchDarklyInstanceMock.updateUserMetadata).toHaveBeenCalledTimes(1); }); }); }); describe('start', () => { let plugin: CloudExperimentsPlugin; + let launchDarklyInstanceMock: jest.Mocked; const firstKnownFlag = Object.keys(FEATURE_FLAG_NAMES)[0] as keyof typeof FEATURE_FLAG_NAMES; @@ -114,11 +135,19 @@ describe('Cloud Experiments public plugin', () => { flag_overrides: { [firstKnownFlag]: '1234' }, }); plugin = new CloudExperimentsPlugin(initializerContext); + launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) + .mock.instances[0] as jest.Mocked; + }); + + afterEach(() => { + plugin.stop(); }); test('returns the contract', () => { plugin.setup(coreMock.createSetup(), { cloud: cloudMock.createSetup() }); - const startContract = plugin.start(coreMock.createStart()); + const startContract = plugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), + }); expect(startContract).toStrictEqual( expect.objectContaining({ getVariation: expect.any(Function), @@ -127,24 +156,45 @@ describe('Cloud Experiments public plugin', () => { ); }); + test('triggers a userMetadataUpdate for `has_data`', async () => { + plugin.setup(coreMock.createSetup(), { + cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, + }); + + const dataViews = dataViewPluginMocks.createStartContract(); + plugin.start(coreMock.createStart(), { dataViews }); + + // After scheduler kicks in... + await new Promise((resolve) => setTimeout(resolve, 0)); + // 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( + expect.objectContaining({ + has_data: true, + }) + ); + }); + describe('getVariation', () => { - describe('with the user identified', () => { + let startContract: CloudExperimentsPluginStart; + describe('with the client created', () => { beforeEach(() => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); + startContract = plugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), + }); }); test('uses the flag overrides to respond early', async () => { - const startContract = plugin.start(coreMock.createStart()); await expect(startContract.getVariation(firstKnownFlag, 123)).resolves.toStrictEqual( '1234' ); }); test('calls the client', async () => { - const startContract = plugin.start(coreMock.createStart()); - ldClientMock.variation.mockReturnValue('12345'); + launchDarklyInstanceMock.getVariation.mockResolvedValue('12345'); await expect( startContract.getVariation( // @ts-expect-error We only allow existing flags in FEATURE_FLAG_NAMES @@ -152,29 +202,36 @@ describe('Cloud Experiments public plugin', () => { 123 ) ).resolves.toStrictEqual('12345'); - expect(ldClientMock.variation).toHaveBeenCalledWith( + expect(launchDarklyInstanceMock.getVariation).toHaveBeenCalledWith( undefined, // it couldn't find it in FEATURE_FLAG_NAMES 123 ); }); }); - describe('with the user not identified', () => { + describe('with the client not created', () => { beforeEach(() => { - plugin.setup(coreMock.createSetup(), { - cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, + const initializerContext = coreMock.createPluginInitializerContext({ + flag_overrides: { [firstKnownFlag]: '1234' }, + metadata_refresh_interval: duration(1, 'h'), + }); + const customPlugin = new CloudExperimentsPlugin(initializerContext); + customPlugin.setup(coreMock.createSetup(), { + cloud: cloudMock.createSetup(), + }); + expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); + startContract = customPlugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), }); }); test('uses the flag overrides to respond early', async () => { - const startContract = plugin.start(coreMock.createStart()); await expect(startContract.getVariation(firstKnownFlag, 123)).resolves.toStrictEqual( '1234' ); }); test('returns the default value without calling the client', async () => { - const startContract = plugin.start(coreMock.createStart()); await expect( startContract.getVariation( // @ts-expect-error We only allow existing flags in FEATURE_FLAG_NAMES @@ -182,28 +239,31 @@ describe('Cloud Experiments public plugin', () => { 123 ) ).resolves.toStrictEqual(123); - expect(ldClientMock.variation).not.toHaveBeenCalled(); + expect(launchDarklyInstanceMock.getVariation).not.toHaveBeenCalled(); }); }); }); describe('reportMetric', () => { - describe('with the user identified', () => { + let startContract: CloudExperimentsPluginStart; + describe('with the client created', () => { beforeEach(() => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); + startContract = plugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), + }); }); test('calls the track API', () => { - const startContract = plugin.start(coreMock.createStart()); startContract.reportMetric({ // @ts-expect-error We only allow existing flags in METRIC_NAMES name: 'my-flag', meta: {}, value: 1, }); - expect(ldClientMock.track).toHaveBeenCalledWith( + expect(launchDarklyInstanceMock.reportMetric).toHaveBeenCalledWith( undefined, // it couldn't find it in METRIC_NAMES {}, 1 @@ -211,22 +271,30 @@ describe('Cloud Experiments public plugin', () => { }); }); - describe('with the user not identified', () => { + describe('with the client not created', () => { beforeEach(() => { - plugin.setup(coreMock.createSetup(), { - cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, + const initializerContext = coreMock.createPluginInitializerContext({ + flag_overrides: { [firstKnownFlag]: '1234' }, + metadata_refresh_interval: duration(1, 'h'), + }); + const customPlugin = new CloudExperimentsPlugin(initializerContext); + customPlugin.setup(coreMock.createSetup(), { + cloud: cloudMock.createSetup(), + }); + expect(customPlugin).toHaveProperty('launchDarklyClient', undefined); + startContract = customPlugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), }); }); test('calls the track API', () => { - const startContract = plugin.start(coreMock.createStart()); startContract.reportMetric({ // @ts-expect-error We only allow existing flags in METRIC_NAMES name: 'my-flag', meta: {}, value: 1, }); - expect(ldClientMock.track).not.toHaveBeenCalled(); + expect(launchDarklyInstanceMock.reportMetric).not.toHaveBeenCalled(); }); }); }); @@ -234,33 +302,28 @@ describe('Cloud Experiments public plugin', () => { describe('stop', () => { let plugin: CloudExperimentsPlugin; + let launchDarklyInstanceMock: jest.Mocked; beforeEach(() => { const initializerContext = coreMock.createPluginInitializerContext({ launch_darkly: { client_id: '1234' }, flag_overrides: { my_flag: '1234' }, + metadata_refresh_interval: duration(1, 'h'), }); plugin = new CloudExperimentsPlugin(initializerContext); + launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) + .mock.instances[0] as jest.Mocked; plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); - plugin.start(coreMock.createStart()); + plugin.start(coreMock.createStart(), { + dataViews: dataViewPluginMocks.createStartContract(), + }); }); test('flushes the events on stop', () => { - ldClientMock.flush.mockResolvedValue(); - expect(() => plugin.stop()).not.toThrow(); - expect(ldClientMock.flush).toHaveBeenCalledTimes(1); - }); - - test('handles errors when flushing events', async () => { - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); - const error = new Error('Something went terribly wrong'); - ldClientMock.flush.mockRejectedValue(error); expect(() => plugin.stop()).not.toThrow(); - expect(ldClientMock.flush).toHaveBeenCalledTimes(1); - await new Promise((resolve) => process.nextTick(resolve)); - expect(consoleWarnSpy).toHaveBeenCalledWith(error); + expect(launchDarklyInstanceMock.stop).toHaveBeenCalledTimes(1); }); }); }); 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 45ca45ab08af1..4db1cbecca1fd 100755 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts @@ -6,29 +6,37 @@ */ import type { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '@kbn/core/public'; -import type { LDClient } from 'launchdarkly-js-client-sdk'; import { get, has } from 'lodash'; +import { duration } from 'moment'; +import { concatMap, filter } from 'rxjs'; import { Sha256 } from '@kbn/crypto-browser'; import type { CloudSetup } from '@kbn/cloud-plugin/public'; +import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public'; +import { LaunchDarklyClient, type LaunchDarklyClientConfig } from './launch_darkly_client'; import type { CloudExperimentsFeatureFlagNames, CloudExperimentsMetric, CloudExperimentsPluginStart, } from '../common'; +import { MetadataService } from '../common/metadata_service'; import { FEATURE_FLAG_NAMES, METRIC_NAMES } from '../common/constants'; interface CloudExperimentsPluginSetupDeps { cloud: CloudSetup; } +interface CloudExperimentsPluginStartDeps { + dataViews: DataViewsPublicPluginStart; +} + /** * Browser-side implementation of the Cloud Experiments plugin */ export class CloudExperimentsPlugin implements Plugin { - private launchDarklyClient?: LDClient; - private readonly clientId?: string; + private readonly metadataService: MetadataService; + private readonly launchDarklyClient?: LaunchDarklyClient; private readonly kibanaVersion: string; private readonly flagOverrides?: Record; private readonly isDev: boolean; @@ -38,22 +46,28 @@ export class CloudExperimentsPlugin this.isDev = initializerContext.env.mode.dev; this.kibanaVersion = initializerContext.env.packageInfo.version; const config = initializerContext.config.get<{ - launch_darkly?: { client_id: string }; + launch_darkly?: LaunchDarklyClientConfig; flag_overrides?: Record; + metadata_refresh_interval: string; }>(); + + this.metadataService = new MetadataService({ + metadata_refresh_interval: duration(config.metadata_refresh_interval), + }); + if (config.flag_overrides) { this.flagOverrides = config.flag_overrides; } const ldConfig = config.launch_darkly; - if (!ldConfig && !initializerContext.env.mode.dev) { + if (!ldConfig?.client_id && !initializerContext.env.mode.dev) { // If the plugin is enabled, and it's in prod mode, launch_darkly must exist // (config-schema should enforce it, but just in case). throw new Error( 'xpack.cloud_integrations.experiments.launch_darkly configuration should exist' ); } - if (ldConfig) { - this.clientId = ldConfig.client_id; + if (ldConfig?.client_id) { + this.launchDarklyClient = new LaunchDarklyClient(ldConfig, this.kibanaVersion); } } @@ -63,26 +77,25 @@ export class CloudExperimentsPlugin * @param deps {@link CloudExperimentsPluginSetupDeps} */ public setup(core: CoreSetup, deps: CloudExperimentsPluginSetupDeps) { - if (deps.cloud.isCloudEnabled && deps.cloud.cloudId && this.clientId) { - import('launchdarkly-js-client-sdk').then( - (LaunchDarkly) => { - this.launchDarklyClient = LaunchDarkly.initialize( - this.clientId!, - { - // We use the Hashed Cloud Deployment ID as the userId in the Cloud Experiments - key: sha256(deps.cloud.cloudId!), - custom: { - kibanaVersion: this.kibanaVersion, - }, - }, - { application: { id: 'kibana-browser', version: this.kibanaVersion } } - ); - }, - (err) => { - // eslint-disable-next-line no-console - console.debug(`Error setting up LaunchDarkly: ${err.toString()}`); - } - ); + if (deps.cloud.isCloudEnabled && deps.cloud.cloudId && this.launchDarklyClient) { + this.metadataService.setup({ + userId: sha256(deps.cloud.cloudId), + kibanaVersion: this.kibanaVersion, + trial_end_date: deps.cloud.trialEndDate?.toISOString(), + is_elastic_staff_owned: deps.cloud.isElasticStaffOwned, + }); + + // 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) + ) + ) + .subscribe(); // This subscription will stop on when the metadataService stops because it completes the Observable } } @@ -90,7 +103,13 @@ export class CloudExperimentsPlugin * Returns the contract {@link CloudExperimentsPluginStart} * @param core {@link CoreStart} */ - public start(core: CoreStart): CloudExperimentsPluginStart { + 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, @@ -101,10 +120,8 @@ export class CloudExperimentsPlugin * Cleans up and flush the sending queues. */ public stop() { - this.launchDarklyClient - ?.flush() - // eslint-disable-next-line no-console - .catch((err) => console.warn(err)); + this.launchDarklyClient?.stop(); + this.metadataService.stop(); } private getVariation = async ( @@ -116,14 +133,13 @@ export class CloudExperimentsPlugin if (this.flagOverrides && has(this.flagOverrides, configKey)) { return get(this.flagOverrides, configKey, defaultValue) as Data; } - if (!this.launchDarklyClient) return defaultValue; // Skip any action if no LD User is defined - await this.launchDarklyClient.waitForInitialization(); - return this.launchDarklyClient.variation(configKey, defaultValue); + if (!this.launchDarklyClient) return defaultValue; // Skip any action if no LD Client is defined + return await this.launchDarklyClient.getVariation(configKey, defaultValue); }; private reportMetric = ({ name, meta, value }: CloudExperimentsMetric): void => { const metricName = METRIC_NAMES[name]; - this.launchDarklyClient?.track(metricName, meta, value); + this.launchDarklyClient?.reportMetric(metricName, meta, value); if (this.isDev) { // eslint-disable-next-line no-console console.debug(`Reported experimentation metric ${metricName}`, { diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.test.ts index 2e762ece1d8fe..146de2c3ddc9a 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.test.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.test.ts @@ -5,13 +5,17 @@ * 2.0. */ +import moment from 'moment'; import { config } from './config'; describe('cloudExperiments config', () => { describe.each([true, false])('when disabled (dev: %p)', (dev) => { const ctx = { dev }; test('should default to `enabled:false` and the rest empty', () => { - expect(config.schema.validate({}, ctx)).toStrictEqual({ enabled: false }); + expect(config.schema.validate({}, ctx)).toStrictEqual({ + enabled: false, + metadata_refresh_interval: moment.duration(1, 'h'), + }); }); test('it should allow any additional config', () => { @@ -26,7 +30,11 @@ describe('cloudExperiments config', () => { 'my-plugin.my-feature-flag': 1234, }, }; - expect(config.schema.validate(cfg, ctx)).toStrictEqual(cfg); + expect(config.schema.validate(cfg, ctx)).toStrictEqual({ + ...cfg, + // Additional default fields + metadata_refresh_interval: moment.duration(1, 'h'), + }); }); test('it should allow any additional config (missing flag_overrides)', () => { @@ -38,7 +46,10 @@ describe('cloudExperiments config', () => { client_log_level: 'none', }, }; - expect(config.schema.validate(cfg, ctx)).toStrictEqual(cfg); + expect(config.schema.validate(cfg, ctx)).toStrictEqual({ + ...cfg, + metadata_refresh_interval: moment.duration(1, 'h'), + }); }); test('it should allow any additional config (missing launch_darkly)', () => { @@ -48,7 +59,10 @@ describe('cloudExperiments config', () => { 'my-plugin.my-feature-flag': 1234, }, }; - expect(config.schema.validate(cfg, ctx)).toStrictEqual(cfg); + expect(config.schema.validate(cfg, ctx)).toStrictEqual({ + ...cfg, + metadata_refresh_interval: moment.duration(1, 'h'), + }); }); }); @@ -61,11 +75,15 @@ describe('cloudExperiments config', () => { ).toStrictEqual({ enabled: true, flag_overrides: { my_flag: 1 }, + metadata_refresh_interval: moment.duration(1, 'h'), }); }); test('in dev mode, it allows `launch_darkly` and `flag_overrides` to be empty', () => { - expect(config.schema.validate({ enabled: true }, ctx)).toStrictEqual({ enabled: true }); + expect(config.schema.validate({ enabled: true }, ctx)).toStrictEqual({ + enabled: true, + metadata_refresh_interval: moment.duration(1, 'h'), + }); }); }); @@ -98,6 +116,7 @@ describe('cloudExperiments config', () => { client_id: '1234', client_log_level: 'none', }, + metadata_refresh_interval: moment.duration(1, 'h'), }); }); @@ -126,6 +145,7 @@ describe('cloudExperiments config', () => { flag_overrides: { my_flag: 123, }, + metadata_refresh_interval: moment.duration(1, 'h'), }); }); }); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.ts index 79b49bcb77509..a5b5eeb88c2dd 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/config.ts @@ -37,6 +37,7 @@ const configSchema = schema.object({ schema.maybe(launchDarklySchema) ), flag_overrides: schema.maybe(schema.recordOf(schema.string(), schema.any())), + metadata_refresh_interval: schema.duration({ defaultValue: '1h' }), }); export type CloudExperimentsConfigType = TypeOf; @@ -45,8 +46,10 @@ export const config: PluginConfigDescriptor = { exposeToBrowser: { launch_darkly: { client_id: true, + client_log_level: true, }, flag_overrides: true, + metadata_refresh_interval: true, }, schema: configSchema, }; diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/index.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/index.ts new file mode 100644 index 0000000000000..d298aad1ad6c1 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/index.ts @@ -0,0 +1,8 @@ +/* + * 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 { LaunchDarklyClient, type LaunchDarklyUserMetadata } from './launch_darkly_client'; diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.mock.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.mock.ts similarity index 97% rename from x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.mock.ts rename to x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.mock.ts index b76e629458e00..755d565e6b82f 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.test.mock.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.mock.ts @@ -13,7 +13,6 @@ export function createLaunchDarklyClientMock(): jest.Mocked { variation: jest.fn(), allFlagsState: jest.fn(), track: jest.fn(), - identify: jest.fn(), flush: jest.fn(), } as unknown as jest.Mocked; // Using casting because we only use these APIs. No need to declare everything. } diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.ts new file mode 100644 index 0000000000000..cc140ea44ffdb --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.test.ts @@ -0,0 +1,211 @@ +/* + * 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 { loggerMock, type MockedLogger } from '@kbn/logging-mocks'; +import { ldClientMock } from './launch_darkly_client.test.mock'; +import LaunchDarkly from 'launchdarkly-node-server-sdk'; +import { LaunchDarklyClient, type LaunchDarklyClientConfig } from './launch_darkly_client'; + +describe('LaunchDarklyClient - server', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); + + const config: LaunchDarklyClientConfig = { + sdk_key: 'fake-sdk-key', + client_id: 'fake-client-id', + client_log_level: 'debug', + kibana_version: 'version', + }; + + describe('constructor', () => { + let launchDarklyInitSpy: jest.SpyInstance; + + beforeEach(() => { + launchDarklyInitSpy = jest.spyOn(LaunchDarkly, 'init'); + }); + + afterEach(() => { + launchDarklyInitSpy.mockRestore(); + }); + + test('it initializes the LaunchDarkly client', async () => { + const logger = loggerMock.create(); + ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); + + const client = new LaunchDarklyClient(config, logger); + expect(launchDarklyInitSpy).toHaveBeenCalledWith('fake-sdk-key', { + application: { id: 'kibana-server', version: 'version' }, + logger: undefined, // The method basicLogger is mocked without a return value + stream: false, + }); + expect(client).toHaveProperty('launchDarklyClient', ldClientMock); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the waitForInitialization resolution + expect(logger.debug).toHaveBeenCalledWith('LaunchDarkly is initialized!'); + }); + + test('it initializes the LaunchDarkly client... and handles failure', async () => { + const logger = loggerMock.create(); + ldClientMock.waitForInitialization.mockRejectedValue( + new Error('Something went terribly wrong') + ); + + const client = new LaunchDarklyClient(config, logger); + expect(launchDarklyInitSpy).toHaveBeenCalledWith('fake-sdk-key', { + application: { id: 'kibana-server', version: 'version' }, + logger: undefined, // The method basicLogger is mocked without a return value + stream: false, + }); + expect(client).toHaveProperty('launchDarklyClient', ldClientMock); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the waitForInitialization resolution + expect(logger.warn).toHaveBeenCalledWith( + 'Error initializing LaunchDarkly: Error: Something went terribly wrong' + ); + }); + }); + + describe('Public APIs', () => { + let client: LaunchDarklyClient; + let logger: MockedLogger; + const testUserMetadata = { userId: 'fake-user-id', kibanaVersion: 'version' }; + + beforeEach(() => { + logger = loggerMock.create(); + ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); + client = new LaunchDarklyClient(config, logger); + }); + + describe('updateUserMetadata', () => { + test('sets the top-level properties at the root (renaming userId to key) and the rest under `custom`', () => { + expect(client).toHaveProperty('launchDarklyUser', undefined); + + const topFields = { + name: 'First Last', + firstName: 'First', + lastName: 'Last', + email: 'first.last@boring.co', + avatar: 'fake-blue-avatar', + ip: 'my-weird-ip', + country: 'distributed', + }; + + const extraFields = { + other_field: 'my other custom field', + kibanaVersion: 'version', + }; + + client.updateUserMetadata({ userId: 'fake-user-id', ...topFields, ...extraFields }); + + expect(client).toHaveProperty('launchDarklyUser', { + key: 'fake-user-id', + ...topFields, + custom: extraFields, + }); + }); + + test('sets a minimum amount of info', () => { + expect(client).toHaveProperty('launchDarklyUser', undefined); + + client.updateUserMetadata({ userId: 'fake-user-id', kibanaVersion: 'version' }); + + expect(client).toHaveProperty('launchDarklyUser', { + key: 'fake-user-id', + custom: { kibanaVersion: 'version' }, + }); + }); + }); + + describe('getVariation', () => { + test('returns the default value if the user has not been defined', async () => { + await expect(client.getVariation('my-feature-flag', 123)).resolves.toStrictEqual(123); + expect(ldClientMock.variation).toHaveBeenCalledTimes(0); + }); + + test('calls the LaunchDarkly client when the user has been defined', async () => { + ldClientMock.variation.mockResolvedValue(1234); + client.updateUserMetadata(testUserMetadata); + await expect(client.getVariation('my-feature-flag', 123)).resolves.toStrictEqual(1234); + expect(ldClientMock.variation).toHaveBeenCalledTimes(1); + expect(ldClientMock.variation).toHaveBeenCalledWith( + 'my-feature-flag', + { key: 'fake-user-id', custom: { kibanaVersion: 'version' } }, + 123 + ); + }); + }); + + describe('reportMetric', () => { + test('does not call track if the user has not been defined', () => { + client.reportMetric('my-feature-flag', {}, 123); + expect(ldClientMock.track).toHaveBeenCalledTimes(0); + }); + + test('calls the LaunchDarkly client when the user has been defined', () => { + client.updateUserMetadata(testUserMetadata); + client.reportMetric('my-feature-flag', {}, 123); + expect(ldClientMock.track).toHaveBeenCalledTimes(1); + expect(ldClientMock.track).toHaveBeenCalledWith( + 'my-feature-flag', + { key: 'fake-user-id', custom: { kibanaVersion: 'version' } }, + {}, + 123 + ); + }); + }); + + describe('getAllFlags', () => { + test('returns the non-initialized state if the user has not been defined', async () => { + await expect(client.getAllFlags()).resolves.toStrictEqual({ + initialized: false, + flagNames: [], + flags: {}, + }); + expect(ldClientMock.allFlagsState).toHaveBeenCalledTimes(0); + }); + + test('calls the LaunchDarkly client when the user has been defined', async () => { + ldClientMock.allFlagsState.mockResolvedValue({ + valid: true, + allValues: jest.fn().mockReturnValue({ my_flag: '1234' }), + getFlagValue: jest.fn(), + getFlagReason: jest.fn(), + toJSON: jest.fn(), + }); + client.updateUserMetadata(testUserMetadata); + await expect(client.getAllFlags()).resolves.toStrictEqual({ + initialized: true, + flagNames: ['my_flag'], + flags: { my_flag: '1234' }, + }); + expect(ldClientMock.allFlagsState).toHaveBeenCalledTimes(1); + expect(ldClientMock.allFlagsState).toHaveBeenCalledWith({ + key: 'fake-user-id', + custom: { kibanaVersion: 'version' }, + }); + }); + }); + + describe('stop', () => { + test('flushes the events', async () => { + ldClientMock.flush.mockResolvedValue(); + expect(() => client.stop()).not.toThrow(); + expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the flush resolution + expect(logger.error).not.toHaveBeenCalled(); + }); + + test('handles errors when flushing events', async () => { + const err = new Error('Something went terribly wrong'); + ldClientMock.flush.mockRejectedValue(err); + expect(() => client.stop()).not.toThrow(); + expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + await new Promise((resolve) => process.nextTick(resolve)); // wait for the flush resolution + expect(logger.error).toHaveBeenCalledWith(err); + }); + }); + }); +}); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.ts new file mode 100644 index 0000000000000..10126e6d48a46 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/launch_darkly_client.ts @@ -0,0 +1,104 @@ +/* + * 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 LaunchDarkly, { + type LDClient, + type LDFlagSet, + type LDLogLevel, + type LDUser, +} from 'launchdarkly-node-server-sdk'; +import type { Logger } from '@kbn/core/server'; + +export interface LaunchDarklyClientConfig { + sdk_key: string; + client_id: string; + client_log_level: LDLogLevel; + kibana_version: string; +} + +export interface LaunchDarklyUserMetadata + extends Record { + userId: string; + // We are not collecting any of the above, but this is to match the LDUser first-level definition + name?: string; + firstName?: string; + lastName?: string; + email?: string; + avatar?: string; + ip?: string; + country?: string; +} + +export interface LaunchDarklyGetAllFlags { + initialized: boolean; + flags: LDFlagSet; + flagNames: string[]; +} + +export class LaunchDarklyClient { + private readonly launchDarklyClient: LDClient; + private launchDarklyUser?: LDUser; + + constructor(ldConfig: LaunchDarklyClientConfig, private readonly logger: Logger) { + this.launchDarklyClient = LaunchDarkly.init(ldConfig.sdk_key, { + application: { id: `kibana-server`, version: ldConfig.kibana_version }, + logger: LaunchDarkly.basicLogger({ level: ldConfig.client_log_level }), + // For some reason, the stream API does not work in Kibana. `.waitForInitialization()` hangs forever (doesn't throw, neither logs any errors). + // Using polling for now until we resolve that issue. + // Relevant issue: https://github.com/launchdarkly/node-server-sdk/issues/132 + stream: false, + }); + this.launchDarklyClient.waitForInitialization().then( + () => this.logger.debug('LaunchDarkly is initialized!'), + (err) => this.logger.warn(`Error initializing LaunchDarkly: ${err}`) + ); + } + + public updateUserMetadata(userMetadata: LaunchDarklyUserMetadata) { + const { userId, name, firstName, lastName, email, avatar, ip, country, ...custom } = + userMetadata; + this.launchDarklyUser = { + key: userId, + name, + firstName, + lastName, + email, + avatar, + ip, + country, + // This casting is needed because LDUser does not allow `Record` + custom: custom as Record, + }; + } + + public async getVariation(configKey: string, defaultValue: Data): Promise { + if (!this.launchDarklyUser) return defaultValue; // Skip any action if no LD User is defined + await this.launchDarklyClient.waitForInitialization(); + return await this.launchDarklyClient.variation(configKey, this.launchDarklyUser, defaultValue); + } + + public reportMetric(metricName: string, meta?: unknown, value?: number): void { + if (!this.launchDarklyUser) return; // Skip any action if no LD User is defined + this.launchDarklyClient.track(metricName, this.launchDarklyUser, meta, value); + } + + public async getAllFlags(): Promise { + if (!this.launchDarklyUser) return { initialized: false, flagNames: [], flags: {} }; + // According to the docs, this method does not send analytics back to LaunchDarkly, so it does not provide false results + const flagsState = await this.launchDarklyClient.allFlagsState(this.launchDarklyUser); + const flags = flagsState.allValues(); + return { + initialized: flagsState.valid, + flags, + flagNames: Object.keys(flags), + }; + } + + public stop() { + this.launchDarklyClient?.flush().catch((err) => this.logger.error(err)); + } +} diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/mocks.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/mocks.ts new file mode 100644 index 0000000000000..3fe1838815b27 --- /dev/null +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/launch_darkly_client/mocks.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 { PublicMethodsOf } from '@kbn/utility-types'; +import { LaunchDarklyClient } from './launch_darkly_client'; + +function createLaunchDarklyClientMock(): jest.Mocked { + const launchDarklyClientMock: jest.Mocked> = { + updateUserMetadata: jest.fn(), + getVariation: jest.fn(), + getAllFlags: jest.fn(), + reportMetric: jest.fn(), + stop: jest.fn(), + }; + + return launchDarklyClientMock as jest.Mocked; +} + +export const launchDarklyClientMocks = { + launchDarklyClientMock: createLaunchDarklyClientMock(), + createLaunchDarklyClient: createLaunchDarklyClientMock, +}; 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 aa78353b72329..e2f904aad2338 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 @@ -5,24 +5,28 @@ * 2.0. */ +import { fakeSchedulers } from 'rxjs-marbles/jest'; import { coreMock } from '@kbn/core/server/mocks'; import { cloudMock } from '@kbn/cloud-plugin/server/mocks'; import { usageCollectionPluginMock } from '@kbn/usage-collection-plugin/server/mocks'; -import { ldClientMock } from './plugin.test.mock'; +import { + createIndexPatternsStartMock, + dataViewsService, +} from '@kbn/data-views-plugin/server/mocks'; +import { DataViewsServerPluginStart } from '@kbn/data-views-plugin/server'; +import { config } from './config'; import { CloudExperimentsPlugin } from './plugin'; import { FEATURE_FLAG_NAMES } from '../common/constants'; +import { LaunchDarklyClient } from './launch_darkly_client'; +jest.mock('./launch_darkly_client'); describe('Cloud Experiments server plugin', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); + jest.useFakeTimers(); - const ldUser = { - key: '1c2412b751f056aef6e340efa5637d137442d489a4b1e3117071e7c87f8523f2', - custom: { - kibanaVersion: coreMock.createPluginInitializerContext().env.packageInfo.version, - }, - }; + afterEach(() => { + jest.clearAllTimers(); + jest.clearAllMocks(); + }); describe('constructor', () => { test('successfully creates a new plugin if provided an empty configuration', () => { @@ -48,9 +52,9 @@ describe('Cloud Experiments server plugin', () => { const initializerContext = coreMock.createPluginInitializerContext({ launch_darkly: { sdk_key: 'sdk-1234' }, }); - ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); const plugin = new CloudExperimentsPlugin(initializerContext); - expect(plugin).toHaveProperty('launchDarklyClient', ldClientMock); + expect(LaunchDarklyClient).toHaveBeenCalledTimes(1); + expect(plugin).toHaveProperty('launchDarklyClient', expect.any(LaunchDarklyClient)); }); test('it initializes the flagOverrides property', () => { @@ -67,14 +71,22 @@ describe('Cloud Experiments server plugin', () => { let plugin: CloudExperimentsPlugin; beforeEach(() => { - const initializerContext = coreMock.createPluginInitializerContext({ - launch_darkly: { sdk_key: 'sdk-1234' }, - flag_overrides: { my_flag: '1234' }, - }); - ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); + const initializerContext = coreMock.createPluginInitializerContext( + config.schema.validate( + { + launch_darkly: { sdk_key: 'sdk-1234', client_id: 'fake-client-id' }, + flag_overrides: { my_flag: '1234' }, + }, + { dev: true } + ) + ); plugin = new CloudExperimentsPlugin(initializerContext); }); + afterEach(() => { + plugin.stop(); + }); + test('returns the contract', () => { expect( plugin.setup(coreMock.createSetup(), { @@ -93,35 +105,52 @@ describe('Cloud Experiments server plugin', () => { expect(usageCollection.registerCollector).toHaveBeenCalledTimes(1); }); - describe('identifyUser', () => { - test('sets launchDarklyUser and calls identify', () => { - expect(plugin).toHaveProperty('launchDarklyUser', undefined); - plugin.setup(coreMock.createSetup(), { - cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, - }); - expect(plugin).toHaveProperty('launchDarklyUser', ldUser); - expect(ldClientMock.identify).toHaveBeenCalledWith(ldUser); + 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), }); }); }); describe('start', () => { let plugin: CloudExperimentsPlugin; + let dataViews: jest.Mocked; + let launchDarklyInstanceMock: jest.Mocked; const firstKnownFlag = Object.keys(FEATURE_FLAG_NAMES)[0] as keyof typeof FEATURE_FLAG_NAMES; beforeEach(() => { - const initializerContext = coreMock.createPluginInitializerContext({ - launch_darkly: { sdk_key: 'sdk-1234' }, - flag_overrides: { [firstKnownFlag]: '1234' }, - }); - ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); + const initializerContext = coreMock.createPluginInitializerContext( + config.schema.validate( + { + launch_darkly: { sdk_key: 'sdk-1234', client_id: 'fake-client-id' }, + flag_overrides: { [firstKnownFlag]: '1234' }, + }, + { dev: true } + ) + ); plugin = new CloudExperimentsPlugin(initializerContext); + dataViews = createIndexPatternsStartMock(); + launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass) + .mock.instances[0] as jest.Mocked; + }); + + afterEach(() => { + plugin.stop(); }); test('returns the contract', () => { plugin.setup(coreMock.createSetup(), { cloud: cloudMock.createSetup() }); - const startContract = plugin.start(coreMock.createStart()); + const startContract = plugin.start(coreMock.createStart(), { dataViews }); expect(startContract).toStrictEqual( expect.objectContaining({ getVariation: expect.any(Function), @@ -130,8 +159,29 @@ 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, + }) + ); + }) + ); + describe('getVariation', () => { - describe('with the user identified', () => { + describe('with the client created', () => { beforeEach(() => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, @@ -139,15 +189,15 @@ describe('Cloud Experiments server plugin', () => { }); test('uses the flag overrides to respond early', async () => { - const startContract = plugin.start(coreMock.createStart()); + const startContract = plugin.start(coreMock.createStart(), { dataViews }); await expect(startContract.getVariation(firstKnownFlag, 123)).resolves.toStrictEqual( '1234' ); }); test('calls the client', async () => { - const startContract = plugin.start(coreMock.createStart()); - ldClientMock.variation.mockResolvedValue('12345'); + const startContract = plugin.start(coreMock.createStart(), { dataViews }); + launchDarklyInstanceMock.getVariation.mockResolvedValue('12345'); await expect( startContract.getVariation( // @ts-expect-error We only allow existing flags in FEATURE_FLAG_NAMES @@ -155,30 +205,38 @@ describe('Cloud Experiments server plugin', () => { 123 ) ).resolves.toStrictEqual('12345'); - expect(ldClientMock.variation).toHaveBeenCalledWith( + expect(launchDarklyInstanceMock.getVariation).toHaveBeenCalledWith( undefined, // it couldn't find it in FEATURE_FLAG_NAMES - ldUser, 123 ); }); }); - describe('with the user not identified', () => { + describe('with the client not created (missing LD settings)', () => { beforeEach(() => { + const initializerContext = coreMock.createPluginInitializerContext( + config.schema.validate( + { + flag_overrides: { [firstKnownFlag]: '1234' }, + }, + { dev: true } + ) + ); + plugin = new CloudExperimentsPlugin(initializerContext); plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, }); }); test('uses the flag overrides to respond early', async () => { - const startContract = plugin.start(coreMock.createStart()); + const startContract = plugin.start(coreMock.createStart(), { dataViews }); await expect(startContract.getVariation(firstKnownFlag, 123)).resolves.toStrictEqual( '1234' ); }); test('returns the default value without calling the client', async () => { - const startContract = plugin.start(coreMock.createStart()); + const startContract = plugin.start(coreMock.createStart(), { dataViews }); await expect( startContract.getVariation( // @ts-expect-error We only allow existing flags in FEATURE_FLAG_NAMES @@ -186,52 +244,59 @@ describe('Cloud Experiments server plugin', () => { 123 ) ).resolves.toStrictEqual(123); - expect(ldClientMock.variation).not.toHaveBeenCalled(); }); }); }); describe('reportMetric', () => { - describe('with the user identified', () => { + describe('with the client created', () => { beforeEach(() => { plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); }); - test('calls the track API', () => { - const startContract = plugin.start(coreMock.createStart()); + test('calls LaunchDarklyClient.reportMetric', () => { + const startContract = plugin.start(coreMock.createStart(), { dataViews }); startContract.reportMetric({ // @ts-expect-error We only allow existing flags in METRIC_NAMES name: 'my-flag', meta: {}, value: 1, }); - expect(ldClientMock.track).toHaveBeenCalledWith( + expect(launchDarklyInstanceMock.reportMetric).toHaveBeenCalledWith( undefined, // it couldn't find it in METRIC_NAMES - ldUser, {}, 1 ); }); }); - describe('with the user not identified', () => { + describe('with the client not created (missing LD settings)', () => { beforeEach(() => { + const initializerContext = coreMock.createPluginInitializerContext( + config.schema.validate( + { + flag_overrides: { [firstKnownFlag]: '1234' }, + }, + { dev: true } + ) + ); + plugin = new CloudExperimentsPlugin(initializerContext); plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: false }, }); }); - test('calls the track API', () => { - const startContract = plugin.start(coreMock.createStart()); + test('does not call LaunchDarklyClient.reportMetric because the client is not there', () => { + const startContract = plugin.start(coreMock.createStart(), { dataViews }); startContract.reportMetric({ // @ts-expect-error We only allow existing flags in METRIC_NAMES name: 'my-flag', meta: {}, value: 1, }); - expect(ldClientMock.track).not.toHaveBeenCalled(); + expect(plugin).toHaveProperty('launchDarklyClient', undefined); }); }); }); @@ -241,28 +306,36 @@ describe('Cloud Experiments server plugin', () => { let plugin: CloudExperimentsPlugin; beforeEach(() => { - const initializerContext = coreMock.createPluginInitializerContext({ - launch_darkly: { sdk_key: 'sdk-1234' }, - flag_overrides: { my_flag: '1234' }, - }); - ldClientMock.waitForInitialization.mockResolvedValue(ldClientMock); + const initializerContext = coreMock.createPluginInitializerContext( + config.schema.validate( + { + launch_darkly: { sdk_key: 'sdk-1234', client_id: 'fake-client-id' }, + flag_overrides: { my_flag: '1234' }, + }, + { dev: true } + ) + ); plugin = new CloudExperimentsPlugin(initializerContext); + const dataViews = createIndexPatternsStartMock(); plugin.setup(coreMock.createSetup(), { cloud: { ...cloudMock.createSetup(), isCloudEnabled: true }, }); - plugin.start(coreMock.createStart()); + plugin.start(coreMock.createStart(), { dataViews }); }); - test('flushes the events', () => { - ldClientMock.flush.mockResolvedValue(); - expect(() => plugin.stop()).not.toThrow(); - expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + test('stops the LaunchDarkly client', () => { + plugin.stop(); + const launchDarklyInstanceMock = ( + LaunchDarklyClient as jest.MockedClass + ).mock.instances[0] as jest.Mocked; + expect(launchDarklyInstanceMock.stop).toHaveBeenCalledTimes(1); }); - test('handles errors when flushing events', () => { - ldClientMock.flush.mockRejectedValue(new Error('Something went terribly wrong')); - expect(() => plugin.stop()).not.toThrow(); - expect(ldClientMock.flush).toHaveBeenCalledTimes(1); + test('stops the Metadata Service', () => { + // eslint-disable-next-line dot-notation + const metadataServiceStopSpy = jest.spyOn(plugin['metadataService'], 'stop'); + plugin.stop(); + expect(metadataServiceStopSpy).toHaveBeenCalledTimes(1); }); }); }); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.ts index 782159dc12ab9..093b17934b686 100755 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/plugin.ts @@ -13,11 +13,14 @@ import type { Logger, } from '@kbn/core/server'; import { get, has } from 'lodash'; -import LaunchDarkly, { type LDClient, type LDUser } from 'launchdarkly-node-server-sdk'; import { createSHA256Hash } from '@kbn/crypto'; import type { LogMeta } from '@kbn/logging'; import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; import type { CloudSetup } from '@kbn/cloud-plugin/server'; +import type { DataViewsServerPluginStart } from '@kbn/data-views-plugin/server/types'; +import { filter, map } from 'rxjs'; +import { MetadataService } from '../common/metadata_service'; +import { LaunchDarklyClient } from './launch_darkly_client'; import { registerUsageCollector } from './usage'; import type { CloudExperimentsConfigType } from './config'; import type { @@ -32,17 +35,26 @@ interface CloudExperimentsPluginSetupDeps { usageCollection?: UsageCollectionSetup; } +interface CloudExperimentsPluginStartDeps { + dataViews: DataViewsServerPluginStart; +} + export class CloudExperimentsPlugin implements Plugin { private readonly logger: Logger; - private readonly launchDarklyClient?: LDClient; + private readonly launchDarklyClient?: LaunchDarklyClient; private readonly flagOverrides?: Record; - private launchDarklyUser: LDUser | undefined; + private readonly metadataService: MetadataService; constructor(private readonly initializerContext: PluginInitializerContext) { this.logger = initializerContext.logger.get(); const config = initializerContext.config.get(); + + this.metadataService = new MetadataService({ + metadata_refresh_interval: config.metadata_refresh_interval, + }); + if (config.flag_overrides) { this.flagOverrides = config.flag_overrides; } @@ -55,17 +67,12 @@ export class CloudExperimentsPlugin ); } if (ldConfig) { - this.launchDarklyClient = LaunchDarkly.init(ldConfig.sdk_key, { - application: { id: `kibana-server`, version: initializerContext.env.packageInfo.version }, - logger: LaunchDarkly.basicLogger({ level: ldConfig.client_log_level }), - // For some reason, the stream API does not work in Kibana. `.waitForInitialization()` hangs forever (doesn't throw, neither logs any errors). - // Using polling for now until we resolve that issue. - // Relevant issue: https://github.com/launchdarkly/node-server-sdk/issues/132 - stream: false, - }); - this.launchDarklyClient.waitForInitialization().then( - () => this.logger.debug('LaunchDarkly is initialized!'), - (err) => this.logger.warn(`Error initializing LaunchDarkly: ${err}`) + this.launchDarklyClient = new LaunchDarklyClient( + { + ...ldConfig, + kibana_version: initializerContext.env.packageInfo.version, + }, + this.logger.get('launch_darkly') ); } } @@ -74,24 +81,33 @@ export class CloudExperimentsPlugin if (deps.usageCollection) { registerUsageCollector(deps.usageCollection, () => ({ launchDarklyClient: this.launchDarklyClient, - launchDarklyUser: this.launchDarklyUser, })); } if (deps.cloud.isCloudEnabled && deps.cloud.cloudId) { - this.launchDarklyUser = { + this.metadataService.setup({ // We use the Cloud ID as the userId in the Cloud Experiments - key: createSHA256Hash(deps.cloud.cloudId), - custom: { - // This list of deployment metadata will likely grow in future versions - kibanaVersion: this.initializerContext.env.packageInfo.version, - }, - }; - this.launchDarklyClient?.identify(this.launchDarklyUser); + userId: createSHA256Hash(deps.cloud.cloudId), + kibanaVersion: this.initializerContext.env.packageInfo.version, + trial_end_date: deps.cloud.trialEndDate?.toISOString(), + is_elastic_staff_owned: deps.cloud.isElasticStaffOwned, + }); + + // 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 + map((userMetadata) => this.launchDarklyClient?.updateUserMetadata(userMetadata)) + ) + .subscribe(); // This subscription will stop on when the metadataService stops because it completes the Observable } } - public start(core: CoreStart) { + public start(core: CoreStart, deps: CloudExperimentsPluginStartDeps) { + this.metadataService.start({ + hasDataFetcher: async () => await this.addHasDataMetadata(core, deps.dataViews), + }); return { getVariation: this.getVariation, reportMetric: this.reportMetric, @@ -99,7 +115,8 @@ export class CloudExperimentsPlugin } public stop() { - this.launchDarklyClient?.flush().catch((err) => this.logger.error(err)); + this.launchDarklyClient?.stop(); + this.metadataService.stop(); } private getVariation = async ( @@ -111,15 +128,13 @@ export class CloudExperimentsPlugin if (this.flagOverrides && has(this.flagOverrides, configKey)) { return get(this.flagOverrides, configKey, defaultValue) as Data; } - if (!this.launchDarklyUser) return defaultValue; // Skip any action if no LD User is defined - await this.launchDarklyClient?.waitForInitialization(); - return await this.launchDarklyClient?.variation(configKey, this.launchDarklyUser, defaultValue); + if (!this.launchDarklyClient) return defaultValue; + return await this.launchDarklyClient.getVariation(configKey, defaultValue); }; private reportMetric = ({ name, meta, value }: CloudExperimentsMetric): void => { const metricName = METRIC_NAMES[name]; - if (!this.launchDarklyUser) return; // Skip any action if no LD User is defined - this.launchDarklyClient?.track(metricName, this.launchDarklyUser, meta, value); + this.launchDarklyClient?.reportMetric(metricName, meta, value); this.logger.debug<{ experimentationMetric: CloudExperimentsMetric } & LogMeta>( `Reported experimentation metric ${metricName}`, { @@ -127,4 +142,19 @@ export class CloudExperimentsPlugin } ); }; + + private async addHasDataMetadata( + core: CoreStart, + dataViews: DataViewsServerPluginStart + ): Promise<{ has_data: boolean }> { + const dataViewsService = await dataViews.dataViewsServiceFactory( + core.savedObjects.createInternalRepository(), + core.elasticsearch.client.asInternalUser, + void 0, // No Kibana Request to scope the check + true // Ignore capabilities checks + ); + return { + has_data: await dataViewsService.hasUserDataView(), + }; + } } diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.test.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.test.ts index 176bbad4f6702..ab18c2dbed613 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.test.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.test.ts @@ -15,7 +15,7 @@ import { type LaunchDarklyEntitiesGetter, type Usage, } from './register_usage_collector'; -import { createLaunchDarklyClientMock } from '../plugin.test.mock'; +import { launchDarklyClientMocks } from '../launch_darkly_client/mocks'; describe('cloudExperiments usage collector', () => { let collector: Collector; @@ -34,27 +34,7 @@ describe('cloudExperiments usage collector', () => { expect(collector.isReady()).toStrictEqual(true); }); - test('should return initialized false and empty values when the user and the client are not initialized', async () => { - await expect(collector.fetch(createCollectorFetchContextMock())).resolves.toStrictEqual({ - flagNames: [], - flags: {}, - initialized: false, - }); - }); - - test('should return initialized false and empty values when the user is not initialized', async () => { - getLaunchDarklyEntitiesMock.mockReturnValueOnce({ - launchDarklyClient: createLaunchDarklyClientMock(), - }); - await expect(collector.fetch(createCollectorFetchContextMock())).resolves.toStrictEqual({ - flagNames: [], - flags: {}, - initialized: false, - }); - }); - test('should return initialized false and empty values when the client is not initialized', async () => { - getLaunchDarklyEntitiesMock.mockReturnValueOnce({ launchDarklyUser: { key: 'test' } }); await expect(collector.fetch(createCollectorFetchContextMock())).resolves.toStrictEqual({ flagNames: [], flags: {}, @@ -63,21 +43,16 @@ describe('cloudExperiments usage collector', () => { }); test('should return all the flags returned by the client', async () => { - const launchDarklyClient = createLaunchDarklyClientMock(); - getLaunchDarklyEntitiesMock.mockReturnValueOnce({ - launchDarklyClient, - launchDarklyUser: { key: 'test' }, - }); + const launchDarklyClient = launchDarklyClientMocks.createLaunchDarklyClient(); + getLaunchDarklyEntitiesMock.mockReturnValueOnce({ launchDarklyClient }); - launchDarklyClient.allFlagsState.mockResolvedValueOnce({ - valid: true, - getFlagValue: jest.fn(), - getFlagReason: jest.fn(), - toJSON: jest.fn(), - allValues: jest.fn().mockReturnValueOnce({ + launchDarklyClient.getAllFlags.mockResolvedValueOnce({ + initialized: true, + flags: { 'my-plugin.my-feature-flag': true, 'my-plugin.my-other-feature-flag': 22, - }), + }, + flagNames: ['my-plugin.my-feature-flag', 'my-plugin.my-other-feature-flag'], }); await expect(collector.fetch(createCollectorFetchContextMock())).resolves.toStrictEqual({ diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.ts b/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.ts index c9390915fd8c2..9a800dabe9fc8 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/server/usage/register_usage_collector.ts @@ -5,8 +5,8 @@ * 2.0. */ -import type { LDClient, LDUser } from 'launchdarkly-node-server-sdk'; import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; +import type { LaunchDarklyClient } from '../launch_darkly_client'; export interface Usage { initialized: boolean; @@ -15,8 +15,7 @@ export interface Usage { } export type LaunchDarklyEntitiesGetter = () => { - launchDarklyUser?: LDUser; - launchDarklyClient?: LDClient; + launchDarklyClient?: LaunchDarklyClient; }; export function registerUsageCollector( @@ -53,17 +52,9 @@ export function registerUsageCollector( }, }, fetch: async () => { - const { launchDarklyUser, launchDarklyClient } = getLaunchDarklyEntities(); - if (!launchDarklyUser || !launchDarklyClient) - return { initialized: false, flagNames: [], flags: {} }; - // According to the docs, this method does not send analytics back to LaunchDarkly, so it does not provide false results - const flagsState = await launchDarklyClient.allFlagsState(launchDarklyUser); - const flags = flagsState.allValues(); - return { - initialized: flagsState.valid, - flags, - flagNames: Object.keys(flags), - }; + const { launchDarklyClient } = getLaunchDarklyEntities(); + if (!launchDarklyClient) return { initialized: false, flagNames: [], flags: {} }; + return await launchDarklyClient.getAllFlags(); }, }) ); diff --git a/x-pack/plugins/cloud_integrations/cloud_experiments/tsconfig.json b/x-pack/plugins/cloud_integrations/cloud_experiments/tsconfig.json index 917c8c0c9fc53..7f0e98957c5f7 100644 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/tsconfig.json +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/tsconfig.json @@ -15,6 +15,7 @@ ], "references": [ { "path": "../../../../src/core/tsconfig.json" }, + { "path": "../../../../src/plugins/data_views/tsconfig.json" }, { "path": "../../../../src/plugins/usage_collection/tsconfig.json" }, { "path": "../../cloud/tsconfig.json" }, ] diff --git a/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json b/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json index 890e4c8a5e4f1..7b850a44528fd 100644 --- a/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json +++ b/x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json @@ -4970,6 +4970,15 @@ "properties": { "isCloudEnabled": { "type": "boolean" + }, + "trialEndDate": { + "type": "date" + }, + "inTrial": { + "type": "boolean" + }, + "isElasticStaffOwned": { + "type": "boolean" } } }, From ba4f6eee6d1a4f461fdac98891c111b490f96c06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Tue, 11 Oct 2022 17:48:42 +0200 Subject: [PATCH 2/4] Add debounceTime to the metadata service --- .../metadata_service/metadata_service.test.ts | 19 ++--- .../metadata_service/metadata_service.ts | 37 +++++++--- .../cloud_experiments/public/plugin.test.ts | 44 ++++++++---- .../cloud_experiments/public/plugin.ts | 34 ++++----- .../cloud_experiments/server/plugin.test.ts | 70 ++++++++++--------- 5 files changed, 124 insertions(+), 80 deletions(-) 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', () => { From 46cf6432e139befdf7cb4d55fc5495594c143a81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Fri, 14 Oct 2022 18:42:31 +0200 Subject: [PATCH 3/4] Address nit --- .../plugins/cloud/server/collectors/cloud_usage_collector.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts index b107099133e20..147f61a57312b 100644 --- a/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts +++ b/x-pack/plugins/cloud/server/collectors/cloud_usage_collector.ts @@ -22,6 +22,7 @@ interface CloudUsage { export function createCloudUsageCollector(usageCollection: UsageCollectionSetup, config: Config) { const { isCloudEnabled, trialEndDate, isElasticStaffOwned } = config; + const trialEndDateMs = trialEndDate ? new Date(trialEndDate).getTime() : undefined; return usageCollection.makeUsageCollector({ type: 'cloud', isReady: () => true, @@ -36,7 +37,7 @@ export function createCloudUsageCollector(usageCollection: UsageCollectionSetup, isCloudEnabled, isElasticStaffOwned, trialEndDate, - ...(trialEndDate ? { inTrial: Date.now() <= new Date(trialEndDate).getTime() } : {}), + ...(trialEndDateMs ? { inTrial: Date.now() <= trialEndDateMs } : {}), }; }, }); From 48eaafc61032a624a22f6062b56833cca1afa361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Mon, 17 Oct 2022 16:22:09 +0200 Subject: [PATCH 4/4] Address nits --- .../cloud_integrations/cloud_experiments/public/plugin.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 89a214000caba..164d6e45c5294 100755 --- a/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts +++ b/x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts @@ -131,11 +131,17 @@ export class CloudExperimentsPlugin defaultValue: Data ): Promise => { const configKey = FEATURE_FLAG_NAMES[featureFlagName]; + // Apply overrides if they exist without asking LaunchDarkly. if (this.flagOverrides && has(this.flagOverrides, configKey)) { return get(this.flagOverrides, configKey, defaultValue) as Data; } - if (!this.launchDarklyClient) return defaultValue; // Skip any action if no LD Client is defined + + // Skip any action if no LD Client is defined + if (!this.launchDarklyClient) { + return defaultValue; + } + return await this.launchDarklyClient.getVariation(configKey, defaultValue); };