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

Introduce LegacyAppender that forwards log records to the legacy platform. #14354

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions platform/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,26 @@ folder and re-run `npm install`.

Make sure to build the code first, e.g. `npm run ts:build` or `npm run ts:start`.

This builds the code into `./ts-tmp/` for now. If you get into a weird state you
might clean the `ts-tmp` directory.
This builds the code into `./target/` for now. If you get into a weird state you
might clean the `target` directory.

When this completes you can start the server and plugins:
When this completes you can start the server and plugins as a standalone Node application:

```
```bash
node scripts/platform.js
```

Or load it as a part of the legacy platform:

```bash
npm start
```

In the latter case, all Kibana requests will hit the new platform first and it will decide whether request can be
solely handled by the new platform or request should be forwarded to the legacy platform. In this mode new platform does
not read config file directly, but rather transforms config provided by the legacy platform. In addition to that all log
records are forwarded to the legacy platform so that it can layout and output them properly.

## Running tests

Run Jest:
Expand Down
8 changes: 3 additions & 5 deletions platform/config/Env.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as process from 'process';
import { resolve } from 'path';

import { LegacyPlatformProxifier } from '../legacy';
import { LegacyKbnServer } from '../legacy';

interface EnvOptions {
config?: string;
Expand Down Expand Up @@ -48,10 +48,8 @@ export class Env {
/**
* @internal
*/
getNewPlatformProxyListener(): LegacyPlatformProxifier | undefined {
if (this.options.kbnServer !== undefined) {
return this.options.kbnServer.newPlatformProxyListener;
}
getLegacyKbnServer(): LegacyKbnServer | undefined {
return this.options.kbnServer;
}

private getDefaultConfigFile() {
Expand Down
10 changes: 5 additions & 5 deletions platform/config/__tests__/Env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ test('correctly creates default environment with empty options.', () => {
expect(defaultEnv.getPluginDir('some-plugin')).toEqual(
'/test/cwd/core_plugins/some-plugin/target/dist'
);
expect(defaultEnv.getNewPlatformProxyListener()).toBeUndefined();
expect(defaultEnv.getLegacyKbnServer()).toBeUndefined();
});

test('correctly creates default environment with options overrides.', () => {
const proxyListenerMock = {};
const kbnServerMock = {};
const defaultEnv = Env.createDefault({
config: '/some/other/path/some-kibana.yml',
kbnServer: { newPlatformProxyListener: proxyListenerMock }
kbnServer: kbnServerMock
});

expect(defaultEnv.homeDir).toEqual('/test/cwd');
Expand All @@ -49,7 +49,7 @@ test('correctly creates default environment with options overrides.', () => {
expect(defaultEnv.getPluginDir('some-plugin')).toEqual(
'/test/cwd/core_plugins/some-plugin/target/dist'
);
expect(defaultEnv.getNewPlatformProxyListener()).toBe(proxyListenerMock);
expect(defaultEnv.getLegacyKbnServer()).toBe(kbnServerMock);
});

test('correctly creates environment with constructor.', () => {
Expand All @@ -70,5 +70,5 @@ test('correctly creates environment with constructor.', () => {
expect(defaultEnv.getPluginDir('some-plugin')).toEqual(
'/some/home/dir/core_plugins/some-plugin/target/dist'
);
expect(defaultEnv.getNewPlatformProxyListener()).toBeUndefined();
expect(defaultEnv.getLegacyKbnServer()).toBeUndefined();
});
1 change: 1 addition & 0 deletions platform/legacy/LegacyKbnServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { LegacyPlatformProxifier } from './LegacyPlatformProxifier';
*/
export interface LegacyKbnServer {
readonly config: LegacyConfig;
readonly server: { log: (...args: any[]) => void };

/**
* Custom HTTP Listener provided by the new platform and that will be used
Expand Down
19 changes: 1 addition & 18 deletions platform/legacy/LegacyPlatformConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class LegacyConfigToRawConfigAdapter implements RawConfig {
private static transformLogging(configValue: LegacyLoggingConfig) {
const loggingConfig = {
root: { level: 'info' },
appenders: { default: {} }
appenders: { default: { kind: 'legacy-appender' } }
};

if (configValue.silent) {
Expand All @@ -80,23 +80,6 @@ export class LegacyConfigToRawConfigAdapter implements RawConfig {
loggingConfig.root.level = 'all';
}

const layout = configValue.json
? { kind: 'json' }
: { kind: 'pattern', highlight: true };

if (configValue.dest && configValue.dest !== 'stdout') {
loggingConfig.appenders.default = {
kind: 'file',
path: configValue.dest,
layout
};
} else {
loggingConfig.appenders.default = {
kind: 'console',
layout
};
}

return loggingConfig;
}

Expand Down
11 changes: 2 additions & 9 deletions platform/legacy/__tests__/LegacyPlatformConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@ describe('Retrieving values', () => {
expect(configAdapter.get('logging')).toEqual({
root: { level: 'off' },
appenders: {
default: {
kind: 'console',
layout: { kind: 'pattern', highlight: true }
}
default: { kind: 'legacy-appender' }
}
});
});
Expand All @@ -54,11 +51,7 @@ describe('Retrieving values', () => {
expect(configAdapter.get('logging')).toEqual({
root: { level: 'all' },
appenders: {
default: {
kind: 'file',
path: '/some/path.log',
layout: { kind: 'json' }
}
default: { kind: 'legacy-appender' }
}
});
});
Expand Down
6 changes: 3 additions & 3 deletions platform/legacy/__tests__/LegacyPlatformProxifier.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class mockNetServer extends EventEmitter {
return { port: 1234, family: 'test-family', address: 'test-address' };
}

getConnections(callback: (error: Error, count: number) => void) {
getConnections(callback: (error: Error | undefined, count: number) => void) {
callback(undefined, 100500);
}
}
Expand Down Expand Up @@ -57,7 +57,7 @@ test('correctly binds to the server and redirects its events.', () => {
const listener = jest.fn();
proxifier.addListener(eventName, listener);

return [eventName, listener];
return [eventName, listener] as [string, () => void];
})
);

Expand Down Expand Up @@ -86,7 +86,7 @@ test('correctly unbinds from the previous server.', () => {
const listener = jest.fn();
proxifier.addListener(eventName, listener);

return [eventName, listener];
return [eventName, listener] as [string, () => void];
})
);

Expand Down
39 changes: 39 additions & 0 deletions platform/legacy/logging/appenders/LegacyAppender.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { Schema, typeOfSchema } from '../../../types/schema';
import { LogRecord } from '../../../logging/LogRecord';
import { DisposableAppender } from '../../../logging/appenders/Appenders';
import { LegacyKbnServer } from '../../LegacyKbnServer';

const createSchema = (schema: Schema) => {
const { literal, object } = schema;

return object({ kind: literal('legacy-appender') });
};

const schemaType = typeOfSchema(createSchema);
/** @internal */
export type LegacyAppenderConfigType = typeof schemaType;

/**
* Simple appender that just forwards `LogRecord` to the legacy KbnServer log.
* @internal
*/
export class LegacyAppender implements DisposableAppender {
static createConfigSchema = createSchema;

constructor(private readonly kbnServer: LegacyKbnServer) {}

/**
* Forwards `LogRecord` to the legacy platform that will layout and
* write record to the configured destination.
* @param record `LogRecord` instance to forward to.
*/
append(record: LogRecord) {
this.kbnServer.server.log(
[record.level.id.toLowerCase(), ...record.context.split('.')],
record.error || record.message,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: it seems in the legacy platform we can't provide both message and error :/

record.timestamp
);
}

async dispose() {}
}
82 changes: 82 additions & 0 deletions platform/legacy/logging/appenders/__tests__/LegacyAppender.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import * as mockSchema from '../../../../lib/schema';
import { LogLevel } from '../../../../logging/LogLevel';
import { LogRecord } from '../../../../logging/LogRecord';
import { LegacyAppender } from '../LegacyAppender';

test('`createConfigSchema()` creates correct schema.', () => {
const appenderSchema = LegacyAppender.createConfigSchema(mockSchema);
const validConfig = { kind: 'legacy-appender' };
expect(appenderSchema.validate(validConfig)).toEqual({
kind: 'legacy-appender'
});

const wrongConfig = { kind: 'not-legacy-appender' };
expect(() => appenderSchema.validate(wrongConfig)).toThrow();
});

test('`append()` correctly pushes records to legacy platform.', () => {
const timestamp = new Date(Date.UTC(2012, 1, 1, 11, 22, 33, 44));
const records: LogRecord[] = [
{
timestamp,
message: 'message-1',
context: 'context-1',
level: LogLevel.Trace
},
{
timestamp,
message: 'message-2',
context: 'context-2',
level: LogLevel.Debug
},
{
timestamp,
message: 'message-3',
context: 'context-3.sub-context-3',
level: LogLevel.Info
},
{
timestamp,
message: 'message-4',
context: 'context-4.sub-context-4',
level: LogLevel.Warn
},
{
timestamp,
message: 'message-5-with-error',
context: 'context-5',
error: new Error('Some Error'),
level: LogLevel.Error
},
{
timestamp,
message: 'message-6-with-message',
context: 'context-6',
level: LogLevel.Error
},
{
timestamp,
message: 'message-7-with-error',
context: 'context-7.sub-context-7.sub-sub-context-7',
error: new Error('Some Fatal Error'),
level: LogLevel.Fatal
},
{
timestamp,
message: 'message-8-with-message',
context: 'context-8.sub-context-8.sub-sub-context-8',
level: LogLevel.Fatal
}
];

const legacyLogStub = jest.fn();
const appender = new LegacyAppender(
{ server: { log: legacyLogStub } } as any
);

for (const record of records) {
appender.append(record);
}

expect(legacyLogStub.mock.calls).toMatchSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great use of snapshots imo 🎉

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it works pretty well for cases when arguments are predictable, borrowed the approach from your logger mock's _collect method :)

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`\`append()\` correctly pushes records to legacy platform. 1`] = `
Array [
Array [
Array [
"trace",
"context-1",
],
"message-1",
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"debug",
"context-2",
],
"message-2",
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"info",
"context-3",
"sub-context-3",
],
"message-3",
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"warn",
"context-4",
"sub-context-4",
],
"message-4",
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"error",
"context-5",
],
[Error: Some Error],
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"error",
"context-6",
],
"message-6-with-message",
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"fatal",
"context-7",
"sub-context-7",
"sub-sub-context-7",
],
[Error: Some Fatal Error],
2012-02-01T11:22:33.044Z,
],
Array [
Array [
"fatal",
"context-8",
"sub-context-8",
"sub-sub-context-8",
],
"message-8-with-message",
2012-02-01T11:22:33.044Z,
],
]
`;
Loading