Logrotate support by azike · Pull Request #5191 · syslog-ng/syslog-ng

azike · 2024-11-27T14:09:22Z

Description

This pull-requests adds support for filesize-based logrotation (inspired by #4145) to counter problems with constraints on logfile partition sizes.
Limits on file size and number of rotations (history) are defined in the conf file and the destination driver makes sure to rotate logfiles after they reached the defined size.

Overview

The implementation is added directly to the affile module, as it is only effecting file destinations using regular-files. Keywords to allow setting logrotation options, like the maximum file size and number of rotations, are added to affile-grammar.

The AFFileDestDriver objects of the file destination holds a new field containing the logrotation settings (defined in modules/affile/logrotate.h), which are valid for all its writers of type AFFileDestWriter.
After the writer worker-thread performed its task (log_writer_work_perform) and flushed the incoming messages (log_writer_flush_finalize) it notifies its controlling AFFileDestWriter instance via log_pipe_notify. The actual checks, if rotation is to be performed, and the rotation itself are done in affile-dest.c and triggered through the notification.

During rotation the current logfile is renamed, therefore it has to be opened again. As the rotation is triggered from withing the worker-thread, a call to affile_dw_reopen would lead to a deadlock (thread waits on itself). The reopen is done by simply updating the fd of the LogProtoClient (no other thread modifies the LogProtoClient while it is still at work).
Alternatively a blocking main-thread call to open the file is possible, but I don't think it is necessary.

The main-thread watches are again registered afterwards in log_writer_work_finished.

Usage

Below is an example conf file which uses an example_msg_generator source to emit 1000 messages. The file destinations are annotated with the logrotate(...) keyword specifying its rotation settings. Macros in the filename are still possible:

@version: 4.8
@include "scl.conf"

source s_sys{
    system();
};

source s_gen {
    example_msg_generator (
        num(1000)
        freq(0.001)
        template("Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut")
    );
};

# works with MACROs
destination d_file1 {
    file("/tmp/logfile_$HOST.log", logrotate(enable(yes), size(30KB), rotate(5)));
};

destination d_file2 {
    file("/tmp/logfile_simple.log", logrotate(enable(yes), size(20KB), rotate(3)));
};


log {
    source(s_gen);
    source(s_sys);
    destination(d_file1);
    destination(d_file2);
};

TODO's and ideas

Up until now only filesize based rotation is implemented.
No support for compression of rotated files (which might be possible to schedule in a different thread).
On high load imposed by the message source the additional delay through rotation may lead to flow-control and the suspension of the source. I only tested it with the example_msg_generator source set to freq(0.0001), which lead to message loss. However, I am not sure it that frequency is realistic or how other type of source behave when being suspended.
Maybe it makes sense to outsource the renaming of already rotated logfiles to another thread.

Related topics

Resolves #2964
Documentation PR syslog-ng/syslog-ng.github.io#240

kira-syslogng · 2024-11-27T14:09:27Z

Can one of the admins verify this patch?

kira-syslogng · 2024-11-27T14:09:27Z

Can one of the admins verify this patch?

czanik · 2024-11-28T13:06:08Z

Thank you for your contribution! The syslog-ng team will review your code as soon as it finds a bit of time.

Note that daily, weekly, but even hourly rotations are already supported by syslog-ng: you just have to use the time macros in the file names.

azike · 2024-11-28T14:17:51Z

I updated the branch since the pipeline failed previously. Could you restart the checks?

azike · 2025-01-08T07:09:12Z

Hi!
Are there any updates or suggestions from the maintainer team regarding this PR?

BeBinder · 2025-01-28T12:51:37Z

Will this patch be included in future versions of syslog-ng?

HofiOne · 2025-02-21T12:55:15Z

@azike, @BeBinder sorry for the delay, we will pick this up in the near future hopefully, it is at the top of our list, thank you!

HofiOne · 2025-03-04T11:22:17Z

@azike I've re-targeted the PR to the develop branch, from now and on we will use that one primarily for development, master will be dedicated to release builds
thank you.

azike · 2025-03-04T15:02:15Z

Alright! Thank you.

modules/affile/logrotate.c

modules/affile/Makefile.am

modules/affile/affile-grammar.ym

modules/affile/affile-dest.c

lib/logwriter.c

github-actions · 2025-03-12T16:08:50Z

This Pull Request introduces config grammar changes

syslog-ng/1f8286d25722ba0fa45a5d3ae613e8d6c9dbcc5b -> azike/syslog-ng-logrotation

Details

--- a/destination
+++ b/destination

 file(
+    logrotate(
+        <empty>
+        enable(<yesno>)
+        rotations(<positive-integer>)
+        size(<positive-integer>)
+    )
 )

azike · 2025-03-13T07:18:55Z

I updated the last commit message since the commits check pipeline failed.

lib/logproto/logproto-client.h

modules/affile/affile-dest.c

lib/logwriter.c

modules/affile/affile-dest.c

lib/logwriter.c

modules/affile/affile-dest.c

lib/logwriter.c

modules/affile/affile-dest.c

azike · 2025-03-17T15:19:42Z

I added the changes you suggested, but as I already stated we should somehow limit the number of logrotate checks. Doing it for every message could cost performance and in the end logs. If I run my functional-test with make light-check, it fails now because I guess the sink can not keep up.

HofiOne · 2025-03-17T15:45:09Z

I added the changes you suggested, but as I already stated we should somehow limit the number of logrotate checks. Doing it for every message could cost performance and in the end logs. If I run my functional-test with make light-check, it fails now because I guess the sink can not keep up.

yepp, plz see my comment above, I guess a size caching can solve this too

azike · 2025-03-20T15:23:24Z

I just checked it briefly but what I found out is that the LogWriter object keeps track of some metrics including written_bytes, which should get updated here. However, as it seems the underlying counter is not initialized and set to NULL. I did not dig any deeper but I guess the should be a good starting point.
If we can keep track of the written_bytes per writer the size check can be done in memory only and when the limit is reached, the actual file size can be verifyied using stat.

I am away for three weeks now so, you will not hear any updates from me in that time.

HofiOne · 2025-03-20T16:28:21Z

I just checked it briefly but what I found out is that the LogWriter object keeps track of some metrics including written_bytes, which should get updated here. However, as it seems the underlying counter is not initialized and set to NULL. I did not dig any deeper but I guess the should be a good starting point. If we can keep track of the written_bytes per writer the size check can be done in memory only and when the limit is reached, the actual file size can be verifyied using stat.

I am away for three weeks now so, you will not hear any updates from me in that time.

I'd stay away from using stat counters that require costly locking. I'd vote for a local counter instead.

azike · 2025-04-16T12:29:38Z

Hi! I am back now and pushed two ideas I had within the last two commits. Because of the layering that is going it is a little trickier than thought because directly reporting to other layers is not that straightforward.
The major difference between my two approaches is when to call stat to get the current actual filesize.

First I called it directly at the beginning of log_writer_flush and then kept adding the message size reported by the write function. However, in this case stat is always called even if logrotation is disabled because the logwriter does not know about logrotation in gerneral.
Get the filesize when (re-)opening a file and store it in AFFileDestWriter. The size gets updated by calling log_pipe_notify after every message written reporting its output buffer size. This way stat is only called once when the file is opened.

In both cases logrotation only checks against this in-memory cached_filesize. This seems to work so far, also my testcase works again.

Again there are some subtle points that need attention and there I guess I need some help regarding how to deal with them and to not interfere with the design choices of the project. I will discuess them in the comments below.

lib/logwriter.c

modules/affile/affile-dest.c

Logrotation is performed in the function do_logrotate and for now implements a simple size based rotation. There is no in-memory storage or list to keep track of rotated files. With each rotation the number of log files is established through the filenames and their rotation suffixes. The log files are first scanned starting with the main log file until the first roation is found that is not yet 'used'. Then the files are rotated up to that index. If there are 'holes' in the sequence of log files (e.g. file.log, file.log.1, file.log.3 --> file.log.2 is missing) the files that come after that missing one are not rotated. This should prevent the deletion of logfiles in case a reopen attempt fails. Signed-off-by: Alexander Zikulnig <[email protected]>

Add new keywords to affile-grammar to enable the specification of logrotate-settings in the syslog-ng config file. The settings are stored in the current AFFileDestDriver object and are available to all its writer objects through the field logrotate_options. Currently the following keywords are supported: - 'enable' -> turn on/off logrotation - 'size' -> set max file size - 'rotations' -> number of rotations Signed-off-by: Alexander Zikulnig <[email protected]>

Split up reopen function of AFFileDestWriter to handle the logwriter reopen separately. Signed-off-by: Alexander Zikulnig <[email protected]>

Whenever the log file is reopened its current size is read and cached as part of the AFFileDestwriter object. Signed-off-by: Alexander Zikulnig <[email protected]>

Signed-off-by: Alexander Zikulnig <[email protected]>

React to log_pipe_notify with code NC_LOGROTATE for AFFileDestWriter objects. Upon notification it is checked if logrotation should be performed based on the settings stored in logrotate_options and the current cached filesize. The function is intended to be called by the current logwriter worker thread when flushing log messages. If logrotation and reopen of the log file were successful the caller is informed trough log_pipe_notify return codes. Note that affile_dw_reopen can not be called from within log_writer_work_perform as it would result in a deadlock (worker waits on itself to finish). Signed-off-by: Alexander Zikulnig <[email protected]>

azike · 2025-09-04T15:53:15Z

Hi! I pushed the changes. Please check if those are resolved now.

azike · 2025-09-04T15:57:38Z

@azike The current state LGTM.

Could you please update the corresponding documentation parts as well? Thank you in advance!

I forked the repo and started to add the documentation, but I was wondering what I should write for type? Does parameter list of the logrotate() option suffice plus describing the parameters below in the descriptions part?

HofiOne · 2025-09-04T16:06:45Z

@azike The current state LGTM.
Could you please update the corresponding documentation parts as well? Thank you in advance!

I forked the repo and started to add the documentation, but I was wondering what I should write for type? Does parameter list of the logrotate() option suffice plus describing the parameters below in the descriptions part?

@azike you can follow the schema that can be seen in the final, rendered version

like

https://syslog-ng.github.io/admin-guide/070_Destinations/040_File/000_File_destination_options

If logrotation has been performed before but the log file could not be reopened then rotation should not be done again on a subsequent logrotate call. Signed-off-by: Alexander Zikulnig <[email protected]>

Signed-off-by: Alexander Zikulnig <[email protected]>

The logwriter instance notifies its DestWriter object after each log message written. The call contains the length of the log message to keep track of the current filesize. In case logrotation fails the logwriter instance is notified and the ongoing flush is canceled. Signed-off-by: Alexander Zikulnig <[email protected]>

- test_logrotate_all_messages_received: Verify all messages that are sent are received and files are rotated. - test_logrotate_deleting_log_file: Delete main log file and verify it is reopened. - test_logrotate_rotation_failure: Delete first rotated log file and trigger rotation again. Verify oldest log file is not deleted. Signed-off-by: Alexander Zikulnig <[email protected]>

Signed-off-by: Alexander Zikulnig <[email protected]>

azike · 2025-09-05T06:05:00Z

Do you also need tests for the config parser? Like here https://github.com/syslog-ng/syslog-ng/blob/develop/modules/affile/tests/test_wildcard_source.c?

Add description of `logrotate` feature introduced in syslog-ng/syslog-ng#5191.

HofiOne · 2025-09-05T07:41:48Z

Do you also need tests for the config parser? Like here https://github.com/syslog-ng/syslog-ng/blob/develop/modules/affile/tests/test_wildcard_source.c?

@azike it would be nice and useful

EDIT: I suggest moving that to the next iteration. I'll merge this now, thanks.

therandomstring

LGTM

azike · 2025-09-05T08:34:18Z

Cool! Thanks guys.

HofiOne · 2025-09-05T10:26:50Z

@azike Thanks for your work! Unfortunately, some errors occur now in our Kira tests that should have happened in the earlier runs here. I investigate how earlier the Kira runs did not find the problem, and what the reason is of the failures.

azike force-pushed the syslog-ng-logrotation branch from 872aa24 to 5689d19 Compare November 28, 2024 14:16

HofiOne changed the base branch from master to develop March 4, 2025 11:22

HofiOne requested changes Mar 4, 2025

View reviewed changes

azike commented Mar 11, 2025

View reviewed changes

lib/logwriter.c Outdated Show resolved Hide resolved

azike force-pushed the syslog-ng-logrotation branch from 5cef81b to 5e8a1ef Compare March 13, 2025 07:15

azike commented Mar 13, 2025

View reviewed changes

lib/logproto/logproto-client.h Outdated Show resolved Hide resolved

azike commented Mar 17, 2025

View reviewed changes

modules/affile/affile-dest.c Outdated Show resolved Hide resolved

azike commented Mar 17, 2025

View reviewed changes

lib/logwriter.c Outdated Show resolved Hide resolved

azike force-pushed the syslog-ng-logrotation branch from 1e45907 to 307c3d1 Compare March 17, 2025 12:14

HofiOne requested changes Mar 17, 2025

View reviewed changes

azike force-pushed the syslog-ng-logrotation branch from 307c3d1 to cd4f602 Compare March 17, 2025 15:08

azike commented Apr 16, 2025

View reviewed changes

lib/logwriter.c Outdated Show resolved Hide resolved

azike commented Apr 16, 2025

View reviewed changes

modules/affile/affile-dest.c Outdated Show resolved Hide resolved

azike commented Apr 16, 2025

View reviewed changes

modules/affile/affile-dest.c Outdated Show resolved Hide resolved

azike added 7 commits September 4, 2025 17:42

affile: refactor affile_dw_reopen function

826f40d

Split up reopen function of AFFileDestWriter to handle the logwriter reopen separately. Signed-off-by: Alexander Zikulnig <[email protected]>

affile: add file size chaching to AFFileDestWriter

619350b

Whenever the log file is reopened its current size is read and cached as part of the AFFileDestwriter object. Signed-off-by: Alexander Zikulnig <[email protected]>

lib: add log_pipe_notify code for logrotation

5659857

Signed-off-by: Alexander Zikulnig <[email protected]>

lib: add log_pipe_notfiy error return code

e31dac4

Signed-off-by: Alexander Zikulnig <[email protected]>

azike dismissed HofiOne’s stale review via a0ad786 September 4, 2025 15:50

azike force-pushed the syslog-ng-logrotation branch from 37c938a to a0ad786 Compare September 4, 2025 15:50

azike added 5 commits September 4, 2025 18:17

affile: check for pending logrotate calls

b922f94

If logrotation has been performed before but the log file could not be reopened then rotation should not be done again on a subsequent logrotate call. Signed-off-by: Alexander Zikulnig <[email protected]>

lib: modify log_writer_write_message to return message length

5ea8f50

Signed-off-by: Alexander Zikulnig <[email protected]>

news: feature-5191

c80128a

Signed-off-by: Alexander Zikulnig <[email protected]>

azike force-pushed the syslog-ng-logrotation branch from a0ad786 to c80128a Compare September 4, 2025 16:18

azike mentioned this pull request Sep 4, 2025

Add logrotate documentation syslog-ng/syslog-ng.github.io#240

Merged

HofiOne requested a review from therandomstring September 4, 2025 17:27

HofiOne added a commit to syslog-ng/syslog-ng.github.io that referenced this pull request Sep 5, 2025

Add logrotate documentation (#240)

86d6881

Add description of `logrotate` feature introduced in syslog-ng/syslog-ng#5191.

therandomstring approved these changes Sep 5, 2025

View reviewed changes

HofiOne merged commit 1f37484 into syslog-ng:develop Sep 5, 2025
60 checks passed

azike deleted the syslog-ng-logrotation branch September 5, 2025 10:19

This was referenced Sep 5, 2025

Revert "Logrotate support " #5461

Closed

Revert "Logrotate support" #5462

Closed

Conversation

azike commented Nov 27, 2024 • edited by HofiOne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Overview

Usage

TODO's and ideas

Related topics

Uh oh!

kira-syslogng commented Nov 27, 2024

Uh oh!

kira-syslogng commented Nov 27, 2024

Uh oh!

czanik commented Nov 28, 2024

Uh oh!

azike commented Nov 28, 2024

Uh oh!

azike commented Jan 8, 2025

Uh oh!

BeBinder commented Jan 28, 2025

Uh oh!

HofiOne commented Feb 21, 2025

Uh oh!

HofiOne commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azike commented Mar 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This Pull Request introduces config grammar changes

Uh oh!

azike commented Mar 13, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azike commented Mar 17, 2025

Uh oh!

HofiOne commented Mar 17, 2025

Uh oh!

azike commented Mar 20, 2025

Uh oh!

HofiOne commented Mar 20, 2025

Uh oh!

azike commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azike commented Sep 4, 2025

Uh oh!

azike commented Sep 4, 2025

Uh oh!

HofiOne commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azike commented Sep 5, 2025

Uh oh!

HofiOne commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

therandomstring left a comment

azike commented Nov 27, 2024 •

edited by HofiOne

Loading

HofiOne commented Mar 4, 2025 •

edited

Loading

github-actions bot commented Mar 12, 2025 •

edited

Loading

azike commented Apr 16, 2025 •

edited

Loading

HofiOne commented Sep 4, 2025 •

edited

Loading

HofiOne commented Sep 5, 2025 •

edited

Loading