Re: Add jsonlog log_destination for JSON server logs

Sehrope Sarkuni <sehrope@jackdb.com>

From: Sehrope Sarkuni <sehrope@jackdb.com>

To: Michael Paquier <michael@paquier.xyz>

Cc: Pg Hackers <pgsql-hackers@postgresql.org>, david@fetter.org

Date: 2021-09-17T20:36:57Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Introduce log_destination=jsonlog
- dc686681e079 15.0 landed
Refactor set of routines specific to elog.c
- ac7c80758a7a 15.0 landed
Fix compilation warning in syslogger.c
- 05c4248ad1bf 15.0 landed
Refactor per-destination file rotation in logging collector
- 5c6e33f07153 15.0 landed
Refactor output file handling when forking syslogger under EXEC_BACKEND
- 5b0b699f748e 15.0 landed
Add regression tests for csvlog with the logging collector
- 72b76f76161c 15.0 landed
Refactor the syslogger pipe protocol to use a bitmask for its options
- 2d77d835403a 15.0 landed

Attachments

v6-0001-Split-out-syslogger-EXEC_BACKEND-fd-serialization-an.patch (text/x-patch) patch v6-0001
v6-0002-Refactor-syslogger-to-consolidate-common-tasks-for-f.patch (text/x-patch) patch v6-0002

On Thu, Sep 16, 2021 at 9:36 PM Michael Paquier <michael@paquier.xyz> wrote:

> I am not really impressed by 0001 and the introduction of
> LOG_DESTINATIONS_WITH_FILES.  So I would leave that out and keep the
> list of destinations listed instead.  And we are talking about two
> places here, only within syslogger.c.
>

I've taken that out for now. The idea was to simplify the additions when
jsonlog is added but can circle back to that later if it makes sense.

> 0002 looks like an improvement,

Nice. That's left unchanged (renamed to 0001 now).

> but I think that 0003 makes the
> readability of the code worse with the introduction of eight static
> routines to handle all those cases.
>
> file_log_dest_should_rotate_for_size(), file_log_dest_close(),
> file_log_dest_check_rotate_for_size(), get_syslogger_file() don't
> bring major improvements.  On the contrary,
> file_log_dest_write_metadata and file_log_dest_rotate seem to reduce a
> bit the noise.
>

It was helpful to split them out while working on the patch but I see your
point upon re-reading through the result.

Attached version (renamed to 002) adds only three static functions for
checking rotation size, performing the actual rotation, and closing. Also
one other to handle generating the logfile names with a suffix to handle
the pfree-ing (wrapping logfile_open(...)).

The rest of the changes happen in place using the new structures.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/