From 48db54ec9e42b712d97a67b8a8b85f8096e1422d Mon Sep 17 00:00:00 2001 From: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> Date: Tue, 21 Mar 2023 21:04:22 +0200 Subject: [PATCH] Renderer file logging through IPC (#7300) * Renderer file logging through IPC Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Remove pagehide event listener as it may cause UI to freeze Pagehide was needed in cluster frame to better handle main frame close/reload situation. But even empty pagehide listener in cluster frame seems to freeze the UI at least on some situations (multiple clusters open). Beforeunload is not always executed in cluster frame when main frame is reloaded/closed, leaving log files open. To fix that, `stopIpcLoggingInjectable` is introduced to close all log files. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Remove unnecessary formatting changes Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Lint fix Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Winston logger override Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Remove usage of doGeneralOverrides as it has been removed Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Update imports to match the new base Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Remove unnecessary id Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Review improvements Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Extract beforeunload listener to injectable Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Typo fix Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> --------- Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> --- packages/core/src/common/logger.injectable.ts | 11 +- .../common/logger/ipc-file-logger-channel.ts | 24 +++ ...n-logger.global-override-for-injectable.ts | 23 +++ .../src/common/winston-logger.injectable.ts | 18 ++ .../close-ipc-logging-listener.injectable.ts | 21 +++ ...ransport.global-override-for-injectable.ts | 18 ++ .../create-ipc-file-transport.injectable.ts | 28 +++ .../main/logger/file-transport.injectable.ts | 6 +- .../main/logger/ipc-file-logger.injectable.ts | 55 ++++++ .../src/main/logger/ipc-file-logger.test.ts | 160 ++++++++++++++++++ .../logger/ipc-logging-listener.injectable.ts | 39 +++++ .../main/logger/ipc-logging-listener.test.ts | 31 ++++ .../logger/stop-ipc-logging.injectable.ts | 27 +++ .../runnables/listen-unload.injectable.ts | 46 +++++ packages/core/src/renderer/bootstrap.tsx | 4 +- .../init-cluster-frame/init-cluster-frame.ts | 95 +++++------ .../root-frame/init-root-frame.injectable.ts | 10 +- .../logger/close-renderer-log-file-id.test.ts | 56 ++++++ .../close-renderer-log-file.injectable.ts | 28 +++ .../logger/ipc-transport.injectable.ts | 60 +++++++ .../src/renderer/logger/ipc-transport.test.ts | 48 ++++++ .../core/src/renderer/logger/ipc-transport.ts | 41 +++++ .../logger/renderer-log-file-id.injectable.ts | 29 ++++ .../logger/renderer-log-file-id.test.ts | 34 ++++ 24 files changed, 836 insertions(+), 76 deletions(-) create mode 100644 packages/core/src/common/logger/ipc-file-logger-channel.ts create mode 100644 packages/core/src/common/winston-logger.global-override-for-injectable.ts create mode 100644 packages/core/src/common/winston-logger.injectable.ts create mode 100644 packages/core/src/main/logger/close-ipc-logging-listener.injectable.ts create mode 100644 packages/core/src/main/logger/create-ipc-file-transport.global-override-for-injectable.ts create mode 100644 packages/core/src/main/logger/create-ipc-file-transport.injectable.ts create mode 100644 packages/core/src/main/logger/ipc-file-logger.injectable.ts create mode 100644 packages/core/src/main/logger/ipc-file-logger.test.ts create mode 100644 packages/core/src/main/logger/ipc-logging-listener.injectable.ts create mode 100644 packages/core/src/main/logger/ipc-logging-listener.test.ts create mode 100644 packages/core/src/main/logger/stop-ipc-logging.injectable.ts create mode 100644 packages/core/src/renderer/before-frame-starts/runnables/listen-unload.injectable.ts create mode 100644 packages/core/src/renderer/logger/close-renderer-log-file-id.test.ts create mode 100644 packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts create mode 100644 packages/core/src/renderer/logger/ipc-transport.injectable.ts create mode 100644 packages/core/src/renderer/logger/ipc-transport.test.ts create mode 100644 packages/core/src/renderer/logger/ipc-transport.ts create mode 100644 packages/core/src/renderer/logger/renderer-log-file-id.injectable.ts create mode 100644 packages/core/src/renderer/logger/renderer-log-file-id.test.ts diff --git a/packages/core/src/common/logger.injectable.ts b/packages/core/src/common/logger.injectable.ts index bc1c5de71b..e64978e44b 100644 --- a/packages/core/src/common/logger.injectable.ts +++ b/packages/core/src/common/logger.injectable.ts @@ -3,20 +3,13 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; -import { createLogger, format } from "winston"; import type { Logger } from "./logger"; -import { loggerTransportInjectionToken } from "./logger/transports"; +import winstonLoggerInjectable from "./winston-logger.injectable"; const loggerInjectable = getInjectable({ id: "logger", instantiate: (di): Logger => { - const baseLogger = createLogger({ - format: format.combine( - format.splat(), - format.simple(), - ), - transports: di.injectMany(loggerTransportInjectionToken), - }); + const baseLogger = di.inject(winstonLoggerInjectable); return { debug: (message, ...data) => baseLogger.debug(message, ...data), diff --git a/packages/core/src/common/logger/ipc-file-logger-channel.ts b/packages/core/src/common/logger/ipc-file-logger-channel.ts new file mode 100644 index 0000000000..7550f4f314 --- /dev/null +++ b/packages/core/src/common/logger/ipc-file-logger-channel.ts @@ -0,0 +1,24 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { MessageChannel } from "../utils/channel/message-channel-listener-injection-token"; + +export interface IpcFileLogObject { + fileId: string; + entry: { + level: string; + message: string; + internalMessage: string; + }; +} + +export type IpcFileLoggerChannel = MessageChannel; + +export const ipcFileLoggerChannel: IpcFileLoggerChannel = { + id: "ipc-file-logger-channel", +}; + +export const closeIpcFileLoggerChannel: MessageChannel = { + id: "close-ipc-file-logger-channel", +}; diff --git a/packages/core/src/common/winston-logger.global-override-for-injectable.ts b/packages/core/src/common/winston-logger.global-override-for-injectable.ts new file mode 100644 index 0000000000..3d55f914dd --- /dev/null +++ b/packages/core/src/common/winston-logger.global-override-for-injectable.ts @@ -0,0 +1,23 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import type winston from "winston"; +import { getGlobalOverride } from "@k8slens/test-utils"; +import { noop } from "@k8slens/utilities"; +import winstonLoggerInjectable from "./winston-logger.injectable"; + +export default getGlobalOverride(winstonLoggerInjectable, () => ({ + log: noop, + add: noop, + remove: noop, + clear: noop, + close: noop, + + warn: noop, + debug: noop, + error: noop, + info: noop, + silly: noop, +}) as winston.Logger); diff --git a/packages/core/src/common/winston-logger.injectable.ts b/packages/core/src/common/winston-logger.injectable.ts new file mode 100644 index 0000000000..481d520fac --- /dev/null +++ b/packages/core/src/common/winston-logger.injectable.ts @@ -0,0 +1,18 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { createLogger, format } from "winston"; +import { loggerTransportInjectionToken } from "./logger/transports"; + +const winstonLoggerInjectable = getInjectable({ + id: "winston-logger", + instantiate: (di) => + createLogger({ + format: format.combine(format.splat(), format.simple()), + transports: di.injectMany(loggerTransportInjectionToken), + }), +}); + +export default winstonLoggerInjectable; diff --git a/packages/core/src/main/logger/close-ipc-logging-listener.injectable.ts b/packages/core/src/main/logger/close-ipc-logging-listener.injectable.ts new file mode 100644 index 0000000000..6870a29c61 --- /dev/null +++ b/packages/core/src/main/logger/close-ipc-logging-listener.injectable.ts @@ -0,0 +1,21 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import ipcFileLoggerInjectable from "./ipc-file-logger.injectable"; +import { getMessageChannelListenerInjectable } from "../../common/utils/channel/message-channel-listener-injection-token"; +import { + closeIpcFileLoggerChannel, +} from "../../common/logger/ipc-file-logger-channel"; + +const closeIpcFileLoggingListenerInjectable = getMessageChannelListenerInjectable({ + id: "close-ipc-file-logging", + channel: closeIpcFileLoggerChannel, + handler: (di) => { + const ipcFileLogger = di.inject(ipcFileLoggerInjectable); + + return (fileId) => ipcFileLogger.close(fileId); + }, +}); + +export default closeIpcFileLoggingListenerInjectable; diff --git a/packages/core/src/main/logger/create-ipc-file-transport.global-override-for-injectable.ts b/packages/core/src/main/logger/create-ipc-file-transport.global-override-for-injectable.ts new file mode 100644 index 0000000000..98fc62da49 --- /dev/null +++ b/packages/core/src/main/logger/create-ipc-file-transport.global-override-for-injectable.ts @@ -0,0 +1,18 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import type { transports } from "winston"; +import { getGlobalOverride } from "@k8slens/test-utils"; +import { noop } from "@k8slens/utilities"; +import createIpcFileLoggerTransportInjectable from "./create-ipc-file-transport.injectable"; + +export default getGlobalOverride( + createIpcFileLoggerTransportInjectable, + () => () => + ({ + log: noop, + close: noop, + } as typeof transports.File), +); diff --git a/packages/core/src/main/logger/create-ipc-file-transport.injectable.ts b/packages/core/src/main/logger/create-ipc-file-transport.injectable.ts new file mode 100644 index 0000000000..f29e02fc90 --- /dev/null +++ b/packages/core/src/main/logger/create-ipc-file-transport.injectable.ts @@ -0,0 +1,28 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { transports } from "winston"; +import directoryForLogsInjectable from "../../common/app-paths/directory-for-logs.injectable"; + +const createIpcFileLoggerTransportInjectable = getInjectable({ + id: "create-ipc-file-logger-transport", + instantiate: (di) => { + const options = { + dirname: di.inject(directoryForLogsInjectable), + maxsize: 1024 * 1024, + maxFiles: 2, + tailable: true, + }; + + return (fileId: string) => + new transports.File({ + ...options, + filename: `lens-${fileId}.log`, + }); + }, + causesSideEffects: true, +}); + +export default createIpcFileLoggerTransportInjectable; diff --git a/packages/core/src/main/logger/file-transport.injectable.ts b/packages/core/src/main/logger/file-transport.injectable.ts index fcf855eec4..c71b44a2a0 100644 --- a/packages/core/src/main/logger/file-transport.injectable.ts +++ b/packages/core/src/main/logger/file-transport.injectable.ts @@ -7,8 +7,8 @@ import { transports } from "winston"; import directoryForLogsInjectable from "../../common/app-paths/directory-for-logs.injectable"; import { loggerTransportInjectionToken } from "../../common/logger/transports"; -const fileLoggerTranportInjectable = getInjectable({ - id: "file-logger-tranport", +const fileLoggerTransportInjectable = getInjectable({ + id: "file-logger-transport", instantiate: (di) => new transports.File({ handleExceptions: false, level: "debug", @@ -26,4 +26,4 @@ const fileLoggerTranportInjectable = getInjectable({ decorable: false, }); -export default fileLoggerTranportInjectable; +export default fileLoggerTransportInjectable; diff --git a/packages/core/src/main/logger/ipc-file-logger.injectable.ts b/packages/core/src/main/logger/ipc-file-logger.injectable.ts new file mode 100644 index 0000000000..df82ef7c6b --- /dev/null +++ b/packages/core/src/main/logger/ipc-file-logger.injectable.ts @@ -0,0 +1,55 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { getOrInsertWith } from "@k8slens/utilities"; +import type { LogEntry, transports } from "winston"; +import createIpcFileLoggerTransportInjectable from "./create-ipc-file-transport.injectable"; + +export interface IpcFileLogger { + log: (fileLog: { fileId: string; entry: LogEntry }) => void; + close: (fileId: string) => void; + closeAll: () => void; +} + +const ipcFileLoggerInjectable = getInjectable({ + id: "ipc-file-logger", + instantiate: (di): IpcFileLogger => { + const createIpcFileTransport = di.inject(createIpcFileLoggerTransportInjectable); + const fileTransports = new Map(); + + function log({ fileId, entry }: { fileId: string; entry: LogEntry }) { + const transport = getOrInsertWith( + fileTransports, + fileId, + () => createIpcFileTransport(fileId), + ); + + transport?.log?.(entry, () => {}); + } + + function close(fileId: string) { + const transport = fileTransports.get(fileId); + + if (transport) { + transport.close?.(); + fileTransports.delete(fileId); + } + } + + function closeAll() { + for (const fileId of fileTransports.keys()) { + close(fileId); + } + } + + return { + log, + close, + closeAll, + }; + }, +}); + +export default ipcFileLoggerInjectable; diff --git a/packages/core/src/main/logger/ipc-file-logger.test.ts b/packages/core/src/main/logger/ipc-file-logger.test.ts new file mode 100644 index 0000000000..1ff727e200 --- /dev/null +++ b/packages/core/src/main/logger/ipc-file-logger.test.ts @@ -0,0 +1,160 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getDiForUnitTesting } from "../getDiForUnitTesting"; +import createIpcFileLoggerTransportInjectable from "./create-ipc-file-transport.injectable"; +import type { IpcFileLogger } from "./ipc-file-logger.injectable"; +import ipcFileLoggerInjectable from "./ipc-file-logger.injectable"; + +describe("ipc file logger in main", () => { + let logMock: jest.Mock; + let closeMock: jest.Mock; + let createFileTransportMock: jest.Mock; + let logger: IpcFileLogger; + + beforeEach(() => { + logMock = jest.fn(); + closeMock = jest.fn(); + createFileTransportMock = jest.fn(() => ({ + log: logMock, + close: closeMock, + })); + + const di = getDiForUnitTesting(); + + di.override(createIpcFileLoggerTransportInjectable, () => createFileTransportMock); + logger = di.inject(ipcFileLoggerInjectable); + }); + + it("creates a transport for new log file", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + expect(createFileTransportMock).toHaveBeenCalledWith("some-log-file"); + }); + + it("uses existing transport for log file", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + expect(createFileTransportMock).toHaveBeenCalledTimes(1); + + expect(createFileTransportMock).toHaveBeenCalledWith("some-log-file"); + }); + + it("creates separate transport for each log file", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.log({ + fileId: "some-other-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.log({ + fileId: "some-yet-another-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + expect(createFileTransportMock).toHaveBeenCalledTimes(3); + + expect(createFileTransportMock).toHaveBeenCalledWith("some-log-file"); + + expect(createFileTransportMock).toHaveBeenCalledWith("some-other-log-file"); + + expect(createFileTransportMock).toHaveBeenCalledWith("some-yet-another-log-file"); + }); + + it("logs using file transport", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "some-log-message" }, + }); + expect(logMock.mock.calls[0][0]).toEqual({ + level: "irrelevant", + message: "some-log-message", + }); + }); + + it("logs to correct files", () => { + const someLogMock = jest.fn(); + const someOthertLogMock = jest.fn(); + + createFileTransportMock.mockImplementation((fileId: string) => { + if (fileId === "some-log-file") { + return { log: someLogMock }; + } + + if (fileId === "some-other-log-file") { + return { log: someOthertLogMock }; + } + + return null; + }); + + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "some-log-message" }, + }); + logger.log({ + fileId: "some-other-log-file", + entry: { level: "irrelevant", message: "some-other-log-message" }, + }); + + expect(someLogMock).toHaveBeenCalledTimes(1); + expect(someLogMock.mock.calls[0][0]).toEqual({ + level: "irrelevant", + message: "some-log-message", + }); + expect(someOthertLogMock).toHaveBeenCalledTimes(1); + expect(someOthertLogMock.mock.calls[0][0]).toEqual({ + level: "irrelevant", + message: "some-other-log-message", + }); + }); + + it("closes transport (to ensure no file handles are left open)", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.close("some-log-file"); + + expect(closeMock).toHaveBeenCalled(); + }); + + it("creates a new transport once needed after closing previous", () => { + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + logger.close("some-log-file"); + + logger.log({ + fileId: "some-log-file", + entry: { level: "irrelevant", message: "irrelevant" }, + }); + + expect(createFileTransportMock).toHaveBeenCalledTimes(2); + expect(logMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/core/src/main/logger/ipc-logging-listener.injectable.ts b/packages/core/src/main/logger/ipc-logging-listener.injectable.ts new file mode 100644 index 0000000000..3a3748846b --- /dev/null +++ b/packages/core/src/main/logger/ipc-logging-listener.injectable.ts @@ -0,0 +1,39 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import ipcFileLoggerInjectable from "./ipc-file-logger.injectable"; +import { getMessageChannelListenerInjectable } from "../../common/utils/channel/message-channel-listener-injection-token"; +import type { IpcFileLogObject } from "../../common/logger/ipc-file-logger-channel"; +import { ipcFileLoggerChannel } from "../../common/logger/ipc-file-logger-channel"; +import { MESSAGE } from "triple-beam"; + +/** + * Winston uses symbol property for the actual message. + * + * For that to get through IPC, use the internalMessage property instead + */ +export function deserializeLogFromIpc(ipcFileLogObject: IpcFileLogObject) { + const { internalMessage, ...standardEntry } = ipcFileLogObject.entry; + + return { + ...ipcFileLogObject, + entry: { + ...standardEntry, + [MESSAGE]: internalMessage, + }, + }; +} + +const ipcFileLoggingListenerInjectable = getMessageChannelListenerInjectable({ + id: "ipc-file-logging", + channel: ipcFileLoggerChannel, + handler: (di) => { + const logger = di.inject(ipcFileLoggerInjectable); + + return (ipcFileLogObject) => + logger.log(deserializeLogFromIpc(ipcFileLogObject)); + }, +}); + +export default ipcFileLoggingListenerInjectable; diff --git a/packages/core/src/main/logger/ipc-logging-listener.test.ts b/packages/core/src/main/logger/ipc-logging-listener.test.ts new file mode 100644 index 0000000000..55bb64f4c4 --- /dev/null +++ b/packages/core/src/main/logger/ipc-logging-listener.test.ts @@ -0,0 +1,31 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { MESSAGE } from "triple-beam"; +import { deserializeLogFromIpc } from "./ipc-logging-listener.injectable"; + +describe("Ipc log deserialization", () => { + it("fills in the unique symbol message property Winston transports use internally", () => { + const logObject = { + fileId: "irrelevant", + entry: { + level: "irrelevant", + message: "some public message", + internalMessage: "some internal message", + someProperty: "irrelevant", + }, + }; + + expect(deserializeLogFromIpc(logObject)).toEqual({ + entry: { + level: "irrelevant", + message: "some public message", + [MESSAGE]: "some internal message", + someProperty: "irrelevant", + }, + fileId: "irrelevant", + }); + }); +}); diff --git a/packages/core/src/main/logger/stop-ipc-logging.injectable.ts b/packages/core/src/main/logger/stop-ipc-logging.injectable.ts new file mode 100644 index 0000000000..bdb94a412e --- /dev/null +++ b/packages/core/src/main/logger/stop-ipc-logging.injectable.ts @@ -0,0 +1,27 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { beforeQuitOfFrontEndInjectionToken } from "../start-main-application/runnable-tokens/phases"; +import ipcFileLoggerInjectable from "./ipc-file-logger.injectable"; + +const stopIpcLoggingInjectable = getInjectable({ + id: "stop-ipc-logging", + + instantiate: (di) => { + const ipcFileLogger = di.inject(ipcFileLoggerInjectable); + + return { + run: () => { + ipcFileLogger.closeAll(); + + return undefined; + }, + }; + }, + + injectionToken: beforeQuitOfFrontEndInjectionToken, +}); + +export default stopIpcLoggingInjectable; diff --git a/packages/core/src/renderer/before-frame-starts/runnables/listen-unload.injectable.ts b/packages/core/src/renderer/before-frame-starts/runnables/listen-unload.injectable.ts new file mode 100644 index 0000000000..6b6e0e751c --- /dev/null +++ b/packages/core/src/renderer/before-frame-starts/runnables/listen-unload.injectable.ts @@ -0,0 +1,46 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import currentlyInClusterFrameInjectable from "../../routes/currently-in-cluster-frame.injectable"; +import { beforeFrameStartsSecondInjectionToken } from "../tokens"; +import loggerInjectable from "../../../common/logger.injectable"; +import hostedClusterInjectable from "../../cluster-frame-context/hosted-cluster.injectable"; +import frameRoutingIdInjectable from "../../frames/cluster-frame/init-cluster-frame/frame-routing-id/frame-routing-id.injectable"; +import closeRendererLogFileInjectable from "../../logger/close-renderer-log-file.injectable"; +import { unmountComponentAtNode } from "react-dom"; + +const listenUnloadInjectable = getInjectable({ + id: "listen-unload", + instantiate: (di) => ({ + run: () => { + const closeRendererLogFile = di.inject(closeRendererLogFileInjectable); + const isClusterFrame = di.inject(currentlyInClusterFrameInjectable); + const logger = di.inject(loggerInjectable); + + window.addEventListener("beforeunload", () => { + if (isClusterFrame) { + const hostedCluster = di.inject(hostedClusterInjectable); + const frameRoutingId = di.inject(frameRoutingIdInjectable); + + logger.info( + `[CLUSTER-FRAME] Unload dashboard, clusterId=${hostedCluster?.id}, frameId=${frameRoutingId}`, + ); + } else { + logger.info("[ROOT-FRAME]: Unload app"); + } + + closeRendererLogFile(); + const rootElem = document.getElementById("app"); + + if (rootElem) { + unmountComponentAtNode(rootElem); + } + }); + }, + }), + injectionToken: beforeFrameStartsSecondInjectionToken, +}); + +export default listenUnloadInjectable; diff --git a/packages/core/src/renderer/bootstrap.tsx b/packages/core/src/renderer/bootstrap.tsx index a811f07d5c..75439fce13 100644 --- a/packages/core/src/renderer/bootstrap.tsx +++ b/packages/core/src/renderer/bootstrap.tsx @@ -6,7 +6,7 @@ import "./components/app.scss"; import React from "react"; -import { render, unmountComponentAtNode } from "react-dom"; +import { render } from "react-dom"; import { DefaultProps } from "./mui-base-theme"; import { DiContextProvider } from "@ogre-tools/injectable-react"; import type { @@ -43,7 +43,7 @@ export async function bootstrap(di: DiContainerForInjection) { } try { - await initializeApp(() => unmountComponentAtNode(rootElem)); + await initializeApp(); } catch (error) { console.error(`[BOOTSTRAP]: view initialization error: ${error}`, { origin: location.href, diff --git a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts index 9bd0a26a3c..9e901a8060 100644 --- a/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts +++ b/packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts @@ -22,62 +22,51 @@ interface Dependencies { const logPrefix = "[CLUSTER-FRAME]:"; -export const initClusterFrame = ({ - hostedCluster, - loadExtensions, - catalogEntityRegistry, - frameRoutingId, - emitAppEvent, - logger, - showErrorNotification, -}: Dependencies) => - async (unmountRoot: () => void) => { +export const initClusterFrame = + ({ + hostedCluster, + loadExtensions, + catalogEntityRegistry, + frameRoutingId, + emitAppEvent, + logger, + showErrorNotification, + }: Dependencies) => + async () => { // TODO: Make catalogEntityRegistry already initialized when passed as dependency - catalogEntityRegistry.init(); + catalogEntityRegistry.init(); - logger.info( - `${logPrefix} Init dashboard, clusterId=${hostedCluster.id}, frameId=${frameRoutingId}`, - ); - - await requestSetClusterFrameId(hostedCluster.id); - await when(() => hostedCluster.ready.get()); // cluster.activate() is done at this point - - catalogEntityRegistry.activeEntity = hostedCluster.id; - - // Only load the extensions once the catalog has been populated. - // Note that the Catalog might still have unprocessed entities until the extensions are fully loaded. - when( - () => catalogEntityRegistry.items.get().length > 0, - () => - loadExtensions(), - { - timeout: 15_000, - onError: (error) => { - logger.warn( - "[CLUSTER-FRAME]: error from activeEntity when()", - error, - ); - - showErrorNotification("Failed to get KubernetesCluster for this view. Extensions will not be loaded."); - }, - }, - ); - - setTimeout(() => { - emitAppEvent({ - name: "cluster", - action: "open", - params: { - clusterId: hostedCluster.id, - }, - }); - }); - - window.onbeforeunload = () => { logger.info( - `${logPrefix} Unload dashboard, clusterId=${(hostedCluster.id)}, frameId=${frameRoutingId}`, + `${logPrefix} Init dashboard, clusterId=${hostedCluster.id}, frameId=${frameRoutingId}`, ); - unmountRoot(); + await requestSetClusterFrameId(hostedCluster.id); + await when(() => hostedCluster.ready.get()); // cluster.activate() is done at this point + + catalogEntityRegistry.activeEntity = hostedCluster.id; + + // Only load the extensions once the catalog has been populated. + // Note that the Catalog might still have unprocessed entities until the extensions are fully loaded. + when( + () => catalogEntityRegistry.items.get().length > 0, + () => loadExtensions(), + { + timeout: 15_000, + onError: (error) => { + logger.warn("[CLUSTER-FRAME]: error from activeEntity when()", error); + + showErrorNotification("Failed to get KubernetesCluster for this view. Extensions will not be loaded."); + }, + }, + ); + + setTimeout(() => { + emitAppEvent({ + name: "cluster", + action: "open", + params: { + clusterId: hostedCluster.id, + }, + }); + }); }; - }; diff --git a/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts b/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts index f1e3024d80..e1bedb0c88 100644 --- a/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts +++ b/packages/core/src/renderer/frames/root-frame/init-root-frame.injectable.ts @@ -9,7 +9,6 @@ import lensProtocolRouterRendererInjectable from "../../protocol-handler/lens-pr import catalogEntityRegistryInjectable from "../../api/catalog/entity/registry.injectable"; import registerIpcListenersInjectable from "../../ipc/register-ipc-listeners.injectable"; import loadExtensionsInjectable from "../load-extensions.injectable"; -import loggerInjectable from "../../../common/logger.injectable"; import { delay } from "@k8slens/utilities"; import { broadcastMessage } from "../../../common/ipc"; import { bundledExtensionsLoaded } from "../../../common/ipc/extension-handling"; @@ -23,9 +22,8 @@ const initRootFrameInjectable = getInjectable({ const bindProtocolAddRouteHandlers = di.inject(bindProtocolAddRouteHandlersInjectable); const lensProtocolRouterRenderer = di.inject(lensProtocolRouterRendererInjectable); const catalogEntityRegistry = di.inject(catalogEntityRegistryInjectable); - const logger = di.inject(loggerInjectable); - return async (unmountRoot: () => void) => { + return async () => { catalogEntityRegistry.init(); try { @@ -56,12 +54,6 @@ const initRootFrameInjectable = getInjectable({ window.addEventListener("online", () => broadcastMessage("network:online")); registerIpcListeners(); - - window.addEventListener("beforeunload", () => { - logger.info("[ROOT-FRAME]: Unload app"); - - unmountRoot(); - }); }; }, }); diff --git a/packages/core/src/renderer/logger/close-renderer-log-file-id.test.ts b/packages/core/src/renderer/logger/close-renderer-log-file-id.test.ts new file mode 100644 index 0000000000..1520844c30 --- /dev/null +++ b/packages/core/src/renderer/logger/close-renderer-log-file-id.test.ts @@ -0,0 +1,56 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import winstonLoggerInjectable from "../../common/winston-logger.injectable"; +import { getDiForUnitTesting } from "../getDiForUnitTesting"; +import closeRendererLogFileInjectable from "./close-renderer-log-file.injectable"; +import type { DiContainer } from "@ogre-tools/injectable"; +import type winston from "winston"; +import type { SendMessageToChannel } from "../../common/utils/channel/message-to-channel-injection-token"; +import { sendMessageToChannelInjectionToken } from "../../common/utils/channel/message-to-channel-injection-token"; +import rendererLogFileIdInjectable from "./renderer-log-file-id.injectable"; +import ipcLogTransportInjectable from "./ipc-transport.injectable"; +import type IpcLogTransport from "./ipc-transport"; + +describe("close renderer file logging", () => { + let di: DiContainer; + let sendIpcMock: SendMessageToChannel; + let winstonMock: winston.Logger; + let ipcTransportMock: IpcLogTransport; + + beforeEach(() => { + di = getDiForUnitTesting(); + sendIpcMock = jest.fn(); + winstonMock = { + remove: jest.fn(), + } as any as winston.Logger; + ipcTransportMock = { name: "ipc-renderer-transport" } as IpcLogTransport; + + di.override(winstonLoggerInjectable, () => winstonMock); + di.override(sendMessageToChannelInjectionToken, () => sendIpcMock); + di.override(rendererLogFileIdInjectable, () => "some-log-id"); + di.override(ipcLogTransportInjectable, () => ipcTransportMock); + }); + + it("sends the ipc close message with correct log id", () => { + const closeLog = di.inject(closeRendererLogFileInjectable); + + closeLog(); + + expect(sendIpcMock).toHaveBeenCalledWith( + { id: "close-ipc-file-logger-channel" }, + "some-log-id", + ); + }); + + it("removes the transport to prevent further logging to closed file", () => { + const closeLog = di.inject(closeRendererLogFileInjectable); + + closeLog(); + + expect(winstonMock.remove).toHaveBeenCalledWith({ + name: "ipc-renderer-transport", + }); + }); +}); diff --git a/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts b/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts new file mode 100644 index 0000000000..8015708d84 --- /dev/null +++ b/packages/core/src/renderer/logger/close-renderer-log-file.injectable.ts @@ -0,0 +1,28 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import winstonLoggerInjectable from "../../common/winston-logger.injectable"; +import { closeIpcFileLoggerChannel } from "../../common/logger/ipc-file-logger-channel"; +import { sendMessageToChannelInjectionToken } from "../../common/utils/channel/message-to-channel-injection-token"; +import rendererLogFileIdInjectable from "./renderer-log-file-id.injectable"; +import ipcLogTransportInjectable from "./ipc-transport.injectable"; + +const closeRendererLogFileInjectable = getInjectable({ + id: "close-renderer-log-file", + instantiate: (di) => { + const winstonLogger = di.inject(winstonLoggerInjectable); + const ipcLogTransport = di.inject(ipcLogTransportInjectable); + const messageToChannel = di.inject(sendMessageToChannelInjectionToken); + const fileId = di.inject(rendererLogFileIdInjectable); + + + return () => { + messageToChannel(closeIpcFileLoggerChannel, fileId); + winstonLogger.remove(ipcLogTransport); + }; + }, +}); + +export default closeRendererLogFileInjectable; diff --git a/packages/core/src/renderer/logger/ipc-transport.injectable.ts b/packages/core/src/renderer/logger/ipc-transport.injectable.ts new file mode 100644 index 0000000000..45139cd917 --- /dev/null +++ b/packages/core/src/renderer/logger/ipc-transport.injectable.ts @@ -0,0 +1,60 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import { loggerTransportInjectionToken } from "../../common/logger/transports"; +import type winston from "winston"; +import { MESSAGE } from "triple-beam"; + +import IpcLogTransport from "./ipc-transport"; +import { sendMessageToChannelInjectionToken } from "../../common/utils/channel/message-to-channel-injection-token"; +import type { + IpcFileLogObject } from "../../common/logger/ipc-file-logger-channel"; +import { + closeIpcFileLoggerChannel, + ipcFileLoggerChannel, +} from "../../common/logger/ipc-file-logger-channel"; +import rendererLogFileIdInjectable from "./renderer-log-file-id.injectable"; + +/** + * Winston uses symbol property for the actual message. + * + * For that to get through IPC, use the internalMessage property instead + */ +function serializeLogForIpc( + fileId: string, + entry: winston.LogEntry, +): IpcFileLogObject { + return { + fileId, + entry: { + level: entry.level, + message: entry.message, + internalMessage: Object.getOwnPropertyDescriptor(entry, MESSAGE)?.value, + }, + }; +} + +const ipcLogTransportInjectable = getInjectable({ + id: "renderer-file-logger-transport", + instantiate: (di) => { + const messageToChannel = di.inject(sendMessageToChannelInjectionToken); + const fileId = di.inject(rendererLogFileIdInjectable); + + return new IpcLogTransport({ + sendIpcLogMessage: (entry) => + messageToChannel( + ipcFileLoggerChannel, + serializeLogForIpc(fileId, entry), + ), + closeIpcLogging: () => + messageToChannel(closeIpcFileLoggerChannel, fileId), + handleExceptions: false, + level: "info", + }); + }, + injectionToken: loggerTransportInjectionToken, +}); + +export default ipcLogTransportInjectable; diff --git a/packages/core/src/renderer/logger/ipc-transport.test.ts b/packages/core/src/renderer/logger/ipc-transport.test.ts new file mode 100644 index 0000000000..931df377c7 --- /dev/null +++ b/packages/core/src/renderer/logger/ipc-transport.test.ts @@ -0,0 +1,48 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { DiContainer } from "@ogre-tools/injectable"; +import type { SendMessageToChannel } from "../../common/utils/channel/message-to-channel-injection-token"; +import { sendMessageToChannelInjectionToken } from "../../common/utils/channel/message-to-channel-injection-token"; +import { getDiForUnitTesting } from "../getDiForUnitTesting"; +import rendererLogFileIdInjectable from "./renderer-log-file-id.injectable"; +import ipcLogTransportInjectable from "./ipc-transport.injectable"; +import { MESSAGE } from "triple-beam"; + +describe("renderer log transport through ipc", () => { + let di: DiContainer; + let sendIpcMock: SendMessageToChannel; + + beforeEach(() => { + sendIpcMock = jest.fn(); + di = getDiForUnitTesting(); + di.override(sendMessageToChannelInjectionToken, () => sendIpcMock); + di.override(rendererLogFileIdInjectable, () => "some-log-id"); + }); + + it("send serialized ipc messages on log", () => { + const logTransport = di.inject(ipcLogTransportInjectable); + + logTransport.log( + { + level: "info", + message: "some log text", + [MESSAGE]: "actual winston log text", + }, + () => {}, + ); + + expect(sendIpcMock).toHaveBeenCalledWith( + { id: "ipc-file-logger-channel" }, + { + entry: { + level: "info", + message: "some log text", + internalMessage: "actual winston log text", + }, + fileId: "some-log-id", + }, + ); + }); +}); diff --git a/packages/core/src/renderer/logger/ipc-transport.ts b/packages/core/src/renderer/logger/ipc-transport.ts new file mode 100644 index 0000000000..a1f6fa819e --- /dev/null +++ b/packages/core/src/renderer/logger/ipc-transport.ts @@ -0,0 +1,41 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import type { LogEntry } from "winston"; +import type { TransportStreamOptions } from "winston-transport"; +import TransportStream from "winston-transport"; + +interface IpcLogTransportOptions extends TransportStreamOptions { + sendIpcLogMessage: (entry: LogEntry) => void; + closeIpcLogging: () => void; +} + +class IpcLogTransport extends TransportStream { + sendIpcLogMessage: (entry: LogEntry) => void; + closeIpcLogging: () => void; + name = "ipc-renderer-transport"; + + constructor(options: IpcLogTransportOptions) { + const { sendIpcLogMessage, closeIpcLogging, ...winstonOptions } = options; + + super(winstonOptions); + + this.sendIpcLogMessage = sendIpcLogMessage; + this.closeIpcLogging = closeIpcLogging; + } + + log(logEntry: LogEntry, next: () => void) { + setImmediate(() => { + this.emit("logged", logEntry); + }); + this.sendIpcLogMessage(logEntry); + next(); + } + + close() { + this.closeIpcLogging(); + } +} + +export default IpcLogTransport; diff --git a/packages/core/src/renderer/logger/renderer-log-file-id.injectable.ts b/packages/core/src/renderer/logger/renderer-log-file-id.injectable.ts new file mode 100644 index 0000000000..81f4196b08 --- /dev/null +++ b/packages/core/src/renderer/logger/renderer-log-file-id.injectable.ts @@ -0,0 +1,29 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import windowLocationInjectable from "../../common/k8s-api/window-location.injectable"; +import currentlyInClusterFrameInjectable from "../routes/currently-in-cluster-frame.injectable"; +import { getClusterIdFromHost } from "../../common/utils"; + +const rendererLogFileIdInjectable = getInjectable({ + id: "renderer-log-file-id", + instantiate: (di) => { + let frameId: string; + const currentlyInClusterFrame = di.inject(currentlyInClusterFrameInjectable); + + if (currentlyInClusterFrame) { + const { host } = di.inject(windowLocationInjectable); + const clusterId = getClusterIdFromHost(host); + + frameId = `cluster-${clusterId}`; + } else { + frameId = "main"; + } + + return `renderer-${frameId}`; + }, +}); + +export default rendererLogFileIdInjectable; diff --git a/packages/core/src/renderer/logger/renderer-log-file-id.test.ts b/packages/core/src/renderer/logger/renderer-log-file-id.test.ts new file mode 100644 index 0000000000..bd2abf7a63 --- /dev/null +++ b/packages/core/src/renderer/logger/renderer-log-file-id.test.ts @@ -0,0 +1,34 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import windowLocationInjectable from "../../common/k8s-api/window-location.injectable"; +import { getDiForUnitTesting } from "../getDiForUnitTesting"; +import currentlyInClusterFrameInjectable from "../routes/currently-in-cluster-frame.injectable"; +import rendererLogFileIdInjectable from "./renderer-log-file-id.injectable"; + +describe("renderer log file id", () => { + + it("clearly names log for renderer main frame", () => { + const di = getDiForUnitTesting(); + + di.override(currentlyInClusterFrameInjectable, () => false); + + const mainFileId = di.inject(rendererLogFileIdInjectable); + + expect(mainFileId).toBe("renderer-main"); + }); + + it("includes cluster id in renderer log file names", () => { + const di = getDiForUnitTesting(); + + di.override(currentlyInClusterFrameInjectable, () => true); + di.override(windowLocationInjectable, () => ({ + host: "some-cluster.lens.app", + port: "irrelevant", + })); + const clusterFileId = di.inject(rendererLogFileIdInjectable); + + expect(clusterFileId).toBe("renderer-cluster-some-cluster"); + }); +});