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

System > Settings > Logging > Max log filesize (MB) is actually GB #7397

Closed
huamiao opened this issue Apr 19, 2024 · 7 comments
Closed

System > Settings > Logging > Max log filesize (MB) is actually GB #7397

huamiao opened this issue Apr 19, 2024 · 7 comments
Assignees
Labels
bug Production bug
Milestone

Comments

@huamiao
Copy link

huamiao commented Apr 19, 2024

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug

System > Settings > Logging > Max log filesize (MB) is actually GB

This bug is a copy of #7273 since it did not use a valid template. And I also have the same issue.

To Reproduce

Steps to reproduce the behavior:

  1. Go to 'System > Settings > Logging'
  2. Set a proper value for "Maximum file size (MB)", e.g: 1 (MB)
  3. Check the log files under /var/log, Let's check firewall logs here under /var/log/filter/
  4. The log file could be more larger than 1MB but won't rotate. The value 1 means 1GB but not 1MB here.

Expected behavior

The log files should rotate when they reach the max file size.

Describe alternatives you considered

Please change MB to GB in the description or threat the value as MB, not GB. I personally prefer MB.

Screenshots
Configuration field as the screenshot
image

Relevant log files

N/A

Additional context

N/A

Environment

OPNsense 24.1.6 (amd64).

@fichtner
Copy link
Member

To be honest this looks fine:

if (!empty($maxfilesize) && (filesize($items[0]) / 1024 / 1024) > $maxfilesize) {

https://www.php.net/manual/en/function.filesize.php

"Returns the size of the file in bytes"

What datapoints are you looking at on the file system? A log file can grow as much as it will given the amount stored and the log rotation is kicked in every hour which could skew your conclusion. It's actually the minimum size to start rotation but it cannot alter a file if it has grown bigger.

Cheers,
Franco

@huamiao
Copy link
Author

huamiao commented Apr 19, 2024

I found the issue #7273 from this post [https://forum.opnsense.org/index.php?topic=38966.0]. And the post said the log won't be rotated after a day.
In my case, I'm looking at filter's log under /var/log/filter. As the screenshot, I set the max value to 10MB and the file grows to 40MB+ in 1 hour and won't rotate after 6 hours ( That's my last check and then I disable the logs). I'll try it again today.
I'm not familiar with php but the php manual said the filesize() result might be cached in some scenarios. Is that could be the reason?

@huamiao
Copy link
Author

huamiao commented Apr 19, 2024

Update:

  1. Set file limit to 1MB
    image
  2. Enabled firewall logs and the file size is 351KB at 22:44
    image
  3. Check the file at 22:47, file size is 1.3MB
    image
  4. Check the file at 23:37, file size is 15MB. And there is a latest link point to the log file which created at 23:01. This link should be created by the log_archive script.
    image
  5. Check the file at 23:48, file size is 18MB
image

As you see the oversized file was not archived. I'll keep watching tomorrow.

@huamiao
Copy link
Author

huamiao commented Apr 20, 2024

Update 2:

log_archive works properly at the second day:

  1. The new log file for the second day has been created
    image
  2. Wait for couple hours, the log files rotate as expected.
    image

Then, I reset the log (Disable firewall logs, delete all filter log files and Enable firewall logs). And do the steps again and the issue of first day just come back.

  1. Start log at 10:50, log file size is 39KB
  2. Check files at 13:40, log file size is 40MB, no rotate files.
image

@fichtner
Copy link
Member

IMO this could only mean it’s mismatching on the file to check. I’ll look at it next week.

@AdSchellevis AdSchellevis added bug Production bug and removed support Community support labels Apr 20, 2024
@AdSchellevis AdSchellevis self-assigned this Apr 20, 2024
@AdSchellevis AdSchellevis added this to the 24.7 milestone Apr 20, 2024
@AdSchellevis
Copy link
Member

The problem seems to be the "delete all logs" here, it needed at least two files to calculate the suffix, 73c3b88 should fix this.

@fichtner
Copy link
Member

👍

fichtner pushed a commit that referenced this issue Apr 22, 2024
…s only one file in the directory. closes #7397

The rotate function needed the next file to calculate the suffix, which is problematic if someone just cleaned all logs. This patch improves the match a bit (last _ instead of assuming _ doesn't exist in the name) and uses "1" for the first rotate action.

(cherry picked from commit 73c3b88)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Production bug
Development

No branches or pull requests

3 participants