From 4ab9ef57845a9407e3130549e60520aec657f4fa Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Fri, 28 Oct 2022 09:31:32 +0200 Subject: [PATCH] Implement `Logger.isLevelEnabled` (#144033) * Implement `Logger.isLevelEnabled` * fix manual logger * fix more manual logger * updating snapshots Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../src/logger.ts | 1 + .../src/logger.test.ts | 22 +++++++++++++++++++ .../src/logger.ts | 14 +++++++++++- .../src/logger_adapter.test.ts | 11 +++++++--- .../src/logger_adapter.ts | 6 ++++- packages/kbn-cli-dev-mode/src/log_adapter.ts | 1 + packages/kbn-logging-mocks/src/logger.mock.ts | 4 +++- packages/kbn-logging/src/logger.ts | 21 ++++++++++++++++-- src/cli_setup/utils.ts | 1 + src/plugins/expressions/public/plugin.ts | 1 + .../stack/email/send_email.test.ts | 1 + .../connector_types/stack/teams/index.test.ts | 2 ++ .../stack/webhook/index.test.ts | 2 ++ .../apis/metrics_ui/create_fake_logger.ts | 1 + .../tests/trial/lifecycle_executor.ts | 1 + 15 files changed, 81 insertions(+), 8 deletions(-) diff --git a/packages/core/analytics/core-analytics-browser-internal/src/logger.ts b/packages/core/analytics/core-analytics-browser-internal/src/logger.ts index 869fdbbd9fd85..ff403a7d46d2c 100644 --- a/packages/core/analytics/core-analytics-browser-internal/src/logger.ts +++ b/packages/core/analytics/core-analytics-browser-internal/src/logger.ts @@ -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, }; diff --git a/packages/core/logging/core-logging-server-internal/src/logger.test.ts b/packages/core/logging/core-logging-server-internal/src/logger.test.ts index c57ce2563ca3d..5c90118d7b1bb 100644 --- a/packages/core/logging/core-logging-server-internal/src/logger.test.ts +++ b/packages/core/logging/core-logging-server-internal/src/logger.test.ts @@ -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); + } + }); + } +}); diff --git a/packages/core/logging/core-logging-server-internal/src/logger.ts b/packages/core/logging/core-logging-server-internal/src/logger.ts index 2c9283da54897..7a18d9a74ebaa 100644 --- a/packages/core/logging/core-logging-server-internal/src/logger.ts +++ b/packages/core/logging/core-logging-server-internal/src/logger.ts @@ -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; @@ -45,6 +53,10 @@ export class BaseLogger implements Logger { this.log(this.createLogRecord(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; diff --git a/packages/core/logging/core-logging-server-internal/src/logger_adapter.test.ts b/packages/core/logging/core-logging-server-internal/src/logger_adapter.test.ts index 28f747ef3fcf6..5c6a64e7c10f4 100644 --- a/packages/core/logging/core-logging-server-internal/src/logger_adapter.test.ts +++ b/packages/core/logging/core-logging-server-internal/src/logger_adapter.test.ts @@ -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(); @@ -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.', () => { @@ -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'); diff --git a/packages/core/logging/core-logging-server-internal/src/logger_adapter.ts b/packages/core/logging/core-logging-server-internal/src/logger_adapter.ts index 5439fe0205796..13ce292936e6c 100644 --- a/packages/core/logging/core-logging-server-internal/src/logger_adapter.ts +++ b/packages/core/logging/core-logging-server-internal/src/logger_adapter.ts @@ -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 */ @@ -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); } diff --git a/packages/kbn-cli-dev-mode/src/log_adapter.ts b/packages/kbn-cli-dev-mode/src/log_adapter.ts index 65161fcc56e0e..58260939a6dae 100644 --- a/packages/kbn-cli-dev-mode/src/log_adapter.ts +++ b/packages/kbn-cli-dev-mode/src/log_adapter.ts @@ -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; diff --git a/packages/kbn-logging-mocks/src/logger.mock.ts b/packages/kbn-logging-mocks/src/logger.mock.ts index b5f1f409ee457..dd3303dda9410 100644 --- a/packages/kbn-logging-mocks/src/logger.mock.ts +++ b/packages/kbn-logging-mocks/src/logger.mock.ts @@ -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 & { context: string[] }; @@ -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; }; diff --git a/packages/kbn-logging/src/logger.ts b/packages/kbn-logging/src/logger.ts index fda3cf45b9d79..bd31d4c42f805 100644 --- a/packages/kbn-logging/src/logger.ts +++ b/packages/kbn-logging/src/logger.ts @@ -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 @@ -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. * diff --git a/src/cli_setup/utils.ts b/src/cli_setup/utils.ts index 47b8199f16ea0..33efb2ff802b8 100644 --- a/src/cli_setup/utils.ts +++ b/src/cli_setup/utils.ts @@ -29,6 +29,7 @@ const logger: Logger = { fatal: noop, log: noop, get: () => logger, + isLevelEnabled: () => true, }; export const kibanaConfigWriter = new KibanaConfigWriter(getConfigPath(), getDataPath(), logger); diff --git a/src/plugins/expressions/public/plugin.ts b/src/plugins/expressions/public/plugin.ts index d71a1457381ff..134b0a7c511ac 100644 --- a/src/plugins/expressions/public/plugin.ts +++ b/src/plugins/expressions/public/plugin.ts @@ -42,6 +42,7 @@ export class ExpressionsPublicPlugin implements Plugin true, }; private readonly expressions: ExpressionsService = new ExpressionsService({ diff --git a/x-pack/plugins/stack_connectors/server/connector_types/stack/email/send_email.test.ts b/x-pack/plugins/stack_connectors/server/connector_types/stack/email/send_email.test.ts index 9aa780788bc10..a439a7e16d6d9 100644 --- a/x-pack/plugins/stack_connectors/server/connector_types/stack/email/send_email.test.ts +++ b/x-pack/plugins/stack_connectors/server/connector_types/stack/email/send_email.test.ts @@ -178,6 +178,7 @@ describe('send_email module', () => { "fatal": [MockFunction], "get": [MockFunction], "info": [MockFunction], + "isLevelEnabled": [MockFunction], "log": [MockFunction], "trace": [MockFunction], "warn": [MockFunction], diff --git a/x-pack/plugins/stack_connectors/server/connector_types/stack/teams/index.test.ts b/x-pack/plugins/stack_connectors/server/connector_types/stack/teams/index.test.ts index 6d1cbb2bfbbbe..0b144bceb05c7 100644 --- a/x-pack/plugins/stack_connectors/server/connector_types/stack/teams/index.test.ts +++ b/x-pack/plugins/stack_connectors/server/connector_types/stack/teams/index.test.ts @@ -194,6 +194,7 @@ describe('execute()', () => { "fatal": [MockFunction], "get": [MockFunction], "info": [MockFunction], + "isLevelEnabled": [MockFunction], "log": [MockFunction], "trace": [MockFunction], "warn": [MockFunction], @@ -249,6 +250,7 @@ describe('execute()', () => { "fatal": [MockFunction], "get": [MockFunction], "info": [MockFunction], + "isLevelEnabled": [MockFunction], "log": [MockFunction], "trace": [MockFunction], "warn": [MockFunction], diff --git a/x-pack/plugins/stack_connectors/server/connector_types/stack/webhook/index.test.ts b/x-pack/plugins/stack_connectors/server/connector_types/stack/webhook/index.test.ts index 4e3369af35c56..222463711a2a5 100644 --- a/x-pack/plugins/stack_connectors/server/connector_types/stack/webhook/index.test.ts +++ b/x-pack/plugins/stack_connectors/server/connector_types/stack/webhook/index.test.ts @@ -303,6 +303,7 @@ describe('execute()', () => { "fatal": [MockFunction], "get": [MockFunction], "info": [MockFunction], + "isLevelEnabled": [MockFunction], "log": [MockFunction], "trace": [MockFunction], "warn": [MockFunction], @@ -389,6 +390,7 @@ describe('execute()', () => { "fatal": [MockFunction], "get": [MockFunction], "info": [MockFunction], + "isLevelEnabled": [MockFunction], "log": [MockFunction], "trace": [MockFunction], "warn": [MockFunction], diff --git a/x-pack/test/api_integration/apis/metrics_ui/create_fake_logger.ts b/x-pack/test/api_integration/apis/metrics_ui/create_fake_logger.ts index b99ee3de96a43..ae591c51e767b 100644 --- a/x-pack/test/api_integration/apis/metrics_ui/create_fake_logger.ts +++ b/x-pack/test/api_integration/apis/metrics_ui/create_fake_logger.ts @@ -22,5 +22,6 @@ export const createFakeLogger = (log: ToolingLog) => { fatal: fakeLogger, log: sinon.stub(), get: sinon.stub(), + isLevelEnabled: sinon.stub(), } as Logger; }; diff --git a/x-pack/test/rule_registry/spaces_only/tests/trial/lifecycle_executor.ts b/x-pack/test/rule_registry/spaces_only/tests/trial/lifecycle_executor.ts index dcad92db0d36a..66a6605d1d20c 100644 --- a/x-pack/test/rule_registry/spaces_only/tests/trial/lifecycle_executor.ts +++ b/x-pack/test/rule_registry/spaces_only/tests/trial/lifecycle_executor.ts @@ -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 = () => {