Hello!

On Sun, May 30, 2021 at 01:02:24PM +0300, Gena Makhomed wrote:

> On 29.05.2021 0:55, Maxim Dounin wrote:
>
> >> Main purpose of variable $time_short is usage for logging, access.log.
> >>
> >> Existing variables $time_iso8601 and $time_local has bad readability,
> >> because these variables contains timezone after seconds and it has
> >> no space between date and time fields.
> >>
> >> Compare readability:
> >>
> >> 27/May/2021:21:19:12 +0300
> >> 27/May/2021:21:19:18 +0300
> >> 27/May/2021:21:19:25 +0300
> >> 27/May/2021:21:19:32 +0300
> >> 27/May/2021:21:19:43 +0300
> >>
> >> 2021-05-27T21:19:12+03:00
> >> 2021-05-27T21:19:18+03:00
> >> 2021-05-27T21:19:25+03:00
> >> 2021-05-27T21:19:32+03:00
> >> 2021-05-27T21:19:43+03:00
> >>
> >> 2021/05/27 21:19:12
> >> 2021/05/27 21:19:18
> >> 2021/05/27 21:19:25
> >> 2021/05/27 21:19:32
> >> 2021/05/27 21:19:43
> >>
> >> But only proposed variable $time_short has excellent readability,
> >> also this variable is very short, and contains no visual garbage.
> >
> > Well, if you want to improve readability and don't need a timezone
> > in your logs, something like this might work instead:
> >
> > map $time_iso8601 $time {
> > "~([0-9-]+)T([0-9:]+)" "$1 $2";
> > volatile;
> > }
> >
> > I don't think that introducing additional builtin variables for
> > additional data/time formats is a good idea.
>
> Using map directive for creating $time variable indeed allow us
> improve readability of log files, but this method is not cheap,
> - regular expressions runs for every record in access.log file.
>
> On busy servers (under DDoS attacks) can be hundreds of records
> per second and this regexp in map directive will just eat CPU.
>
> Human-readable variable $time / $time_short intended to use on
> busy servers, under DDoS-attacks, with Load Average 60 and more.
> See https://github.com/makhomed/autofilter for details about this.

While native implementation is by no means cheaper than a
map-based one, it's hardly comparable even with a single syscall.
Not to mention parsing the log line from a Python code you are
referring to. As long as you don't introduce catastrophic
backtracking in your regular expression, changing time format with
a regular expression should be good enough.

On the other hand, there are certainly other time formats to
introduce as well. And we certainly don't want to support many
native variables with all possible time formats. We already have
three - $time_local, $time_iso8601, and $msec.

> > I don't think that introducing additional builtin variables for
> > additional data/time formats is a good idea.
>
> Why not?
>
> Builtin variable ngx_cached_err_log_time already exists in nginx,
> patch for ngx_http_log_module is very small, just few lines of code:

The error log time format is to be used in error logs. Exposing
it as a variable leaves us two basic options:

- Document the variable as data/time in the "1970/09/28 12:00:00"
format. That is, if we'll ever consider changing the error log
time format, for example, to add milliseconds as already asked
multiple times, we'll have to introduce a separate builtin time
format for the variable.

- Document it as matching the format used for error log. That is,
if we'll change the error log format, the change will as sell
break the setup using the variable.

Both options do not look attractive to me. Especially given that
we already have time in three different formats available, and, as
outlined above, it's trivial to introduce additional time formats
by using map.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel