Skip to content

Commit

Permalink
Add debounceTime to the metadata service
Browse files Browse the repository at this point in the history
  • Loading branch information
@afharo
afharo committed Oct 11, 2022
1 parent 4aa4a95 commit ba4f6ee
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 80 deletions.
19 changes: 10 additions & 9 deletions ...ins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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(
Expand All @@ -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',
Expand Down
37 changes: 29 additions & 8 deletions .../plugins/cloud_integrations/cloud_experiments/common/metadata_service/metadata_service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -28,13 +40,13 @@ export interface MetadataServiceConfig {
}

export class MetadataService {
public readonly userMetadata$ = new BehaviorSubject<UserMetadata | undefined>(undefined);
private readonly _userMetadata$ = new BehaviorSubject<UserMetadata | undefined>(undefined);
private readonly stop$ = new Subject<void>();

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.
Expand All @@ -48,9 +60,18 @@ export class MetadataService {
}
}

public get userMetadata$(): Observable<UserMetadata> {
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(),
Expand All @@ -61,7 +82,7 @@ export class MetadataService {

public stop() {
this.stop$.next();
this.userMetadata$.complete();
this._userMetadata$.complete();
}

/**
Expand All @@ -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();
Expand Down
44 changes: 31 additions & 13 deletions x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof LaunchDarklyClient>
).mock.instances[0] as jest.Mocked<LaunchDarklyClient>;

return launchDarklyClientInstanceMock;
}

describe('Cloud Experiments public plugin', () => {
jest.spyOn(console, 'debug').mockImplementation(); // silence console.debug logs

Expand All @@ -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', () => {
Expand Down Expand Up @@ -66,7 +76,7 @@ describe('Cloud Experiments public plugin', () => {

describe('setup', () => {
let plugin: CloudExperimentsPlugin;
let launchDarklyInstanceMock: jest.Mocked<LaunchDarklyClient>;
let metadataServiceSetupSpy: jest.SpyInstance;

beforeEach(() => {
const initializerContext = coreMock.createPluginInitializerContext({
Expand All @@ -75,8 +85,8 @@ describe('Cloud Experiments public plugin', () => {
metadata_refresh_interval: duration(1, 'h'),
});
plugin = new CloudExperimentsPlugin(initializerContext);
launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass<typeof LaunchDarklyClient>)
.mock.instances[0] as jest.Mocked<LaunchDarklyClient>;
// eslint-disable-next-line dot-notation
metadataServiceSetupSpy = jest.spyOn(plugin['metadataService'], 'setup');
});

afterEach(() => {
Expand Down Expand Up @@ -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',
});
});
});
});
Expand All @@ -135,8 +149,7 @@ describe('Cloud Experiments public plugin', () => {
flag_overrides: { [firstKnownFlag]: '1234' },
});
plugin = new CloudExperimentsPlugin(initializerContext);
launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass<typeof LaunchDarklyClient>)
.mock.instances[0] as jest.Mocked<LaunchDarklyClient>;
launchDarklyInstanceMock = getLaunchDarklyClientInstanceMock();
});

afterEach(() => {
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(),
});
});
Expand Down Expand Up @@ -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(),
});
});
Expand Down Expand Up @@ -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(),
});
});
Expand Down Expand Up @@ -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(),
});
});
Expand Down Expand Up @@ -311,12 +329,12 @@ describe('Cloud Experiments public plugin', () => {
metadata_refresh_interval: duration(1, 'h'),
});
plugin = new CloudExperimentsPlugin(initializerContext);
launchDarklyInstanceMock = (LaunchDarklyClient as jest.MockedClass<typeof LaunchDarklyClient>)
.mock.instances[0] as jest.Mocked<LaunchDarklyClient>;
launchDarklyInstanceMock = getLaunchDarklyClientInstanceMock();
plugin.setup(coreMock.createSetup(), {
cloud: { ...cloudMock.createSetup(), isCloudEnabled: true },
});
plugin.start(coreMock.createStart(), {
cloud: cloudMock.createStart(),
dataViews: dataViewPluginMocks.createStartContract(),
});
});
Expand Down
34 changes: 18 additions & 16 deletions x-pack/plugins/cloud_integrations/cloud_experiments/public/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -26,6 +26,7 @@ interface CloudExperimentsPluginSetupDeps {
}

interface CloudExperimentsPluginStartDeps {
cloud: CloudStart;
dataViews: DataViewsPublicPluginStart;
}

Expand Down Expand Up @@ -84,32 +85,33 @@ 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)
)
)
.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,
Expand Down
Loading

0 comments on commit ba4f6ee

Please sign in to comment.