Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[telemetry] report config deprecations #99887

Merged
merged 12 commits into from
May 26, 2021
2 changes: 2 additions & 0 deletions packages/kbn-config/src/config_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 9 additions & 2 deletions packages/kbn-config/src/config_service.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
25 changes: 20 additions & 5 deletions packages/kbn-config/src/config_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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);
});
});
12 changes: 11 additions & 1 deletion packages/kbn-config/src/config_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ConfigDeprecationProvider,
configDeprecationFactory,
DeprecatedConfigDetails,
ChangedDeprecatedPaths,
} from './deprecation';
import { LegacyObjectToConfigAdapter } from './legacy';

Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
29 changes: 25 additions & 4 deletions packages/kbn-config/src/deprecation/apply_deprecations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')),
]);
Expand All @@ -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')),
]);

Expand All @@ -110,7 +110,7 @@ describe('applyDeprecations', () => {
return { unset: [{ path: 'unknown' }] };
});

const migrated = applyDeprecations(
const { config: migrated } = applyDeprecations(
initialConfig,
[wrapHandler(handler, 'pathA')],
createAddDeprecation
Expand All @@ -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'],
});
});
});
19 changes: 16 additions & 3 deletions packages/kbn-config/src/deprecation/apply_deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand All @@ -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);
Bamieh marked this conversation as resolved.
Show resolved Hide resolved
});
}
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,
};
};
1 change: 1 addition & 0 deletions packages/kbn-config/src/deprecation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type {
AddConfigDeprecation,
ConfigDeprecationProvider,
DeprecatedConfigDetails,
ChangedDeprecatedPaths,
} from './types';
export { configDeprecationFactory } from './deprecation_factory';
export { applyDeprecations } from './apply_deprecations';
10 changes: 10 additions & 0 deletions packages/kbn-config/src/deprecation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type {
ConfigDeprecationWithContext,
ConfigDeprecation,
ConfigDeprecationCommand,
ChangedDeprecatedPaths,
} from './deprecation';

export { applyDeprecations, configDeprecationFactory } from './deprecation';
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/config/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ const createStartContractMock = () => {
maxImportExportSize: 10000,
maxImportPayloadBytes: 26214400,
},
deprecatedKeys: {
set: ['path.to.a.prop'],
unset: [],
},
},
environment: {
memory: {
Expand Down
Loading