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 rotation and formatting #213

Closed
wants to merge 12 commits into from
Closed

Conversation

fluca1978
Copy link
Collaborator

This is a possible not-so-efficient implementation of log rotation and formatting (see #45 and #44).
The idea is that log_path supports strftime(3), so that rotation can be enabled. Also added log_rotation_age (seconds) and log_rotation_size (bytes) and log_line_prefix (string).
Example of final configuration:

log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log   
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

There are now several log_rotation_xxx functions that help keeping the code cleaner. If any of log_rotation_age or log_rotation_size parameters are enabled, and the logging is to a file, the rotation is enabled, otherwise is disabled.
Whenever a new entry is loggeg, the application looks for the need to rotate.
This is not efficient, since under heavy load the system could spend too much time in trying to understand if rotation is required.
If a rotation is required, the log file is closed and a new one is opened.
The log_rotation_age is used in conjunction with a global variable that keeps track of when the next rotation will happen.
Since rotation is evaluated only when a new log entry is issued, rotation will not happen automatically until the next log entry arrives. This means that is an almost-rotation.

Drawbacks:

  • rotation happens when a new log entry arrives. Probably we should improve this by means of a kind of "try every n entries", like inspecting for rotation only after 10 entries are logged to improve performances.
  • log_line_prefix does not allow spaces, since the extract_key_value does not handle spaces too.

We should also evaluate to store global variables next_log_rotation_age and current_log_path within the configuration structure, because they could be useful also outside of the logging.c file (I'm not sure).

The as_logging_type() function must return
the default logging type PGAGROAL_LOGGING_TYPE_CONSOLE
that has the value 0.
This does not change the behaviour of the application
but makes the code coherent with other
configuration parsing functions.
This is a possible implementation of log rotation.
The idea is to have the following configuration option:
- log_rotation_size expresses the bytes of max size for a file
  before its rotation;
- log_rotation_age expresses the seconds since the last
  rotation.

In order to allow log rotation, log_path must allow for
strftime(3) format strings.
Moreover, there is the need for a set of utility functions
to quickly open and rotate the log file.
The truncation is performed via the already existing
log_mode configuration option.

The parsing of log_roation_size accepts a number (bytes)
and a few suffixes, like 'M' for megabytes.
Similarly, log_rotation_age accepts a number (seconds)
and a few suffixes like 'd' for days.
Only one suffix is supported at a time.

If the logging is done to console or in general not to
a file, the rotation is disabled.

There is a global variable within logginc.g that
keeps track about the current filename as
"expanded" from strftime, so that when
a rotation is required, the file can be flushed
and closed and a new one can be opened.

Close agroal#45
Ensure that the log rotation will always be disabled
if the system is configured to log to something different
than a file.

Improvements on log rotation.

log_rotation_age is now expressed as seconds.
Avoid re-creating the log file when init + start are called in
sequence.
This introduces the configuration parameter `log_line_prefix`
that accepts a strftime(3) valid string that is parsed
and stored at the very first log entry in the configuration
structure (`config->log_line_prefix`).
Logging to a file and to the console is done using such
prefix via strftime.

Documentation updated.

Close agroal#44
@jesperpedersen
Copy link
Collaborator

I think most people will use info as their level, which means very little logging will happen.

So, having the overhead of each log entry is ok - debugging with debug5 will be very specific so minimal log size there as well. I don't think we need to support spaces in log_line_prefix, and likewise I don't think we need to account for gaps in the file naming.

All in all looks good - just need to whack the indentation monster

@jesperpedersen
Copy link
Collaborator

BTW, you could move the documentation to the .h file ? And, rebase on master ?

This is a possible implementation of log rotation.
The idea is to have the following configuration option:
- log_rotation_size expresses the bytes of max size for a file
  before its rotation;
- log_rotation_age expresses the seconds since the last
  rotation.

In order to allow log rotation, log_path must allow for
strftime(3) format strings.
Moreover, there is the need for a set of utility functions
to quickly open and rotate the log file.
The truncation is performed via the already existing
log_mode configuration option.

The parsing of log_roation_size accepts a number (bytes)
and a few suffixes, like 'M' for megabytes.
Similarly, log_rotation_age accepts a number (seconds)
and a few suffixes like 'd' for days.
Only one suffix is supported at a time.

If the logging is done to console or in general not to
a file, the rotation is disabled.

There is a global variable within logginc.g that
keeps track about the current filename as
"expanded" from strftime, so that when
a rotation is required, the file can be flushed
and closed and a new one can be opened.

Close agroal#45
Ensure that the log rotation will always be disabled
if the system is configured to log to something different
than a file.

Improvements on log rotation.

log_rotation_age is now expressed as seconds.
Avoid re-creating the log file when init + start are called in
sequence.
This introduces the configuration parameter `log_line_prefix`
that accepts a strftime(3) valid string that is parsed
and stored at the very first log entry in the configuration
structure (`config->log_line_prefix`).
Logging to a file and to the console is done using such
prefix via strftime.

Documentation updated.

Close agroal#44
@fluca1978
Copy link
Collaborator Author

Hope the code is clean now.

@jesperpedersen
Copy link
Collaborator

You merged my latest commit into your patch -- if you look at the "Files changed" tab you can see my patch. You need to rebase instead.

There are some whitespace and indentation issues still, but lets do a rebase first

@jesperpedersen
Copy link
Collaborator

You could do a git log to get the ids of your patches. Then git format-patch all of them. Then

git fetch upstream
git branch -m old_logging_rotation
git checkout master
git rebase -i upstream/master
git checkout -b logging_rotation master

and then start with git applys. At the end git push -f origin logging_rotation.

@jesperpedersen
Copy link
Collaborator

You can use tools like gitk to view the repository tree. The upstream/master commits should always be below your patch changes - in a straight line (no merge branches).

fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request Feb 28, 2022
See initial work in agroal#213.
This patch implements the log rotation and formatting support.
There are now three new configuration options:
- log_line_prefix is a strftime(3) compatible string that is used
  as a prefix whenever a new line is logged to console or file;
- log_rotation_age provides the age (in seconds) of when the log must
  be rotated. There is raw support for human strings like '1d' for
  'one day';
- log_rotation_size provides the size in bytes when the log must be
  rotated.

Log rotation is evaluated at every new log line issuing, that means
the age and the size are not honored effectively until a new log line
is issued. When the system decides to rotate the log, the log file
is flushed and a new one is opened. To allow rotation, log_path
supports a strftime(3) compatible string like
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
so that whnever a new log file must to be created, it can be
placed side by side the closed one.
In the case the new log file already exists, the log_mode is
used to decide if the log must be truncated or used in append mode.

In the case the log_mode is set to console or to syslog, the
log rotation is automatically disabled, therefore the overhead of
the rotation machinery will be applied only when logging to a file.

Example of configuration:

log_type  = file
log_level = info
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

Close agroal#44
Close agroal#45
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request Feb 28, 2022
See initial work in agroal#213.
This patch implements the log rotation and formatting support.
There are now three new configuration options:
- log_line_prefix is a strftime(3) compatible string that is used
  as a prefix whenever a new line is logged to console or file;
- log_rotation_age provides the age (in seconds) of when the log must
  be rotated. There is raw support for human strings like '1d' for
  'one day';
- log_rotation_size provides the size in bytes when the log must be
  rotated.

Log rotation is evaluated at every new log line issuing, that means
the age and the size are not honored effectively until a new log line
is issued. When the system decides to rotate the log, the log file
is flushed and a new one is opened. To allow rotation, log_path
supports a strftime(3) compatible string like
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
so that whnever a new log file must to be created, it can be
placed side by side the closed one.
In the case the new log file already exists, the log_mode is
used to decide if the log must be truncated or used in append mode.

In the case the log_mode is set to console or to syslog, the
log rotation is automatically disabled, therefore the overhead of
the rotation machinery will be applied only when logging to a file.

Example of configuration:

log_type  = file
log_level = info
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

Close agroal#44
Close agroal#45
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request Feb 28, 2022
See initial work in agroal#213.
This patch implements the log rotation and formatting support.
There are now three new configuration options:
- log_line_prefix is a strftime(3) compatible string that is used
  as a prefix whenever a new line is logged to console or file;
- log_rotation_age provides the age (in seconds) of when the log must
  be rotated. There is raw support for human strings like '1d' for
  'one day';
- log_rotation_size provides the size in bytes when the log must be
  rotated.

Log rotation is evaluated at every new log line issuing, that means
the age and the size are not honored effectively until a new log line
is issued. When the system decides to rotate the log, the log file
is flushed and a new one is opened. To allow rotation, log_path
supports a strftime(3) compatible string like
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
so that whnever a new log file must to be created, it can be
placed side by side the closed one.
In the case the new log file already exists, the log_mode is
used to decide if the log must be truncated or used in append mode.

In the case the log_mode is set to console or to syslog, the
log rotation is automatically disabled, therefore the overhead of
the rotation machinery will be applied only when logging to a file.

Example of configuration:

log_type  = file
log_level = info
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

Close agroal#44
Close agroal#45
@fluca1978
Copy link
Collaborator Author

See #216

@fluca1978 fluca1978 closed this Feb 28, 2022
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request Feb 28, 2022
See initial work in agroal#213.
This patch implements the log rotation and formatting support.
There are now three new configuration options:
- log_line_prefix is a strftime(3) compatible string that is used
  as a prefix whenever a new line is logged to console or file;
- log_rotation_age provides the age (in seconds) of when the log must
  be rotated. There is raw support for human strings like '1d' for
  'one day';
- log_rotation_size provides the size in bytes when the log must be
  rotated.

Log rotation is evaluated at every new log line issuing, that means
the age and the size are not honored effectively until a new log line
is issued. When the system decides to rotate the log, the log file
is flushed and a new one is opened. To allow rotation, log_path
supports a strftime(3) compatible string like
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
so that whnever a new log file must to be created, it can be
placed side by side the closed one.
In the case the new log file already exists, the log_mode is
used to decide if the log must be truncated or used in append mode.

In the case the log_mode is set to console or to syslog, the
log rotation is automatically disabled, therefore the overhead of
the rotation machinery will be applied only when logging to a file.

Example of configuration:

log_type  = file
log_level = info
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

Close agroal#44
Close agroal#45
jesperpedersen pushed a commit that referenced this pull request Mar 2, 2022
See initial work in #213.
This patch implements the log rotation and formatting support.
There are now three new configuration options:
- log_line_prefix is a strftime(3) compatible string that is used
  as a prefix whenever a new line is logged to console or file;
- log_rotation_age provides the age (in seconds) of when the log must
  be rotated. There is raw support for human strings like '1d' for
  'one day';
- log_rotation_size provides the size in bytes when the log must be
  rotated.

Log rotation is evaluated at every new log line issuing, that means
the age and the size are not honored effectively until a new log line
is issued. When the system decides to rotate the log, the log file
is flushed and a new one is opened. To allow rotation, log_path
supports a strftime(3) compatible string like
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
so that whnever a new log file must to be created, it can be
placed side by side the closed one.
In the case the new log file already exists, the log_mode is
used to decide if the log must be truncated or used in append mode.

In the case the log_mode is set to console or to syslog, the
log rotation is automatically disabled, therefore the overhead of
the rotation machinery will be applied only when logging to a file.

Example of configuration:

log_type  = file
log_level = info
log_path  = /var/log/pgagroal/pgagroal-%Y-%m-%d-%H-%M-%S.log
log_mode  = create
log_rotation_size = 2M
log_rotation_age = 1m
log_line_prefix  = PGAGROAL-%Y-%m-%d-%H:%M:%S'

Close #44
Close #45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants