Renderer file logging transport #6795

samitiilikainen · 2022-12-20T12:39:05Z

Add file logging to renderer, writing separate log files for renderer main frame and each cluster frame.

Closes file handles (otherwise left open by Winston) on main and cluster frame refresh and when cluster frame is closed through cluster disconnect.

Related to lensapp/support-lens-extension#118

@Nokel81

Move navigation logging to `setupLoggingForNavigationInjectable` to prevent cycle of injectables from occurring. Wasn't eventually needed for #6795 but still an improvement. Credit for the implementation goes to @Nokel81 , thanks!

Nokel81 · 2022-12-20T12:53:02Z

@samitiilikainen Please sign your commit

@Nokel81

Move navigation logging to `setupLoggingForNavigationInjectable` to prevent cycle of injectables from occurring. Wasn't eventually needed for #6795 but still an improvement. Credit for the implementation goes to @Nokel81 , thanks! Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

@Nokel81

Move navigation logging to `setupLoggingForNavigationInjectable` to prevent cycle of injectables from occurring. Wasn't eventually needed for #6795 but still an improvement. Credit for the implementation goes to @Nokel81 , thanks! Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

github-actions · 2022-12-23T11:26:57Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2022-12-23T11:49:06Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions · 2023-01-09T21:18:04Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2023-01-24T18:48:53Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2023-01-27T07:39:19Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

jim-docker

I noticed we have many console.warn etc calls still in the core code. They show up in the dev tools console but not in the log file of course. Out of scope for this PR, just noting it.

jim-docker · 2023-01-27T16:41:50Z

packages/core/src/renderer/logger/file-transport.injectable.ts

+import currentlyInClusterFrameInjectable from "../routes/currently-in-cluster-frame.injectable";
+import { getClusterIdFromHost } from "../utils";
+
+const rendererFileLoggerTransportInjectable = getInjectable({


usually the filename matches the injectable name/id ?

jim-docker · 2023-01-27T16:46:36Z

packages/core/src/renderer/frames/cluster-frame/init-cluster-frame/init-cluster-frame.ts

+    });
+
+    window.addEventListener("beforeunload", onCloseFrame, { capture: true });
+    window.addEventListener("pagehide", onCloseFrame, { capture: true });


Does this mean logging to file stops if the cluster frame is not in view? If I disconnect a cluster while on the catalog cluster list, any disconnect logging is missed?

Not all logging. It stops cluster frame logging right before the iframe is removed from DOM (when also all code execution in that iframe should stop). Stopping logging of the cluster frame does not affect logging done by the renderer main frame (catalog page).

jim-docker · 2023-01-27T18:26:25Z

packages/core/src/renderer/logger/file-transport.injectable.ts

+      maxsize: 1024 * 1024,
+      maxFiles: 0,
+      tailable: true,


Not sure what the expectation is here, but I tested with a file that was just under 1M in size and observed that when the file reached 1M nothing more was written to the file or any other file, and nothing more was written to the developer tools console either??

rendererlogging.mov

This is on Ubuntu 22.04. I artificially padded the file with logs to get it near 1M in size.

Thanks! Expectation was that it would keep writing logs and older lines would be lost but I was mistaken. I'll increase the maxFiles to allow it to properly rotate files.

Add file logging to renderer, writing separate log files for renderer main frame and each cluster frame. Related to lensapp/support-lens-extension#118 Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

There is too much noise on debug level from api responses etc Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

It seems cluster onbeforeunload is not triggered when iframe is removed from dom by the parent (when disconnecting a cluster). While on root/main frame the beforeunload event does work, it seems to be adviced to use pagehide instead. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

This should cover reloading main and cluster frames and closing cluster frame throught disconnecting cluster. No file handles should be left open now. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

jim-docker

Thanks for doing this!

(users with many many clusters may notice large disk usage by Lens due to all the new log files)

Does not seem to be needed for log files to be successfully closed and caused integration tests to fail. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Nokel81

Finally looks like this is green!

Renderer file logging still caused UI freezing (at least on apple silicon macs) when cluster frame was open and main frame was reloaded. See #544 This reverts commit ac2d0e4.

Renderer file logging still caused UI freezing (at least on apple silicon macs) when cluster frame was open and main frame was reloaded. See #544 This reverts commit ac2d0e4. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

