Re: Add jsonlog log_destination for JSON server logs

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sehrope Sarkuni <sehrope(at)jackdb(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, david(at)fetter(dot)org
Subject: Re: Add jsonlog log_destination for JSON server logs
Date: 2021-10-05 07:18:17
Message-ID: YVv8OYe1Bb5GHf5f@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 29, 2021 at 11:02:10AM +0900, Michael Paquier wrote:
> This happens to not be a problem as only 32 bits are significant for
> handles for both Win32 and Win64. This also means that we should be
> able to remove the use for "long" in this code, making the routines
> more symmetric. I have done more tests with Win32 and Win64, and
> applied it. I don't have MinGW environments at hand, but looking at
> the upstream code that should not matter. The buildfarm will let
> us know soon enough if there is a problem thanks to the TAP tests of
> pg_ctl.

So, I have been looking at the rest of the patch set for the last
couple of days, and I think that I have spotted all the code paths
that need to be smarter when it comes to multiple file-based log
destinations. Attached is a new patch set:
- 0001 does some refactoring of the file rotation in syslogger.c,
that's the same patch as previously posted.
- 0002 is more refactoring of elog.c, adding routines for the start
timestamp, log timestamp, the backend type and an extra one to check
if a query can be logged or not.
- 0003 is a change to send_message_to_server_log() to be smarter
regarding the fallback to stderr if a csvlog (or a jsonlog!) entry
cannot be logged because the redirection is not ready yet. The code
of HEAD processes first stderr, then csvlog, with csvlog moving back
to stderr if not done yet. That's a bit strange, because for example
on WIN32 we would lose any csvlog entry for a service. I propose here
to do csvlog first, and fallback to stderr so as it gets done in one
code path instead of two. I have spent quite a bit of time thinking
about the best way to handle the case of multiple file log
destinations here because we don't want to log multiple times to
stderr if csvlog and jsonlog are both enabled. And I think that this
is the simplest thing we could do.
- 0004 moves the CSV-specific code into its own file. This include
some refactoring of elog.c that should be moved to 0002, as this
requires more routines of elog.c to be published:
-- write_pipe_chunks()
-- error_severity()
- 0005 is the main meat, that introduces JSON as log_destination.
This compiles and passes all my tests, but I have not really done an
in-depth review of this code yet.

0002 and 0004 could be more polished and most of their pieces had
better be squashed together. 0003, though, would improve the case of
WIN32 where only csvlog is enabled so as log entries are properly
redirected to the event logs if the redirection is not done yet. I'd
like to move on with 0001 and 0003 as independent pieces.

Sehrope, any thoughts?
--
Michael

Attachment Content-Type Size
v4-0001-Refactor-per-destination-file-rotation-in-syslogg.patch text/x-diff 8.9 KB
v4-0002-Some-refactoring-of-elog-specific-routines.patch text/x-diff 8.7 KB
v4-0003-Refactor-fallback-to-stderr-for-some-cases-with-c.patch text/x-diff 3.3 KB
v4-0004-Refactor-CSV-specific-code-into-its-own-file.patch text/x-diff 17.0 KB
v4-0005-JSON-logging.patch text/x-diff 29.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2021-10-05 07:25:52 Re: jsonb crash
Previous Message Etsuro Fujita 2021-10-05 06:24:40 Re: RfC entries in CF 2021-09