From 00f0b9fce4ff62edfa597d511de4810402db4f43 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Fri, 19 May 2023 10:13:27 -0400 Subject: [PATCH] fix: Remove erroneous prefixes when using loggerInjectable and prefixLoggerInjectable Signed-off-by: Sebastian Malton --- packages/logger/index.ts | 3 +- packages/logger/src/logger.injectable.ts | 48 ++++++------ packages/logger/src/logger.test.ts | 75 +++++++++++++++++-- packages/logger/src/logger.ts | 12 --- .../logger/src/prefixed-logger.injectable.ts | 3 +- packages/logger/src/transports.ts | 8 +- 6 files changed, 100 insertions(+), 49 deletions(-) delete mode 100644 packages/logger/src/logger.ts diff --git a/packages/logger/index.ts b/packages/logger/index.ts index 2120274f36..c0abf69e53 100644 --- a/packages/logger/index.ts +++ b/packages/logger/index.ts @@ -3,7 +3,7 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -export type { LogFunction } from "./src/logger.injectable"; +export type { LogFunction, Logger } from "./src/logger.injectable"; export { logDebugInjectionToken, logErrorInjectionToken, @@ -12,7 +12,6 @@ export { logWarningInjectionToken, } from "./src/logger.injectable"; -export type { Logger } from "./src/logger"; /** @deprecated Use specific injectionToken, eg. logErrorInjectionToken */ export { loggerInjectionToken } from "./src/logger.injectable"; /** @deprecated Use specific injectionToken, eg. logErrorInjectionToken */ diff --git a/packages/logger/src/logger.injectable.ts b/packages/logger/src/logger.injectable.ts index 896fb596d5..5a2b95e2a4 100644 --- a/packages/logger/src/logger.injectable.ts +++ b/packages/logger/src/logger.injectable.ts @@ -9,10 +9,17 @@ import { getInjectionToken, lifecycleEnum, } from "@ogre-tools/injectable"; -import type { Logger } from "./logger"; import { winstonLoggerInjectable } from "./winston-logger.injectable"; import { pipeline } from "@ogre-tools/fp"; +export interface Logger { + info: LogFunction; + error: LogFunction; + debug: LogFunction; + warn: LogFunction; + silly: LogFunction; +} + /** @deprecated Use specific injectionToken, eg. logErrorInjectionToken */ export const loggerInjectionToken = getInjectionToken({ id: "logger-injection-token", @@ -21,18 +28,18 @@ export const loggerInjectionToken = getInjectionToken({ export const loggerInjectable = getInjectable({ id: "logger", instantiate: (di): Logger => ({ - debug: di.inject(logDebugInjectionToken), - info: di.inject(logInfoInjectionToken), - warn: di.inject(logWarningInjectionToken), - error: di.inject(logErrorInjectionToken), - silly: di.inject(logSillyInjectionToken), + debug: getLogFunctionFor("debug", undefined)(di), + info: getLogFunctionFor("info", undefined)(di), + warn: getLogFunctionFor("warn", undefined)(di), + error: getLogFunctionFor("error", undefined)(di), + silly: getLogFunctionFor("silly", undefined)(di), }), decorable: false, injectionToken: loggerInjectionToken, }); -export type LogFunction = (message: string, ...data: any[]) => void; +export type LogFunction = (message: string, ...data: unknown[]) => void; export const logDebugInjectionToken = getInjectionToken({ id: "log-debug-injection-token", @@ -56,24 +63,23 @@ export const logSillyInjectionToken = getInjectionToken({ const screamingKebabCase = (str: string) => pipeline(str, kebabCase, toUpper); -const getLogFunctionFor = - (scenario: keyof Logger) => - (di: DiContainerForInjection): LogFunction => { +const getLogFunctionFor = (scenario: keyof Logger, namespace: string | undefined) => { + const prefix = namespace + ? `[${screamingKebabCase(namespace.replace(/-feature$/, ""))}]: ` + : ""; + + return (di: DiContainerForInjection): LogFunction => { const winstonLogger = di.inject(winstonLoggerInjectable); return (message, ...data) => { - winstonLogger[scenario]( - di.sourceNamespace - ? `[${screamingKebabCase(di.sourceNamespace)}]: ${message}` - : message, - ...data - ); + winstonLogger[scenario](`${prefix}${message}`, ...data); }; }; +}; export const logDebugInjectable = getInjectable({ id: "log-debug", - instantiate: getLogFunctionFor("debug"), + instantiate: (di) => getLogFunctionFor("debug", di.sourceNamespace)(di), injectionToken: logDebugInjectionToken, lifecycle: lifecycleEnum.keyedSingleton({ getInstanceKey: (di) => di.sourceNamespace, @@ -82,7 +88,7 @@ export const logDebugInjectable = getInjectable({ export const logInfoInjectable = getInjectable({ id: "log-info", - instantiate: getLogFunctionFor("info"), + instantiate: (di) => getLogFunctionFor("info", di.sourceNamespace)(di), injectionToken: logInfoInjectionToken, lifecycle: lifecycleEnum.keyedSingleton({ getInstanceKey: (di) => di.sourceNamespace, @@ -91,7 +97,7 @@ export const logInfoInjectable = getInjectable({ export const logWarningInjectable = getInjectable({ id: "log-warning", - instantiate: getLogFunctionFor("warn"), + instantiate: (di) => getLogFunctionFor("warn", di.sourceNamespace)(di), injectionToken: logWarningInjectionToken, lifecycle: lifecycleEnum.keyedSingleton({ getInstanceKey: (di) => di.sourceNamespace, @@ -100,7 +106,7 @@ export const logWarningInjectable = getInjectable({ export const logErrorInjectable = getInjectable({ id: "log-error", - instantiate: getLogFunctionFor("error"), + instantiate: (di) => getLogFunctionFor("error", di.sourceNamespace)(di), injectionToken: logErrorInjectionToken, lifecycle: lifecycleEnum.keyedSingleton({ getInstanceKey: (di) => di.sourceNamespace, @@ -109,7 +115,7 @@ export const logErrorInjectable = getInjectable({ export const logSillyInjectable = getInjectable({ id: "log-silly", - instantiate: getLogFunctionFor("silly"), + instantiate: (di) => getLogFunctionFor("silly", di.sourceNamespace)(di), injectionToken: logSillyInjectionToken, lifecycle: lifecycleEnum.keyedSingleton({ getInstanceKey: (di) => di.sourceNamespace, diff --git a/packages/logger/src/logger.test.ts b/packages/logger/src/logger.test.ts index e8a689ff71..1904cc0b05 100644 --- a/packages/logger/src/logger.test.ts +++ b/packages/logger/src/logger.test.ts @@ -2,24 +2,28 @@ import { createContainer, getInjectable } from "@ogre-tools/injectable"; import { registerFeature } from "@k8slens/feature-core"; import { loggerFeature } from "./feature"; import { winstonLoggerInjectable } from "./winston-logger.injectable"; +import TransportStream from "winston-transport"; import { logDebugInjectionToken, logErrorInjectionToken, + loggerInjectable, logInfoInjectionToken, logSillyInjectionToken, logWarningInjectionToken, } from "./logger.injectable"; import { getFeature } from "@k8slens/feature-core/src/feature"; +import { loggerTransportInjectionToken } from "./transports"; +import { prefixedLoggerInjectable } from "./prefixed-logger.injectable"; describe("logger", () => { [ - { scenario: "debug", injectionToken: logDebugInjectionToken }, - { scenario: "info", injectionToken: logInfoInjectionToken }, - { scenario: "warn", injectionToken: logWarningInjectionToken }, - { scenario: "error", injectionToken: logErrorInjectionToken }, - { scenario: "silly", injectionToken: logSillyInjectionToken }, + { scenario: "debug" as const, injectionToken: logDebugInjectionToken }, + { scenario: "info" as const, injectionToken: logInfoInjectionToken }, + { scenario: "warn" as const, injectionToken: logWarningInjectionToken }, + { scenario: "error" as const, injectionToken: logErrorInjectionToken }, + { scenario: "silly" as const, injectionToken: logSillyInjectionToken }, ].forEach(({ scenario, injectionToken }) => { it(`given not inside a Feature, when logging "${scenario}", does so without a prefix`, () => { const di = createContainer("irrelevant"); @@ -40,7 +44,45 @@ describe("logger", () => { ); }); - it(`given inside a Feature, when logging "${scenario}", does so with Feature's id as prefix`, () => { + it(`given not inside a Feature, when logging "${scenario}" using legacy logger, does so without a prefix`, () => { + const di = createContainer("irrelevant"); + + registerFeature(di, loggerFeature); + + const winstonLoggerStub = { [scenario]: jest.fn() } as any; + + di.override(winstonLoggerInjectable, () => winstonLoggerStub); + + const logger = di.inject(loggerInjectable); + + logger[scenario]("some-message", "some-data"); + + expect(winstonLoggerStub[scenario]).toHaveBeenCalledWith( + "some-message", + "some-data" + ); + }); + + it(`given not inside a Feature, when logging "${scenario}" using legacy prefixed logger, does so without a feature prefix`, () => { + const di = createContainer("irrelevant"); + + registerFeature(di, loggerFeature); + + const winstonLoggerStub = { [scenario]: jest.fn() } as any; + + di.override(winstonLoggerInjectable, () => winstonLoggerStub); + + const logger = di.inject(prefixedLoggerInjectable, "A-PREFIX"); + + logger[scenario]("some-message", "some-data"); + + expect(winstonLoggerStub[scenario]).toHaveBeenCalledWith( + "[A-PREFIX]: some-message", + "some-data" + ); + }); + + it(`given inside a Feature, when logging "${scenario}", does so with Feature's id as prefix without trailing '-feature' to avoid redundancy`, () => { const di = createContainer("irrelevant"); const logScenarioInFeature = getInjectable({ @@ -70,9 +112,28 @@ describe("logger", () => { logScenario("some-message", "some-data"); expect(winstonLoggerStub[scenario]).toHaveBeenCalledWith( - "[SOME-FEATURE]: some-message", + "[SOME]: some-message", "some-data" ); }); }); + + it("given a single transport, when logging, does not throw, and does call the transport", () => { + const di = createContainer("irrelevant"); + const log = jest.fn().mockImplementation((data, next) => next()); + + registerFeature(di, loggerFeature); + + di.register(getInjectable({ + id: "some-transport", + instantiate: () => new TransportStream({ log }), + injectionToken: loggerTransportInjectionToken, + })) + + const logger = di.inject(loggerInjectable); + + logger.info("some-message", "some-data"); + + expect(log).toHaveBeenCalled(); + }); }); diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts deleted file mode 100644 index 5de5f3b628..0000000000 --- a/packages/logger/src/logger.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * Copyright (c) OpenLens Authors. All rights reserved. - * Licensed under MIT License. See LICENSE in root directory for more information. - */ - -export interface Logger { - info: (message: string, ...args: any) => void; - error: (message: string, ...args: any) => void; - debug: (message: string, ...args: any) => void; - warn: (message: string, ...args: any) => void; - silly: (message: string, ...args: any) => void; -} diff --git a/packages/logger/src/prefixed-logger.injectable.ts b/packages/logger/src/prefixed-logger.injectable.ts index 61806d08c4..4b6d008a30 100644 --- a/packages/logger/src/prefixed-logger.injectable.ts +++ b/packages/logger/src/prefixed-logger.injectable.ts @@ -3,8 +3,7 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable"; -import type { Logger } from "./logger"; -import { loggerInjectionToken } from "./logger.injectable"; +import { Logger, loggerInjectionToken } from "./logger.injectable"; /** @deprecated Use specific injectionToken, eg. logErrorInjectionToken */ export const prefixedLoggerInjectable = getInjectable({ diff --git a/packages/logger/src/transports.ts b/packages/logger/src/transports.ts index 510dcbd7da..1407eb91b8 100644 --- a/packages/logger/src/transports.ts +++ b/packages/logger/src/transports.ts @@ -6,8 +6,6 @@ import { getInjectionToken } from "@ogre-tools/injectable"; import type TransportStream from "winston-transport"; -export const loggerTransportInjectionToken = getInjectionToken( - { - id: "logger-transport", - } -); +export const loggerTransportInjectionToken = getInjectionToken({ + id: "logger-transport", +});