jsonlog missing from logging_collector description
Hi Hackers,
Enabling logging_collector is required for json log. During jsonlog
implementation [1]/messages/by-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com, log destination and logging_collector comments in
postgresql.conf.sample and documentation was updated but
logging_collector short description was not updated in guc.c (which is
guc_table.c now).
Attached is the patch for this minor fix.
[1]: /messages/by-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Regards
--
Umar Hayat
Bitnine (https://bitnine.net/)
Attachments:
v0-logging-collector-jsonlog.patchapplication/octet-stream; name=v0-logging-collector-jsonlog.patchDownload
From ae9bf934cadd4f3aba62847ea071af3ec1b407df Mon Sep 17 00:00:00 2001
From: Umar Hayat <postgresql.wizard@gmail.com>
Date: Fri, 31 Jan 2025 17:55:13 +0900
Subject: [PATCH] Add jsonlog in logging_collector short description
---
src/backend/utils/misc/guc_tables.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 38cb9e970d..a2ddea1837 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1692,7 +1692,7 @@ struct config_bool ConfigureNamesBool[] =
},
{
{"logging_collector", PGC_POSTMASTER, LOGGING_WHERE,
- gettext_noop("Start a subprocess to capture stderr output and/or csvlogs into log files."),
+ gettext_noop("Start a subprocess to capture stderr, csvlog and/or jsonlog into log files."),
NULL
},
&Logging_collector,
--
2.31.0
On Fri, Jan 31, 2025 at 06:09:48PM +0900, Umar Hayat wrote:
Enabling logging_collector is required for json log. During jsonlog
implementation [1], log destination and logging_collector comments in
postgresql.conf.sample and documentation was updated but
logging_collector short description was not updated in guc.c (which is
guc_table.c now).
Attached is the patch for this minor fix.[1] /messages/by-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Thanks for the report. This one is on me, will fix.
--
Michael
On Fri, Jan 31, 2025 at 2:43 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Jan 31, 2025 at 06:09:48PM +0900, Umar Hayat wrote:
Enabling logging_collector is required for json log. During jsonlog
implementation [1], log destination and logging_collector comments in
postgresql.conf.sample and documentation was updated but
logging_collector short description was not updated in guc.c (which is
guc_table.c now).
Attached is the patch for this minor fix.[1] /messages/by-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Thanks for the report. This one is on me, will fix.
--
Michael
- gettext_noop("Start a subprocess to capture stderr output and/or
csvlogs into log files."),
+ gettext_noop("Start a subprocess to capture stderr, csvlog and/or
jsonlog into log files."),Did you remove the word "output" intentionally? It is related to
stderr and not jsonlog. I think, stderr usually refers to a file or a
file descriptor hence we added word "output" there. I wouldn't touch
it.
--
Best Wishes,
Ashutosh Bapat
On Fri, 31 Jan 2025 at 20:41, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
On Fri, Jan 31, 2025 at 2:43 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Jan 31, 2025 at 06:09:48PM +0900, Umar Hayat wrote:
Enabling logging_collector is required for json log. During jsonlog
implementation [1], log destination and logging_collector comments in
postgresql.conf.sample and documentation was updated but
logging_collector short description was not updated in guc.c (which is
guc_table.c now).
Attached is the patch for this minor fix.[1] /messages/by-id/CAH7T-aqswBM6JWe4pDehi1uOiufqe06DJWaU5=X7dDLyqUExHg@mail.gmail.com
Thanks for the report. This one is on me, will fix.
--
Michael- gettext_noop("Start a subprocess to capture stderr output and/or csvlogs into log files."), + gettext_noop("Start a subprocess to capture stderr, csvlog and/or jsonlog into log files."),Did you remove the word "output" intentionally? It is related to
stderr and not jsonlog. I think, stderr usually refers to a file or a
file descriptor hence we added word "output" there. I wouldn't touch
it.
Yes, It was omitted intentionally to match the wording in
postgresql.conf.sample and documentation of logging_collector.
--
Best Wishes,
Ashutosh Bapat
--
Umar Hayat
Bitnine (https://bitnine.net/)
Umar Hayat <postgresql.wizard@gmail.com> writes:
On Fri, 31 Jan 2025 at 20:41, Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:- gettext_noop("Start a subprocess to capture stderr output and/or csvlogs into log files."), + gettext_noop("Start a subprocess to capture stderr, csvlog and/or jsonlog into log files."),Did you remove the word "output" intentionally? It is related to
stderr and not jsonlog. I think, stderr usually refers to a file or a
file descriptor hence we added word "output" there. I wouldn't touch
it.
Yes, It was omitted intentionally to match the wording in
postgresql.conf.sample and documentation of logging_collector.
FWIW, it seems weird to me to use the word "output" with only one
of those terms. The proposed new wording is fine by me, or we
could do
"Start a subprocess to capture stderr, csvlog and/or jsonlog output
into log files."
I read that as 'stderr, csvlog and/or jsonlog' all being modifiers
for 'output', which is sensible grammar.
regards, tom lane
On Fri, Jan 31, 2025 at 10:19:29AM -0500, Tom Lane wrote:
FWIW, it seems weird to me to use the word "output" with only one
of those terms. The proposed new wording is fine by me, or we
could do"Start a subprocess to capture stderr, csvlog and/or jsonlog output
into log files."I read that as 'stderr, csvlog and/or jsonlog' all being modifiers
for 'output', which is sensible grammar.
FWIW, I'd choose for keeping things simpler and just remove the word
"output", keeping this list as all the possible values. Hence, the
patch is OK here:
- gettext_noop("Start a subprocess to capture stderr output and/or csvlogs into log files."),
+ gettext_noop("Start a subprocess to capture stderr, csvlog and/or jsonlog into log files."),
--
MichaelMichael Paquier <michael@paquier.xyz> writes:
FWIW, I'd choose for keeping things simpler and just remove the word "output", keeping this list as all the possible values. Hence, the patch is OK here: - gettext_noop("Start a subprocess to capture stderr output and/or csvlogs into log files."), + gettext_noop("Start a subprocess to capture stderr, csvlog and/or jsonlog into log files."),
Sure, fine by me.
regards, tom lane