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/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;

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),
+);

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;

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;

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<string, transports.FileTransportInstance>();
+
+    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;

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);
+  });
+});

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;

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",
+    });
+  });
+});

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;