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

Implement Logger.isLevelEnabled #144033

Merged
merged 5 commits into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export function createLogger(isDev: boolean): Logger {
trace: (...args) => (isDev ? console.trace(...args) : void 0),
// eslint-disable-next-line no-console
log: (...args) => (isDev ? console.log(...args) : void 0),
isLevelEnabled: () => true,
get: () => logger,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,25 @@ test('passes log record to appenders only if log level is supported.', () => {
});
}
});

describe('isLevelEnabled', () => {
const orderedLogLevels = [
LogLevel.Fatal,
LogLevel.Error,
LogLevel.Warn,
LogLevel.Info,
LogLevel.Debug,
LogLevel.Trace,
LogLevel.All,
];

for (const logLevel of orderedLogLevels) {
it(`returns the correct value for a '${logLevel.id}' level logger`, () => {
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory);
for (const level of orderedLogLevels) {
const levelEnabled = logLevel.supports(level);
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled);
}
});
}
Comment on lines +444 to +452
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
for (const logLevel of orderedLogLevels) {
it(`returns the correct value for a '${logLevel.id}' level logger`, () => {
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory);
for (const level of orderedLogLevels) {
const levelEnabled = logLevel.supports(level);
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled);
}
});
}
it.each(orderedLogLevels)(`returns the correct value for a '$id' level logger`, (logLevel) => {
const levelLogger = new BaseLogger(context, logLevel, appenderMocks, factory);
for (const level of orderedLogLevels) {
const levelEnabled = logLevel.supports(level);
expect(levelLogger.isLevelEnabled(level.id)).toEqual(levelEnabled);
}
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@
* Side Public License, v 1.
*/
import apmAgent from 'elastic-apm-node';
import { Appender, LogLevel, LogRecord, LoggerFactory, LogMeta, Logger } from '@kbn/logging';
import {
Appender,
LogLevel,
LogLevelId,
LogRecord,
LoggerFactory,
LogMeta,
Logger,
} from '@kbn/logging';

function isError(x: any): x is Error {
return x instanceof Error;
Expand Down Expand Up @@ -45,6 +53,10 @@ export class BaseLogger implements Logger {
this.log(this.createLogRecord<Meta>(LogLevel.Fatal, errorOrMessage, meta));
}

public isLevelEnabled(levelId: LogLevelId): boolean {
return this.level.supports(LogLevel.fromId(levelId));
}

public log(record: LogRecord) {
if (!this.level.supports(record.level)) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
*/

import type { Logger } from '@kbn/logging';
import { loggerMock } from '@kbn/logging-mocks';
import { loggerMock, type MockedLogger } from '@kbn/logging-mocks';
import { LoggerAdapter } from './logger_adapter';

describe('LoggerAdapter', () => {
let internalLogger: Logger;
let internalLogger: MockedLogger;

beforeEach(() => {
internalLogger = loggerMock.create();
Expand Down Expand Up @@ -47,6 +47,11 @@ describe('LoggerAdapter', () => {
adapter.get('context');
expect(internalLogger.get).toHaveBeenCalledTimes(1);
expect(internalLogger.get).toHaveBeenCalledWith('context');

internalLogger.isLevelEnabled.mockReturnValue(false);
expect(adapter.isLevelEnabled('info')).toEqual(false);
expect(internalLogger.isLevelEnabled).toHaveBeenCalledTimes(1);
expect(internalLogger.isLevelEnabled).toHaveBeenCalledWith('info');
});

test('forwards all method calls to new internal logger if it is updated.', () => {
Expand All @@ -57,7 +62,7 @@ describe('LoggerAdapter', () => {
adapter.trace('trace-message');
expect(internalLogger.trace).toHaveBeenCalledTimes(1);
expect(internalLogger.trace).toHaveBeenCalledWith('trace-message', undefined);
(internalLogger.trace as jest.Mock<() => void>).mockReset();
internalLogger.trace.mockReset();

adapter.updateLogger(newInternalLogger);
adapter.trace('trace-message');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { LogRecord, Logger, LogMeta } from '@kbn/logging';
import { LogRecord, Logger, LogMeta, LogLevelId } from '@kbn/logging';
import { GlobalContext, mergeGlobalContext } from './global_context';

/** @internal */
Expand Down Expand Up @@ -65,6 +65,10 @@ export class LoggerAdapter implements Logger {
this.logger.log({ ...record, meta: mergeGlobalContext(this.globalContext, record.meta) });
}

public isLevelEnabled(level: LogLevelId): boolean {
return this.logger.isLevelEnabled(level);
}

public get(...contextParts: string[]): Logger {
return this.logger.get(...contextParts);
}
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-cli-dev-mode/src/log_adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const convertToLogger = (cliLog: Log): Logger => {
error: (msgOrError) => cliLog.bad('error', getErrorMessage(msgOrError)),
fatal: (msgOrError) => cliLog.bad('fatal', getErrorMessage(msgOrError)),
log: (record) => cliLog.write(record.message),
isLevelEnabled: () => true,
get: () => adapter,
};
return adapter;
Expand Down
4 changes: 3 additions & 1 deletion packages/kbn-logging-mocks/src/logger.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { Logger } from '@kbn/logging';
import type { Logger } from '@kbn/logging';

export type MockedLogger = jest.Mocked<Logger> & { context: string[] };

Expand All @@ -21,11 +21,13 @@ const createLoggerMock = (context: string[] = []) => {
trace: jest.fn(),
warn: jest.fn(),
get: jest.fn(),
isLevelEnabled: jest.fn(),
};
mockLog.get.mockImplementation((...ctx) => ({
ctx,
...mockLog,
}));
mockLog.isLevelEnabled.mockReturnValue(true);

return mockLog;
};
Expand Down
21 changes: 19 additions & 2 deletions packages/kbn-logging/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
* Side Public License, v 1.
*/

import { LogMeta } from './log_meta';
import { LogRecord } from './log_record';
import type { LogMeta } from './log_meta';
import type { LogRecord } from './log_record';
import type { LogLevelId } from './log_level';

/**
* Logger exposes all the necessary methods to log any type of information and
Expand Down Expand Up @@ -64,6 +65,22 @@ export interface Logger {
/** @internal */
log(record: LogRecord): void;

/**
* Checks if given level is currently enabled for this logger.
* Can be used to wrap expensive logging operations into conditional blocks
*
* @example
* ```ts
* if(logger.isLevelEnabled('info')) {
* const meta = await someExpensiveOperation();
* logger.info('some message', meta);
* }
* ```
*
* @param level The log level to check for.
*/
isLevelEnabled(level: LogLevelId): boolean;

/**
* Returns a new {@link Logger} instance extending the current logger context.
*
Expand Down
1 change: 1 addition & 0 deletions src/cli_setup/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const logger: Logger = {
fatal: noop,
log: noop,
get: () => logger,
isLevelEnabled: () => true,
};

export const kibanaConfigWriter = new KibanaConfigWriter(getConfigPath(), getDataPath(), logger);
Expand Down
1 change: 1 addition & 0 deletions src/plugins/expressions/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class ExpressionsPublicPlugin implements Plugin<ExpressionsSetup, Express
get() {
return this;
},
isLevelEnabled: () => true,
};

private readonly expressions: ExpressionsService = new ExpressionsService({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ describe('send_email module', () => {
"fatal": [MockFunction],
"get": [MockFunction],
"info": [MockFunction],
"isLevelEnabled": [MockFunction],
"log": [MockFunction],
"trace": [MockFunction],
"warn": [MockFunction],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ describe('execute()', () => {
"fatal": [MockFunction],
"get": [MockFunction],
"info": [MockFunction],
"isLevelEnabled": [MockFunction],
"log": [MockFunction],
"trace": [MockFunction],
"warn": [MockFunction],
Expand Down Expand Up @@ -249,6 +250,7 @@ describe('execute()', () => {
"fatal": [MockFunction],
"get": [MockFunction],
"info": [MockFunction],
"isLevelEnabled": [MockFunction],
"log": [MockFunction],
"trace": [MockFunction],
"warn": [MockFunction],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ describe('execute()', () => {
"fatal": [MockFunction],
"get": [MockFunction],
"info": [MockFunction],
"isLevelEnabled": [MockFunction],
"log": [MockFunction],
"trace": [MockFunction],
"warn": [MockFunction],
Expand Down Expand Up @@ -389,6 +390,7 @@ describe('execute()', () => {
"fatal": [MockFunction],
"get": [MockFunction],
"info": [MockFunction],
"isLevelEnabled": [MockFunction],
"log": [MockFunction],
"trace": [MockFunction],
"warn": [MockFunction],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ export const createFakeLogger = (log: ToolingLog) => {
fatal: fakeLogger,
log: sinon.stub(),
get: sinon.stub(),
isLevelEnabled: sinon.stub(),
} as Logger;
};
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export default function createLifecycleExecutorApiTest({ getService }: FtrProvid
fatal: fakeLogger,
log: sinon.stub(),
get: sinon.stub(),
isLevelEnabled: sinon.stub(),
} as Logger;

const getClusterClient = () => {
Expand Down