On Fri, 12 Jul 2024 at 15:58, Greg Sabino Mullane <htamfids@gmail.com> wrote:
On Thu, Jul 11, 2024 at 6:47 AM Alastair Turner <minion@decodable.me> wrote:
The other category of logging which would benefit from a separate file is audit. It also can create massive volumes of log content. Splitting audit information off into a separate file for use by a separate team or function is also a request I have heard from some financial institutions adopting Postgres. With audit being provided by an extension, this would become quite an intrusive change.
Thanks for the feedback. I agree pgaudit is another thing that can create massive log files, and should be solved at some point. However, I wanted to keep this patch self-contained to in-core stuff. And pgaudit is already an odd duck, in that it puts CSV into your stderr stream (and into your json!). Ideally it would put a single CSV stream into a separate csv file. Perhaps even something that did not necessarily live in log_directory.
Would an extension be able to safely modify the message_type field you have added using emit_log_hook? If so, the field becomes more of a log destination label than a type marker. If an extension could hook into the log file creation/rotation process, that would be a nice basis for enabling extensions (I'm particularly thinking of pgAudit) to manage separate logging destinations.
Yes, I had more than duration in mind when I created errmessagetype. A hook to set it would be the obvious next step, and then some sort of way of mapping that to arbitrary log files. But I see that as mostly orthagonal to this patch (and certainly a much larger endeavor).
Ok. This facility to separate out the logging of the duration messages is useful on its own and I can see the reasoning for using the core functionality for log rotation to manage these separate logs rather than redoing all the file handling work in an extension. A broader framework for mapping messages to arbitrary log files is a far larger set of changes which can be tackled later if desired.
I've had a look at the patch. The test cases look comprehensive. The patch applies cleanly. The newly supplied tests (13 of the 40) and the test_misc/003_check_guc (1 - no parameters missing from postgresql.conf.sample) fail.
To leave some runway for this idea to be extended on without disrupting the user experience could the GUC name be feature qualified as duration_log.log_destination? This would provide a clear naming structure for the most obvious follow-on patch to this one - allowing users to set log_directory separately for these duration logs - as well as any further separate logging efforts. I know that these dot-separated GUC names are generally associated with extensions, but I can't find a hard rule on the issue anywhere, and it feels like a reasonable way to group up the purpose (in this case logging duration messages) for which there are specific values of a number of GUCs (log_destination, log_directory, even log_filename, ...).
