Print full date and time in logs #618
Conversation
Signed-off-by: Paul Wayper <paulway@mabula.net>
|
Haven't checked if this compiles or not yet, and haven't removed the old One possibility I though of was implementing this as an option that could be turned on or off, say by a command-line or configuration option. Haven't investigated how to do that yet. |
Signed-off-by: Paul Wayper <paulway@mabula.net>
|
Please add flag / config option for that to avoid breaking changes |
|
OK - do you want a flag to turn on the new log format, or a flag to turn on the old log format? I.e. which log format should be the preferred? I'm curious to know what value you see in the old format? I'm happy to avoid a 'breaking' change in a significant feature or piece of functionality, but I'm not sure I see the utility of the old functionality. I suppose I see it as a bug to be fixed rather than a feature that needs to be preserved. Help me understand the significance of the old relative timestamp. |
|
And would you prefer that flag, e.g. |
|
This whole log printing is really puzzling me. There's two separate places which format log lines. One is in Is there a particular reason it's done this way? What I'd propose to do here is to format the log line once and then print that to stdout, the log file and syslog as required by the relevant flags. This would all be handled in the one function But I won't go ahead with that if there's strong opposition to it, or if there's a particular reason for doing it the current way which I don't understand. So please let me know what you'd prefer me to do here. |
This provides the 'use_new_timestamp_log_format' variable in `ns_turn_utils.h`. By default it is set to 0 and the old 'seconds since daemon was started' timestamp will be emitted. However, if it is set to 1 or any 'true' number the new date and time timestamp format will be used instead. This has also resulted in a small clean-up of some of the string length handling. Signed-off-by: Paul Wayper <paulway@mabula.net>
The function `turn_log_func_default` calls the function `vrtpprintf` to print to syslog or the log file. The latter does exactly the same string formatting as the former, so here we merge the two functions into one to do the string formatting once. This also makes sure that the log line is consistent on all outputs. Signed-off-by: Paul Wayper <paulway@mabula.net>
This allows the timestamp format in log output to be set by a function. Signed-off-by: Paul Wayper <paulway@mabula.net>
Signed-off-by: Paul Wayper <paulway@mabula.net>
This adds the `--new-log-timestamp` and `--new-timestamp-format <value>` options to the `turnserver` program. Setting `--new-log-timestamp` on the command line, or `new-log-timestamp` in the configuration file, will cause all logs to be written with an ISI-8601 timestamp (`YYYY-MM-DDTHH:MM:SSZZZZZ` with `T` being literal and `ZZZZZ` being `+` or `-` and the hour and minute offset from GMT for the local timezone). This replaces the 'number of seconds since daemon was started' format. Setting the `--new-timestamp-format <format>` option with a given format, or `new-log-timestamp=<format>` in the configuration file, will use this instead of the standard timestamp format. Timestamp format strings up to 48 characters can be accommodated; more will be truncated. This will only be used when the `--new-log-timestamp` option (above) is set. Thanks to Hendrik Huels <hendrik.huels@outlook.de> for the idea and some of the code for setting the log timestamp string. Signed-off-by: Paul Wayper <paulway@mabula.net>
ac4883c to
c315c28
Compare
|
So I've merged the log printing into one function, and allowed users to set the timestamp as well. Still trying to test the code to see if it actually works. |
|
@misi Any thoughts? |
|
@PaulWay Sorry for my late reply!
I want a flag for the new to be backward compatible.
I believe we need smooth transition. It could be someone built systems on it, so I propose to have new flag and indicate in the docs that it will flipped in the next version and your will be the default, and after one version iteration the default behavior will change to the new, to give time to prepare for the change. |
Unfortunately I am not the original author of the code, so I can't help in it why it was designed that way. We can simplify, but need to be backward compatible and not break any usage. So let's make it in a different PR.. |
|
We need to also think that wide OS supported so it should work and compatible with more exotic OSs like mac, win, etc. |
|
Hi @misi - thanks for that, sorry for not getting back to you earlier. Hope this all works well! |
|
@PaulWay I adjusted little bit (add doc, fix cli etc.) but it seems working fine now. Thanks for your help and contribution! |
The 'number of seconds since the daemon was started' is amusing and compact, but it hinders debugging and correlation with other events. This implements logs that use a complete
YYYY-MM-DDTHH:MM:SSoutput which is easy to read and correlate.Signed-off-by: Paul Wayper paulway@mabula.net