Fix SO migration integration tests (#98478)

* do not restart Kibana server on integration tests writing logs

* unskip tests

* do not write to ended stream to avoid a race condition

* revert changes to the File appender

* fix race condition on the logging_system level

enforce buffer usage for all the logs created during  disposal phase
This commit is contained in:
Mikhail Shustov 2021-04-27 21:00:44 +02:00 committed by GitHub
parent 31660419ea
commit a386fd86f0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 84 additions and 15 deletions

View file

@ -42,21 +42,25 @@ it('produces the right watch and ignore list', () => {
/\\\\\\.\\(md\\|sh\\|txt\\)\\$/,
/debug\\\\\\.log\\$/,
<absolute path>/src/plugins/*/test/**,
<absolute path>/src/plugins/*/integration_tests/**,
<absolute path>/src/plugins/*/build/**,
<absolute path>/src/plugins/*/target/**,
<absolute path>/src/plugins/*/scripts/**,
<absolute path>/src/plugins/*/docs/**,
<absolute path>/test/plugin_functional/plugins/*/test/**,
<absolute path>/test/plugin_functional/plugins/*/integration_tests/**,
<absolute path>/test/plugin_functional/plugins/*/build/**,
<absolute path>/test/plugin_functional/plugins/*/target/**,
<absolute path>/test/plugin_functional/plugins/*/scripts/**,
<absolute path>/test/plugin_functional/plugins/*/docs/**,
<absolute path>/x-pack/plugins/*/test/**,
<absolute path>/x-pack/plugins/*/integration_tests/**,
<absolute path>/x-pack/plugins/*/build/**,
<absolute path>/x-pack/plugins/*/target/**,
<absolute path>/x-pack/plugins/*/scripts/**,
<absolute path>/x-pack/plugins/*/docs/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/test/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/integration_tests/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/build/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/target/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/scripts/**,

View file

@ -28,6 +28,7 @@ export function getServerWatchPaths({ pluginPaths, pluginScanDirs }: Options) {
(acc: string[], path) => [
...acc,
Path.resolve(path, 'test/**'),
Path.resolve(path, 'integration_tests/**'),
Path.resolve(path, 'build/**'),
Path.resolve(path, 'target/**'),
Path.resolve(path, 'scripts/**'),

View file

@ -470,3 +470,59 @@ test('subsequent calls to setContextConfig() for the same context name can disab
},
});
});
test('buffers log records for already created appenders', async () => {
// a default config
await system.upgrade(
config.schema.validate({
appenders: { default: { type: 'console', layout: { type: 'json' } } },
root: { level: 'info' },
})
);
const logger = system.get('test', 'context');
const bufferAppendSpy = jest.spyOn((system as any).bufferAppender, 'append');
const upgradePromise = system.upgrade(
config.schema.validate({
appenders: { default: { type: 'console', layout: { type: 'json' } } },
root: { level: 'all' },
})
);
logger.trace('message to the known context');
expect(bufferAppendSpy).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toHaveBeenCalledTimes(0);
await upgradePromise;
expect(JSON.parse(mockConsoleLog.mock.calls[0][0]).message).toBe('message to the known context');
});
test('buffers log records for appenders created during config upgrade', async () => {
// a default config
await system.upgrade(
config.schema.validate({
appenders: { default: { type: 'console', layout: { type: 'json' } } },
root: { level: 'info' },
})
);
const bufferAppendSpy = jest.spyOn((system as any).bufferAppender, 'append');
const upgradePromise = system.upgrade(
config.schema.validate({
appenders: { default: { type: 'console', layout: { type: 'json' } } },
root: { level: 'all' },
})
);
const logger = system.get('test', 'context');
logger.trace('message to a new context');
expect(bufferAppendSpy).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toHaveBeenCalledTimes(0);
await upgradePromise;
expect(JSON.parse(mockConsoleLog.mock.calls[0][0]).message).toBe('message to a new context');
});

View file

@ -167,17 +167,13 @@ export class LoggingSystem implements LoggerFactory {
}
private async applyBaseConfig(newBaseConfig: LoggingConfig) {
this.enforceBufferAppendersUsage();
const computedConfig = [...this.contextConfigs.values()].reduce(
(baseConfig, contextConfig) => baseConfig.extend(contextConfig),
newBaseConfig
);
// reconfigure all the loggers without configuration to have them use the buffer
// appender while we are awaiting for the appenders to be disposed.
for (const [loggerKey, loggerAdapter] of this.loggers) {
loggerAdapter.updateLogger(this.createLogger(loggerKey, undefined));
}
// Appenders must be reset, so we first dispose of the current ones, then
// build up a new set of appenders.
await Promise.all([...this.appenders.values()].map((a) => a.dispose()));
@ -204,18 +200,32 @@ export class LoggingSystem implements LoggerFactory {
}
}
for (const [loggerKey, loggerAdapter] of this.loggers) {
loggerAdapter.updateLogger(this.createLogger(loggerKey, computedConfig));
}
this.enforceConfiguredAppendersUsage(computedConfig);
// We keep a reference to the base config so we can properly extend it
// on each config change.
this.baseConfig = newBaseConfig;
this.computedConfig = computedConfig;
// Re-log all buffered log records with newly configured appenders.
for (const logRecord of this.bufferAppender.flush()) {
this.get(logRecord.context).log(logRecord);
}
}
// reconfigure all the loggers to have them use the buffer appender
// while we are awaiting for the appenders to be disposed.
private enforceBufferAppendersUsage() {
for (const [loggerKey, loggerAdapter] of this.loggers) {
loggerAdapter.updateLogger(this.createLogger(loggerKey, undefined));
}
// new loggers created during applyBaseConfig execution should use the buffer appender as well
this.computedConfig = undefined;
}
private enforceConfiguredAppendersUsage(config: LoggingConfig) {
for (const [loggerKey, loggerAdapter] of this.loggers) {
loggerAdapter.updateLogger(this.createLogger(loggerKey, config));
}
this.computedConfig = config;
}
}

View file

@ -53,8 +53,7 @@ function createRoot() {
);
}
// CI FAILURE: https://github.com/elastic/kibana/issues/98352
describe.skip('migration v2', () => {
describe('migration v2', () => {
let esServer: kbnTestServer.TestElasticsearchUtils;
let root: Root;

View file

@ -88,8 +88,7 @@ function createRoot() {
);
}
// CI FAILURE: https://github.com/elastic/kibana/issues/98351
describe.skip('migration v2', () => {
describe('migration v2', () => {
let esServer: kbnTestServer.TestElasticsearchUtils;
let root: Root;