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

Log file max size #254

Open
cloewen8 opened this issue Aug 9, 2020 · 17 comments
Open

Log file max size #254

cloewen8 opened this issue Aug 9, 2020 · 17 comments
Milestone

Comments

@cloewen8
Copy link

cloewen8 commented Aug 9, 2020

I see an option to specify a log file and log level but nothing to split or discard log files after a certain size. Is this not currently a feature, is it planned and if not, would a pull request to add this be welcome?

@truemedian
Copy link
Contributor

This is not currently a feature. Discordia currently opens the log file in append mode and pruning lines would be more work than necessary. However, this might be possible in 3.0.

@SinisterRectus
Copy link
Owner

Do you have limited disk space or is this just a matter of preference?

I am occasionally working on version 3.0 of the library which is expected to be released in October. There will be some logging enhancements in it. One that I am considering is using different logging states for different contexts (HTTP, WebSocket, other). File size management is something that I can consider. No promises though.

@SinisterRectus SinisterRectus added this to the 3.0 milestone Aug 9, 2020
@cloewen8
Copy link
Author

cloewen8 commented Aug 9, 2020

My biggest motivation for a feature like this would just be to limit how much memory is used. Limiting the size of the log file also makes it easier to view and open later on if needed. Not currently a problem, but all those writes add up.

@SinisterRectus
Copy link
Owner

Memory consumption due to the file size? Or due to console logging?

@cloewen8
Copy link
Author

cloewen8 commented Aug 9, 2020

Memory consumption due to the file size? Or due to console logging?

Due to file size.

I should also clarify, I'm not talking about modifying how logs are written. Rather which file is written to and when files are deleted.

@SinisterRectus
Copy link
Owner

Unless I'm mistaken, the log file does not consume volatile memory. Only disk space.

@cloewen8
Copy link
Author

cloewen8 commented Aug 9, 2020

Unless I'm mistaken, the log file does not consume volatile memory. Only disk space.

That is what I am referring to, although perhaps I explained it badly.

@SinisterRectus
Copy link
Owner

Alright just confirming.

@SinisterRectus
Copy link
Owner

I'm not sure that I want to implement this. Splitting log files based on file size doesn't seem productive because they would use the same disk space while automatically deleting log files would potentially remove useful information. I have a log file that goes back a year to June 2019 and its size is only 1.3 MB. Is that a real issue?

@cloewen8
Copy link
Author

Splitting log files is not usually meant to save space (anymore), rather it is to make accessing them easier (see log rotation: https://en.wikipedia.org/wiki/Log_rotation). For example, if a log file is split every day and a major bug occurs on a specific day, that log file can be saved and shared around to any applicable developers.

They don't need to be deleted, I can do that myself, but I'd really rather not have to automate splitting log files externally.

@Bilal2453
Copy link
Contributor

if a log file is split every day and a major bug occurs on a specific day, that log file can be saved and shared around to any applicable developers.

That honestly sounds like an interesting idea, it might make debugging kinda easier, and more organized for sure.

I am not sure though what would a limit on the size do with that idea, since you don't actually need to know the size nor to split according to the size.

As the idea sounds really interesting, and might actually be useful, it also sounds a bit problematic, for example, it will spam the log files, everyday a new log, you will end up with hundreds of log files on your VPS for example (which is similar to ending up with a single 2MB log file). That could be solved by using compressions and archiving, but at that point is it really worth the implementation and the limited time? It is also not that hard to actually access the current logs, I know it is kinda a mess and I agree that this idea will make them easier to access and read, but like, not by that much, since currently you can basically CTRL+f the current log for specific issue and/or specific date, which is kinda the same as accessing a log file using your method, because you will end up searching for a specific file by date/name where a bunch of log files exists. Both methods are about the same in matter of accessing them.

So can I ask why do you currently need that for @cloewen8 ? so we can decide if it is really worth it at this point or not, I can understand it make reading and accessing them kinda easier but why do you exactly need to access the logs that often, since usually you only need to access them once or two in the entire run.

And maybe... that could be implemented after Discordia 3.X (?)

@cloewen8
Copy link
Author

As the idea sounds really interesting, and might actually be useful, it also sounds a bit problematic, for example, it will spam the log files, everyday a new log, you will end up with hundreds of log files on your VPS for example (which is similar to ending up with a single 2MB log file). That could be solved by using compressions and archiving, but at that point is it really worth the implementation and the limited time? It is also not that hard to actually access the current logs, I know it is kinda a mess and I agree that this idea will make them easier to access and read, but like, not by that much, since currently you can basically CTRL+f the current log for specific issue and/or specific date, which is kinda the same as accessing a log file using your method, because you will end up searching for a specific file by date/name where a bunch of log files exists. Both methods are about the same in matter of accessing them.

Limiting the size of log files is not a new task (so much so that Linux comes with a rotatelog command). At the same time, I agree that this would not be useful for everyone. If it was to be added, it would need to be configurable. In general I would like the options to:

  • Specify a log folder.
  • Either limit the size or specify a time when, a log file is split.
  • Identify how logs are rotated: Limited amount that are written to, a new log file each time, a single new log file.

So can I ask why do you currently need that for @cloewen8 ? so we can decide if it is really worth it at this point or not, I can understand it make reading and accessing them kinda easier but why do you exactly need to access the logs that often, since usually you only need to access them once or two in the entire run.

I'd understand not adding log rotation in this version, it is nice, but not essential. Perhaps I just log a lot more then most people. After some point, opening a log file starts to slow down a text editor (sometimes even causing freezing). And downloading a single large log file takes a lot of unnecessary bandwidth.

I'd be happy to contribute log rotation through a pull request if no one else wants to work on this currently.

@truemedian
Copy link
Contributor

I don't see an actual reason this should implemented, I have a debug log for a small bot starting Jan 01 (~20 months) and it is only 431 MB. 306 MB of that is PRESENCE_UPDATE events being logged. For a normal bot you shouldn't even be using debug logging. (that same 20 month log is about 1.3 MB of logs if you were log only up to the info logLevel).

If for some reason you absolutely need only the logs for a specific day you can use something such as sed -e '/^YYYY-MM-DD/!d' discordia.log > YYYY-MM-DD.log to extract a specific days worth of logs

@cloewen8
Copy link
Author

I don't see an actual reason this should implemented

It should be implemented for the same reason any syntax sugar should be implemented. It's not necessary, but it makes things nicer.

@truemedian
Copy link
Contributor

It's not necessary, but it makes things nicer.

What does this make nicer? Splitting logs by days just adds more files to your system when you (imo) don't need it. Splitting logs by size makes even less sense, because then you're separating information just because you can.

What does this not make nicer? All of the logging code, because you'd either have to stat the log file, or check the current date (depending on how you're splitting the logs). And then either continue appending to the same file or close the current fd, open a new one, and then append to the new file.

@cloewen8
Copy link
Author

What does this make nicer?

Please check the previous comments. I've already noted what advantage this provides to me.

@SinisterRectus
Copy link
Owner

So far, in 3.0, I've added mutators for the Logger class and exposed a handle in the client. At the very least, users will have more dynamic control over the logging behavior this way. If you wanted, you could change the file as necessary. For example:

local client = discordia.Client()
local clock = discordia.Clock()

clock:on("day", function()
  client.logger:setFile("./logs/" .. os.date("%F") .. ".log")
end)

clock:start()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants