Обсуждение: Re: jsonlog missing from logging_collector description

Поиск
Список
Период
Сортировка

Re: jsonlog missing from logging_collector description

От
Ashutosh Bapat
Дата:
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] https://www.postgresql.org/message-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



Re: jsonlog missing from logging_collector description

От
Umar Hayat
Дата:
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] https://www.postgresql.org/message-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/)



Re: jsonlog missing from logging_collector description

От
Tom Lane
Дата:
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



Re: jsonlog missing from logging_collector description

От
Tom Lane
Дата:
Michael 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