[telemetry] report config deprecations (elastic#99887)

* return the list of changes config keys during deprecation * gather changed config keys in the core * adjust Security plugin deprecations tests * update docs * update interface * update telemetry schema * update spaces tests * update tests in other x-pack plugins * remove testing instruction * improve tests. get rid of snapshots
ecezalp · May 26, 2021 · c987129 · c987129
1 parent eacaefd
commit c987129
Show file tree

Hide file tree

Showing 22 changed files with 269 additions and 145 deletions.
diff --git a/packages/kbn-config/src/config_service.mock.ts b/packages/kbn-config/src/config_service.mock.ts
@@ -26,11 +26,13 @@ const createConfigServiceMock = ({
     addDeprecationProvider: jest.fn(),
     validate: jest.fn(),
     getHandledDeprecatedConfigs: jest.fn(),
+    getDeprecatedConfigPath$: jest.fn(),
   };
 
   mocked.atPath.mockReturnValue(new BehaviorSubject(atPath));
   mocked.atPathSync.mockReturnValue(atPath);
   mocked.getConfig$.mockReturnValue(new BehaviorSubject(new ObjectToConfigAdapter(getConfig$)));
+  mocked.getDeprecatedConfigPath$.mockReturnValue(new BehaviorSubject({ set: [], unset: [] }));
   mocked.getUsedPaths.mockResolvedValue([]);
   mocked.getUnusedPaths.mockResolvedValue([]);
   mocked.isEnabledAtPath.mockResolvedValue(true);

diff --git a/packages/kbn-config/src/config_service.test.mocks.ts b/packages/kbn-config/src/config_service.test.mocks.ts
@@ -11,10 +11,17 @@ import type { applyDeprecations } from './deprecation/apply_deprecations';
 
 jest.mock('../../../package.json', () => mockPackage);
 
+const changedPaths = {
+  set: ['foo'],
+  unset: ['bar.baz'],
+};
+
+export { changedPaths as mockedChangedPaths };
+
 export const mockApplyDeprecations = jest.fn<
-  Record<string, any>,
+  ReturnType<typeof applyDeprecations>,
   Parameters<typeof applyDeprecations>
->((config, deprecations, createAddDeprecation) => config);
+>((config, deprecations, createAddDeprecation) => ({ config, changedPaths }));
 
 jest.mock('./deprecation/apply_deprecations', () => ({
   applyDeprecations: mockApplyDeprecations,

diff --git a/packages/kbn-config/src/config_service.test.ts b/packages/kbn-config/src/config_service.test.ts
@@ -9,7 +9,7 @@
 import { BehaviorSubject, Observable } from 'rxjs';
 import { first, take } from 'rxjs/operators';
 
-import { mockApplyDeprecations } from './config_service.test.mocks';
+import { mockApplyDeprecations, mockedChangedPaths } from './config_service.test.mocks';
 import { rawConfigServiceMock } from './raw/raw_config_service.mock';
 
 import { schema } from '@kbn/config-schema';
@@ -420,7 +420,7 @@ test('logs deprecation warning during validation', async () => {
     const addDeprecation = createAddDeprecation!('');
     addDeprecation({ message: 'some deprecation message' });
     addDeprecation({ message: 'another deprecation message' });
-    return config;
+    return { config, changedPaths: mockedChangedPaths };
   });
 
   loggerMock.clear(logger);
@@ -446,12 +446,12 @@ test('does not log warnings for silent deprecations during validation', async ()
       const addDeprecation = createAddDeprecation!('');
       addDeprecation({ message: 'some deprecation message', silent: true });
       addDeprecation({ message: 'another deprecation message' });
-      return config;
+      return { config, changedPaths: mockedChangedPaths };
     })
     .mockImplementationOnce((config, deprecations, createAddDeprecation) => {
       const addDeprecation = createAddDeprecation!('');
       addDeprecation({ message: 'I am silent', silent: true });
-      return config;
+      return { config, changedPaths: mockedChangedPaths };
     });
 
   loggerMock.clear(logger);
@@ -521,7 +521,7 @@ describe('getHandledDeprecatedConfigs', () => {
         const addDeprecation = createAddDeprecation!(deprecation.path);
         addDeprecation({ message: `some deprecation message`, documentationUrl: 'some-url' });
       });
-      return config;
+      return { config, changedPaths: mockedChangedPaths };
     });
 
     await configService.validate();
@@ -541,3 +541,18 @@ describe('getHandledDeprecatedConfigs', () => {
     `);
   });
 });
+
+describe('getDeprecatedConfigPath$', () => {
+  it('returns all config paths changes during deprecation', async () => {
+    const rawConfig$ = new BehaviorSubject<Record<string, any>>({ key: 'value' });
+    const rawConfigProvider = rawConfigServiceMock.create({ rawConfig$ });
+
+    const configService = new ConfigService(rawConfigProvider, defaultEnv, logger);
+    await configService.setSchema('key', schema.string());
+    await configService.validate();
+
+    const deprecatedConfigPath$ = configService.getDeprecatedConfigPath$();
+    const deprecatedConfigPath = await deprecatedConfigPath$.pipe(first()).toPromise();
+    expect(deprecatedConfigPath).toEqual(mockedChangedPaths);
+  });
+});
diff --git a/packages/kbn-config/src/config_service.ts b/packages/kbn-config/src/config_service.ts
@@ -22,6 +22,7 @@ import {
   ConfigDeprecationProvider,
   configDeprecationFactory,
   DeprecatedConfigDetails,
+  ChangedDeprecatedPaths,
 } from './deprecation';
 import { LegacyObjectToConfigAdapter } from './legacy';
 
@@ -36,6 +37,10 @@ export class ConfigService {
   private validated = false;
   private readonly config$: Observable<Config>;
   private lastConfig?: Config;
+  private readonly deprecatedConfigPaths = new BehaviorSubject<ChangedDeprecatedPaths>({
+    set: [],
+    unset: [],
+  });
 
   /**
    * Whenever a config if read at a path, we mark that path as 'handled'. We can
@@ -57,7 +62,8 @@ export class ConfigService {
     this.config$ = combineLatest([this.rawConfigProvider.getConfig$(), this.deprecations]).pipe(
       map(([rawConfig, deprecations]) => {
         const migrated = applyDeprecations(rawConfig, deprecations);
-        return new LegacyObjectToConfigAdapter(migrated);
+        this.deprecatedConfigPaths.next(migrated.changedPaths);
+        return new LegacyObjectToConfigAdapter(migrated.config);
       }),
       tap((config) => {
         this.lastConfig = config;
@@ -191,6 +197,10 @@ export class ConfigService {
     return config.getFlattenedPaths().filter((path) => isPathHandled(path, handledPaths));
   }
 
+  public getDeprecatedConfigPath$() {
+    return this.deprecatedConfigPaths.asObservable();
+  }
+
   private async logDeprecation() {
     const rawConfig = await this.rawConfigProvider.getConfig$().pipe(take(1)).toPromise();
     const deprecations = await this.deprecations.pipe(take(1)).toPromise();

diff --git a/packages/kbn-config/src/deprecation/apply_deprecations.test.ts b/packages/kbn-config/src/deprecation/apply_deprecations.test.ts
@@ -82,7 +82,7 @@ describe('applyDeprecations', () => {
   it('returns the migrated config', () => {
     const initialConfig = { foo: 'bar', deprecated: 'deprecated', renamed: 'renamed' };
 
-    const migrated = applyDeprecations(initialConfig, [
+    const { config: migrated } = applyDeprecations(initialConfig, [
       wrapHandler(deprecations.unused('deprecated')),
       wrapHandler(deprecations.rename('renamed', 'newname')),
     ]);
@@ -93,7 +93,7 @@ describe('applyDeprecations', () => {
   it('does not alter the initial config', () => {
     const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
 
-    const migrated = applyDeprecations(initialConfig, [
+    const { config: migrated } = applyDeprecations(initialConfig, [
       wrapHandler(deprecations.unused('deprecated')),
     ]);
 
@@ -110,7 +110,7 @@ describe('applyDeprecations', () => {
       return { unset: [{ path: 'unknown' }] };
     });
 
-    const migrated = applyDeprecations(
+    const { config: migrated } = applyDeprecations(
       initialConfig,
       [wrapHandler(handler, 'pathA')],
       createAddDeprecation
@@ -128,12 +128,33 @@ describe('applyDeprecations', () => {
       return { rewrite: [{ path: 'foo' }] };
     });
 
-    const migrated = applyDeprecations(
+    const { config: migrated } = applyDeprecations(
       initialConfig,
       [wrapHandler(handler, 'pathA')],
       createAddDeprecation
     );
 
     expect(migrated).toEqual(initialConfig);
   });
+
+  it('returns a list of changes config paths', () => {
+    const addDeprecation = jest.fn();
+    const createAddDeprecation = jest.fn().mockReturnValue(addDeprecation);
+    const initialConfig = { foo: 'bar', deprecated: 'deprecated' };
+
+    const handler = jest.fn().mockImplementation((config) => {
+      return { set: [{ path: 'foo', value: 'bar' }], unset: [{ path: 'baz' }] };
+    });
+
+    const { changedPaths } = applyDeprecations(
+      initialConfig,
+      [wrapHandler(handler, 'pathA')],
+      createAddDeprecation
+    );
+
+    expect(changedPaths).toEqual({
+      set: ['foo'],
+      unset: ['baz'],
+    });
+  });
 });
diff --git a/packages/kbn-config/src/deprecation/apply_deprecations.ts b/packages/kbn-config/src/deprecation/apply_deprecations.ts
@@ -8,7 +8,11 @@
 
 import { cloneDeep, unset } from 'lodash';
 import { set } from '@elastic/safer-lodash-set';
-import { ConfigDeprecationWithContext, AddConfigDeprecation } from './types';
+import type {
+  AddConfigDeprecation,
+  ChangedDeprecatedPaths,
+  ConfigDeprecationWithContext,
+} from './types';
 
 const noopAddDeprecationFactory: () => AddConfigDeprecation = () => () => undefined;
 /**
@@ -22,22 +26,31 @@ export const applyDeprecations = (
   config: Record<string, any>,
   deprecations: ConfigDeprecationWithContext[],
   createAddDeprecation: (pluginId: string) => AddConfigDeprecation = noopAddDeprecationFactory
-) => {
+): { config: Record<string, any>; changedPaths: ChangedDeprecatedPaths } => {
   const result = cloneDeep(config);
+  const changedPaths: ChangedDeprecatedPaths = {
+    set: [],
+    unset: [],
+  };
   deprecations.forEach(({ deprecation, path }) => {
     const commands = deprecation(result, path, createAddDeprecation(path));
     if (commands) {
       if (commands.set) {
+        changedPaths.set.push(...commands.set.map((c) => c.path));
         commands.set.forEach(function ({ path: commandPath, value }) {
           set(result, commandPath, value);
         });
       }
       if (commands.unset) {
+        changedPaths.unset.push(...commands.unset.map((c) => c.path));
         commands.unset.forEach(function ({ path: commandPath }) {
           unset(result, commandPath);
         });
       }
     }
   });
-  return result;
+  return {
+    config: result,
+    changedPaths,
+  };
 };
diff --git a/packages/kbn-config/src/deprecation/index.ts b/packages/kbn-config/src/deprecation/index.ts
@@ -14,6 +14,7 @@ export type {
   AddConfigDeprecation,
   ConfigDeprecationProvider,
   DeprecatedConfigDetails,
+  ChangedDeprecatedPaths,
 } from './types';
 export { configDeprecationFactory } from './deprecation_factory';
 export { applyDeprecations } from './apply_deprecations';
diff --git a/packages/kbn-config/src/deprecation/types.ts b/packages/kbn-config/src/deprecation/types.ts
@@ -55,6 +55,16 @@ export type ConfigDeprecation = (
   addDeprecation: AddConfigDeprecation
 ) => void | ConfigDeprecationCommand;
 
+/**
+ * List of config paths changed during deprecation.
+ *
+ * @public
+ */
+export interface ChangedDeprecatedPaths {
+  set: string[];
+  unset: string[];
+}
+
 /**
  * Outcome of deprecation operation. Allows mutating config values in a declarative way.
  *

diff --git a/packages/kbn-config/src/index.ts b/packages/kbn-config/src/index.ts
@@ -13,6 +13,7 @@ export type {
   ConfigDeprecationWithContext,
   ConfigDeprecation,
   ConfigDeprecationCommand,
+  ChangedDeprecatedPaths,
 } from './deprecation';
 
 export { applyDeprecations, configDeprecationFactory } from './deprecation';

diff --git a/src/core/server/config/test_utils.ts b/src/core/server/config/test_utils.ts
@@ -16,7 +16,7 @@ function collectDeprecations(
 ) {
   const deprecations = provider(configDeprecationFactory);
   const deprecationMessages: string[] = [];
-  const migrated = applyDeprecations(
+  const { config: migrated } = applyDeprecations(
     settings,
     deprecations.map((deprecation) => ({
       deprecation,

diff --git a/src/core/server/core_usage_data/core_usage_data_service.mock.ts b/src/core/server/core_usage_data/core_usage_data_service.mock.ts
@@ -116,6 +116,10 @@ const createStartContractMock = () => {
             maxImportExportSize: 10000,
             maxImportPayloadBytes: 26214400,
           },
+          deprecatedKeys: {
+            set: ['path.to.a.prop'],
+            unset: [],
+          },
         },
         environment: {
           memory: {