[pgpool-hackers: 4567] Re: [PATCH] Feature: CSV Logging support

[Tatsuo Ishii]

> Hi Hackers,
> Recently csv logging support was requested [1]. IMO it's a reasonable
> request and we already have basic infrastructure to extend it for CSV
> logging for log_destination.
> 
> Attached patch is the first draft and it covers changes in code,
> documentation and sample file.

Hi Umar,

> Hi Hackers,
> Recently csv logging support was requested [1]. IMO it's a reasonable
> request and we already have basic infrastructure to extend it for CSV
> logging for log_destination.
> 
> Attached patch is the first draft and it covers changes in code,
> documentation and sample file.
> 
> Example Records:
> "2025-01-15 10:35:39","main",97058,LOG,21,"[No Connection]","[No
> Connection]","pgpool-II successfully started. version 4.6devel
> (chirikoboshi)",,,
> 
> "2025-01-15 10:35:39","main",97058,WARNING,13,"[No Connection]","[No
> Connection]","failed to create INET domain socket","bind on socket
> failed with error ""Cannot assign requested address""",,
> 
> "2025-01-15 10:36:36","psql",97072,LOG,1,"postgres","postgres","pool_send_and_wait:
> Error or notice message from backend: DB node id: 0 backend pid: 97099
> statement: ""select a;"" message: ""column ""a"" does not exist""",,,
> 
> Following fields are logged in CSV record in order:
> datetime, application name, process id, error severity, line number,
> user name, database name, error message, error details, error hint [,
> [function] & filename + line number] (if PGERROR_VERBOSE)
> 
> Please let me know your opinion about this and also about the fields
> covered in csv records and their order.

Thank you for the patch. I think this feature is a good thing.

Some comments for the patch:

(1) Doesn't the feature needs to turn on logging_collector? If so, it
should be noted in the docs.

(2) regarding the CSV record format

- I think we should follow the order of PostgreSQL's csvlog unless
  there's good reason to not do so.

- The datetime should be in millisecond like PostgreSQL.

- It would be nice if we could include backend pids.

These are just my thought.

> Please let me know your opinion about this and also about the fields
> covered in csv records and their order.

Probably we need more discussion on this because once we decide it,
it's very hard to change it as it is hard coded.

(3) coding style

- We follow the same indentation rule as PostgreSQL[1]. To enforce
  this, it is convenient to use PostgreSQL's pgindent tool. I attach
  csvlog.c which is pgindent applied for a reference.

- We follow the PostgreSQL's rule regarding variable
  declarations[2]. That is, we do not use "intermingled variable
  declarations". i.e. variable "now" should be declared earlier in the
  function (usually in the begging of a function or the most inner
  block which the code belongs).

	/* timestamp with milliseconds */
	time_t		now = time(NULL);

[1] https://www.postgresql.org/docs/current/source-format.html
[2] https://www.postgresql.org/docs/current/source-conventions.html

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: csvlog.c
Type: application/octet-stream
Size: 4248 bytes
Desc: not available
URL: <http://www.pgpool.net/pipermail/pgpool-hackers/attachments/20250128/c491974c/attachment.obj>