* General fixes for release-tool (#7238) * General fixes for release-tool Signed-off-by: Sebastian Malton <sebastian@malton.name> * Revert change to number of PRs retrieved Signed-off-by: Sebastian Malton <sebastian@malton.name> --------- Signed-off-by: Sebastian Malton <sebastian@malton.name> * Throw on errors in kubectlApplyFolder (#7239) Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com> * Quick fix for store migration version being wrong (#7243) Signed-off-by: Sebastian Malton <sebastian@malton.name> * Revert "Renderer file logging transport (#6795)" (#7245) Renderer file logging still caused UI freezing (at least on apple silicon macs) when cluster frame was open and main frame was reloaded. See #544 This reverts commit ac2d0e4. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> * Fix extension install (#7247) * Fix extension install - Remove old bundled extension dependencies - Make sure external extensions are installed as optional Signed-off-by: Sebastian Malton <sebastian@malton.name> * Ignore ENOENT errors Signed-off-by: Sebastian Malton <sebastian@malton.name> * Add comment Signed-off-by: Sebastian Malton <sebastian@malton.name> --------- Signed-off-by: Sebastian Malton <sebastian@malton.name> * Release 6.4.0 Signed-off-by: Sebastian Malton <sebastian@malton.name> * Fix lint Signed-off-by: Sebastian Malton <sebastian@malton.name> --------- Signed-off-by: Sebastian Malton <sebastian@malton.name> Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com> Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com> Co-authored-by: Panu Horsmalahti <phorsmalahti@mirantis.com> Co-authored-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

samitiilikainen requested review from Nokel81 and a team December 20, 2022 12:39

samitiilikainen requested a review from a team as a code owner December 20, 2022 12:39

samitiilikainen requested review from jim-docker and removed request for a team December 20, 2022 12:39

samitiilikainen mentioned this pull request Dec 20, 2022

Navigation logging injectable #6797

Merged

Nokel81 added the enhancement New feature or request label Dec 20, 2022

Nokel81 added this to the 6.4.0 milestone Dec 20, 2022

Nokel81 previously approved these changes Dec 20, 2022

View reviewed changes

samitiilikainen dismissed Nokel81’s stale review via a7496aa December 20, 2022 13:24

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from 4b3aa2e to a7496aa Compare December 20, 2022 13:24

samitiilikainen marked this pull request as draft December 20, 2022 14:02

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from 267e139 to bdc0878 Compare December 21, 2022 12:17

Nokel81 modified the milestones: 6.3.0, 6.4.0 Dec 21, 2022

github-actions bot added the PR: needs rebase label Dec 23, 2022

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from 37d7c96 to 3054af9 Compare December 23, 2022 11:48

github-actions bot removed the PR: needs rebase label Dec 23, 2022

samitiilikainen marked this pull request as ready for review December 23, 2022 12:02

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from 3054af9 to ce33663 Compare January 9, 2023 11:28

github-actions bot added the PR: needs rebase label Jan 9, 2023

Nokel81 force-pushed the feature/renderer-file-logging-2 branch from ce33663 to 8025482 Compare January 13, 2023 16:59

github-actions bot removed the PR: needs rebase label Jan 13, 2023

panuhorsmalahti force-pushed the feature/renderer-file-logging-2 branch from 8025482 to 0b6333d Compare January 23, 2023 09:15

github-actions bot added the PR: needs rebase label Jan 24, 2023

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from 0b6333d to c21ca70 Compare January 27, 2023 07:39

github-actions bot removed the PR: needs rebase label Jan 27, 2023

jim-docker reviewed Jan 27, 2023

View reviewed changes

samitiilikainen added 7 commits January 30, 2023 14:10

Renderer file logging transport

03d51a7

Add file logging to renderer, writing separate log files for renderer main frame and each cluster frame. Related to lensapp/support-lens-extension#118 Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Switch renderer file log level to info

e1da883

There is too much noise on debug level from api responses etc Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Close log files on unmount

0c83cfb

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Improve file handle closing in different situations

da946cf

This should cover reloading main and cluster frames and closing cluster frame throught disconnecting cluster. No file handles should be left open now. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Fix renderer log rotation

7a4e3a7

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

Switch back to beforeunload in root frame

e35e470

Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

samitiilikainen force-pushed the feature/renderer-file-logging-2 branch from c0ed6f7 to e35e470 Compare January 30, 2023 12:40

jim-docker previously approved these changes Jan 30, 2023

View reviewed changes

Remove capturing phase event usage

fbb17b7

Does not seem to be needed for log files to be successfully closed and caused integration tests to fail. Signed-off-by: Sami Tiilikainen <97873007+samitiilikainen@users.noreply.github.com>

samitiilikainen dismissed jim-docker’s stale review via fbb17b7 February 6, 2023 14:30

Nokel81 approved these changes Feb 6, 2023

View reviewed changes

Nokel81 merged commit ac2d0e4 into master Feb 8, 2023

Nokel81 deleted the feature/renderer-file-logging-2 branch February 8, 2023 10:50

Nokel81 mentioned this pull request Feb 22, 2023

Release 6.4.0-beta.14 #7213

Merged

Nokel81 mentioned this pull request Mar 1, 2023

Release 6.4.0 #7250

Merged

samitiilikainen mentioned this pull request Mar 7, 2023

Renderer file logging through IPC #7300

Merged

samitiilikainen mentioned this pull request Apr 4, 2023

Renderer file logging through IPC #7499

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renderer file logging transport #6795

Renderer file logging transport #6795

samitiilikainen commented Dec 20, 2022 •

edited

Nokel81 commented Dec 20, 2022

github-actions bot commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

github-actions bot commented Jan 9, 2023

github-actions bot commented Jan 24, 2023

github-actions bot commented Jan 27, 2023

jim-docker left a comment

jim-docker Jan 27, 2023

jim-docker Jan 27, 2023

samitiilikainen Jan 30, 2023

jim-docker Jan 27, 2023

samitiilikainen Jan 30, 2023

jim-docker left a comment

Nokel81 left a comment

Renderer file logging transport #6795

Renderer file logging transport #6795

Conversation

samitiilikainen commented Dec 20, 2022 • edited

Nokel81 commented Dec 20, 2022

github-actions bot commented Dec 23, 2022

github-actions bot commented Dec 23, 2022

github-actions bot commented Jan 9, 2023

github-actions bot commented Jan 24, 2023

github-actions bot commented Jan 27, 2023

jim-docker left a comment

Choose a reason for hiding this comment

jim-docker Jan 27, 2023

Choose a reason for hiding this comment

jim-docker Jan 27, 2023

Choose a reason for hiding this comment

samitiilikainen Jan 30, 2023

Choose a reason for hiding this comment

jim-docker Jan 27, 2023

Choose a reason for hiding this comment

samitiilikainen Jan 30, 2023

Choose a reason for hiding this comment

jim-docker left a comment

Choose a reason for hiding this comment

Nokel81 left a comment

Choose a reason for hiding this comment

samitiilikainen commented Dec 20, 2022 •

edited