On Tue, Aug 31, 2021 at 8:43 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Aug 31, 2021 at 11:34:56AM -0400, Sehrope Sarkuni wrote: > The second commit adds a TAP test for log_destination "csvlog". This was > done to both confirm that the previous change didn't break anything and as > a skeleton for the test in the next commit.
Do you really need a rotation of the log files here? Wouldn't it be better to grab the position of the current log file with a fixed log file name, and then slurp the file from this position with your expected output? That would make the test faster, as well.
Yes, that was intentional. I used the log rotation tap test as a base and kept that piece in there so it verifies that the csv log files are actually rotated. Same for the json logs.
Rather than making elog.c larger, I think that we should try to split that into more files. Why not refactoring out the CSV part first? You could just call that csvlog.c, then create a new jsonlog.c for the meat of the patch.
That's a good idea. I'll try that out.
The list of fields is not up to date. At quick glance, you are missing: - backend type. - leader PID. - query ID. - Session start timestamp (?)