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 1 commit
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
27 changes: 15 additions & 12 deletions platform/legacy/LegacyKbnServer.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
import { LegacyConfig } from './LegacyPlatformConfig';
import { LegacyPlatformProxifier } from './LegacyPlatformProxifier';

/**
* Represents legacy kbnServer instance, provided by the legacy platform.
* Represents a wrapper around legacy `kbnServer` instance that exposes only
* a subset of `kbnServer` APIs used by the new platform.
* @internal
*/
export interface LegacyKbnServer {
readonly config: LegacyConfig;
readonly server: { log: (...args: any[]) => void };
export class LegacyKbnServer {
constructor(private readonly rawKbnServer: any) {}

/**
* Custom HTTP Listener provided by the new platform and that will be used
* within legacy platform by HapiJS server.
* Custom HTTP Listener used by HapiJS server in the legacy platform.
*/
newPlatformProxyListener: LegacyPlatformProxifier;
get newPlatformProxyListener() {
return this.rawKbnServer.newPlatform.proxyListener;
}

/**
* Propagates legacy config updates to the new platform.
* Forwards log request to the legacy platform.
* @param tags A string or array of strings used to briefly identify the event.
* @param [data] Optional string or object to log with the event.
* @param [timestamp] Timestamp value associated with the log record.
*/
updateNewPlatformConfig: (legacyConfig: LegacyConfig) => void;
log(tags: string | string[], data?: string | Error, timestamp?: Date) {
this.rawKbnServer.server.log(tags, data, timestamp);
}
}
28 changes: 28 additions & 0 deletions platform/legacy/__tests__/LegacyKbnServer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { LegacyKbnServer } from '..';

it('correctly returns `newPlatformProxyListener`.', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: it -> test?

Copy link
Contributor

Choose a reason for hiding this comment

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

(I'm not sure here, as I don't know if we are consistent either way, so feel free to skip)

Copy link
Member Author

Choose a reason for hiding this comment

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

ha-ha, right, it should definitely be test - I believe we are pretty consistent on this in the new platform code (I see only 3 more tests that use it and that was me who did that :), so I'll change them to test too).

const rawKbnServer = {
newPlatform: {
proxyListener: {}
}
};

const legacyKbnServer = new LegacyKbnServer(rawKbnServer);
expect(legacyKbnServer.newPlatformProxyListener).toBe(
rawKbnServer.newPlatform.proxyListener
);
});

it('correctly forwards log record.', () => {
const rawKbnServer = {
server: { log: jest.fn() }
};

const legacyKbnServer = new LegacyKbnServer(rawKbnServer);

const timestamp = new Date(Date.UTC(2012, 1, 1, 11, 22, 33, 44));
legacyKbnServer.log(['one', 'two'], 'message', timestamp);
legacyKbnServer.log('three', new Error('log error'), timestamp);

expect(rawKbnServer.server.log.mock.calls).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`correctly forwards log record. 1`] = `
Array [
Array [
Array [
"one",
"two",
],
"message",
2012-02-01T11:22:33.044Z,
],
Array [
"three",
[Error: log error],
2012-02-01T11:22:33.044Z,
],
]
`;
23 changes: 15 additions & 8 deletions platform/legacy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,24 @@ import {
/**
* @internal
*/
export const injectIntoKbnServer = (kbnServer: LegacyKbnServer) => {
const legacyConfig$ = new BehaviorSubject(kbnServer.config);
export const injectIntoKbnServer = (rawKbnServer: any) => {
const legacyConfig$ = new BehaviorSubject(rawKbnServer.config);
const config$ = legacyConfig$.map(
legacyConfig => new LegacyConfigToRawConfigAdapter(legacyConfig)
);

kbnServer.updateNewPlatformConfig = (legacyConfig: LegacyConfig) => {
legacyConfig$.next(legacyConfig);
};
rawKbnServer.newPlatform = {
// Custom HTTP Listener that will be used within legacy platform by HapiJS server.
proxyListener: new LegacyPlatformProxifier(
new Root(
config$,
Env.createDefault({ kbnServer: new LegacyKbnServer(rawKbnServer) })
)
),

kbnServer.newPlatformProxyListener = new LegacyPlatformProxifier(
new Root(config$, Env.createDefault({ kbnServer }))
);
// Propagates legacy config updates to the new platform.
updateConfig(legacyConfig: LegacyConfig) {
legacyConfig$.next(legacyConfig);
}
};
};
2 changes: 1 addition & 1 deletion platform/legacy/logging/appenders/LegacyAppender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class LegacyAppender implements DisposableAppender {
* @param record `LogRecord` instance to forward to.
*/
append(record: LogRecord) {
this.kbnServer.server.log(
this.kbnServer.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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as mockSchema from '../../../../lib/schema';
import { LogLevel } from '../../../../logging/LogLevel';
import { LogRecord } from '../../../../logging/LogRecord';
import { LegacyKbnServer } from '../../../LegacyKbnServer';
import { LegacyAppender } from '../LegacyAppender';

test('`createConfigSchema()` creates correct schema.', () => {
Expand Down Expand Up @@ -69,14 +70,14 @@ test('`append()` correctly pushes records to legacy platform.', () => {
}
];

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

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

expect(legacyLogStub.mock.calls).toMatchSnapshot();
expect(rawKbnServerMock.server.log.mock.calls).toMatchSnapshot();
});
4 changes: 2 additions & 2 deletions src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ export default function (program) {
kbnServer.server.log(['info', 'config'], 'Reloaded logging configuration due to SIGHUP.');

// If new platform config subscription is active, let's notify it with the updated config.
if (kbnServer.updateNewPlatformConfig) {
kbnServer.updateNewPlatformConfig(config);
if (kbnServer.newPlatform) {
kbnServer.newPlatform.updateConfig(config);
}
});

Expand Down
3 changes: 2 additions & 1 deletion src/server/http/setup_connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import { map } from 'lodash';
import secureOptions from './secure_options';

export default function (kbnServer, server, config) {
const newPlatformProxyListener = kbnServer && kbnServer.newPlatformProxyListener;
const newPlatformProxyListener = kbnServer && kbnServer.newPlatform
&& kbnServer.newPlatform.proxyListener;

// this mixin is used outside of the kbn server, so it MUST work without a full kbnServer object.
kbnServer = null;
Expand Down