Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logs in UI #4562

Merged
merged 12 commits into from
Dec 9, 2022
Merged

Logs in UI #4562

merged 12 commits into from
Dec 9, 2022

Conversation

NickM-27
Copy link
Collaborator

@NickM-27 NickM-27 commented Nov 30, 2022

Here is a demo of the new logs page

Screen.Recording.2022-12-05.at.11.38.35.AM.mov

@netlify
Copy link

netlify bot commented Nov 30, 2022

Deploy Preview for frigate-docs canceled.

Name Link
🔨 Latest commit ee8384e
🔍 Latest deploy log https://app.netlify.com/sites/frigate-docs/deploys/63921a0334ab3f00086f0923

@NickM-27
Copy link
Collaborator Author

@felipecrs I saw you commenting on just-containers/s6-overlay#252 and it seems you'll likely have a better understanding of this than I do. We need to log python, go2rtc, and nginx (and future services) to a file in memory (want to avoid writing logs to disk). It seems s6 is the best way to do this.

This log file in memory can then be read to show the logs in the UI, if you have any ideas I'd be happy to hear them

@felipecrs
Copy link
Contributor

I have to tinker with it a bit. I'll be back in a few hours with some snippets for you. :)

@NickM-27
Copy link
Collaborator Author

I have to tinker with it a bit. I'll be back in a few hours with some snippets for you. :)

I appreciate it

@felipecrs
Copy link
Contributor

I think it's finally time to upgrade to s6 v3... Logging is simpler with it. I'll push a PR.

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Dec 5, 2022

Will be rebased on dev once #4587 is merged

@NickM-27 NickM-27 marked this pull request as ready for review December 5, 2022 20:03
@felipecrs
Copy link
Contributor

I wonder if we should omit nginx and go2rtc logs from the container stdout now that we have this feature. I believe it may turn the add-on logs page very confusing with everything combined.

I also thought about prefixing the logs on stdout with the service name, but I'm not sure how exactly we can do that (just-containers/s6-overlay#447 (comment)).

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Dec 7, 2022

I wonder if we should omit nginx and go2rtc logs from the container stdout now that we have this feature. I believe it may turn the add-on logs page very confusing with everything combined.

I also thought about prefixing the logs on stdout with the service name, but I'm not sure how exactly we can do that (just-containers/s6-overlay#447 (comment)).

That's a question for @blakeblackshear I don't really have an opinion one way or the other.

@felipecrs
Copy link
Contributor

Well, actually it doesn't matter much. With the new UI users won't have to look at the container logs. Only if they want.

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Dec 7, 2022

Well, actually it doesn't matter much. With the new UI users won't have to look at the container logs. Only if they want.

right, I think that's the goal. Just need to make sure there are no downsides to the UI logs

@felipecrs
Copy link
Contributor

One future improvement I can think about is colorizing. :)

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Dec 7, 2022

I'm seeing the color characters for go2rtc just haven't had time to look into how html can be set to change them to colors. If you know the answer I'm all ears

@felipecrs
Copy link
Contributor

On a quick search I found this: https://www.npmjs.com/package/ansi-to-html

@felipecrs
Copy link
Contributor

It would be really nice if https://github.com/emilast/vscode-logfile-highlighter could be reused outside of VS Code. It can colorize the logs automatically.

@felipecrs
Copy link
Contributor

felipecrs commented Dec 7, 2022

Oh, there's one thing to have in mind that I only realized now:

Since #4587 the log files are written to memory (/dev/shm). Docker by default allocates 64MB for each container, but that can be increased per container with a CLI flag. Unfortunately, HA doesn't support configuring the shm size.

So, with the current logic, the log files can take up to 60MB. Is anything else writing data to /dev/shm? We can reduce the log files, like keeping no archives but increasing the log size to 15MB, or leaving it as 10MB (and maybe that's plenty already).

@NickM-27
Copy link
Collaborator Author

NickM-27 commented Dec 7, 2022

That's not quite true, the shm size can be increased for HA addons it just has to be set as one value for all addons. (edit: or maybe that was only for /tmp/cache, I can't remember)

And yes, we have the camera frames for detect stored in /dev/shm so that may be a concern for addon users.

@blakeblackshear
Copy link
Owner

I don't think we need to keep log archives beyond 10MB. Containers aren't really supposed to retain state like that, and we probably don't ever want to mess with trying to read those archives for the UI either. These files are essentially an in memory buffer for the UI to read logs. If users want a longer history of logs, they should do that by capturing the container logs and storing them on the host. Frigate also has fairly quiet logs normally. Even with 10MB, I suspect we will get support issues with "Bus error" crashes.

@blakeblackshear blakeblackshear merged commit 964bcc0 into blakeblackshear:dev Dec 9, 2022
@NickM-27 NickM-27 deleted the logs-in-ui branch December 9, 2022 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants