SCL parser for PostgreSQL csvlog #4586

bazsi · 2023-08-10T08:49:43Z

add a new parser to process CSV log formatted by PostgreSQL (https://www.postgresql.org/docs/current/runtime-config-logging.html). The CSV format is extracted into a set of name-value pairs.

scl/pgsql/pgsql.conf

bazsi · 2023-10-22T15:12:20Z

With #4680 this can be improved a lot by using csv-parser() to type up the extracted values.

OverOrion · 2023-11-03T16:05:29Z

As sometimes not every field is filled (e.g, starting PostgreSQL 15.4... log 's internal_query_pos field is "", because there is no running query at that time) I had to make sure that the csv-parser() was fault tolerant regarding typecasts as well (mapping "" to int). For that reason I added the on-error("silently-fallback-to-string") to the scl, which required adding LogTemplateOptions to the CSVParser class. I am not sure if that commit should be standalone PR or not, so I left it here for simplicity's sake. I am open other solutions to this problem as well.

kira-syslogng · 2023-11-03T16:23:39Z

Build FAILURE

alltilla · 2023-11-07T09:31:38Z

modules/csvparser/csvparser.c

+static gboolean
+_should_drop_message(CSVParser *self)
+{
+  return self->drop_invalid || (self->template_options.on_error & ON_ERROR_DROP_MESSAGE);


We could just change the drop-invalid() option to set self->template_options.on_error and remove the self->drop_invalid bool. This would clean up the code I think. Maybe we could even deprecate the usage of drop-invalid() in favor of on-error().

Absolutely, thanks, at the moment I just got rid of the drop_invalid field, I think a separate deprecation PR might be more suitable, what do you think?

We should deprecate it in this PR, I think. @bazsi @MrAnno what do you think?

Hmm there's another case for drop_invalid, where the number of parsed columns is less than the count of expected fields. So it's not just the type casting that can cause the csv-parser() to drop an input.

I also find it suspicious at first sight that a template on-error flag is affecting csv-parser() type casting, which is not using templates?

I may have missed a beat on the patch though.

modules/csvparser/csvparser.c

bazsi · 2023-11-11T07:31:55Z

modules/csvparser/csvparser.c

-          GError *error = NULL;
-
-          if (self->drop_invalid && !type_cast_validate(current_value, current_column->type, &error))
+          if (!_process_column(self, scanner, msg, column_l, key_scratch, _key_formatter))


we should not pass the GList iterator here, passing current_column seems better.

the _key_formatter could be also pushed into the _process_column() function, removing yet another argument.

bazsi · 2023-11-11T09:04:10Z

As sometimes not every field is filled (e.g, starting PostgreSQL 15.4... log 's internal_query_pos field is "", because there is no running query at that time) I had to make sure that the csv-parser() was fault tolerant regarding typecasts as well (mapping "" to int). For that reason I added the on-error("silently-fallback-to-string") to the scl, which required adding LogTemplateOptions to the CSVParser class. I am not sure if that commit should be standalone PR or not, so I left it here for simplicity's sake. I am open other solutions to this problem as well.

I think adding LogTemplateOptions as a whole is a mistake. csv-parser doesn't use templates at all, and type hinting is a lower level concept. I understand that the on-error() is now coupled with template options but we should decouple that instead and only use this one option instead of the whole lot.

Also I think the user visible option is named incorrectly. It should be on-type-error() or something like that. When used as a global option it's really not intuitive what it does.

OverOrion · 2023-11-13T08:51:33Z

Thanks Bazsi! I am more than happy to do the decoupling (separate PR for sure), what I am unsure about is to where it should go once decoupled. Do you have something in mind?

kira-syslogng · 2023-11-13T13:04:44Z

Build FAILURE

kira-syslogng · 2023-11-13T13:59:22Z

Build FAILURE

modules/csvparser/csvparser.c

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

github-actions · 2023-11-24T10:17:34Z

This Pull Request introduces config grammar changes

syslog-ng/c4c127aeaa3258ca8a90b58a684bf764e33d8f57 -> bazsi/scl-pgsql-parser

--- a/parser
+++ b/parser

 csv-parser(
+    on-type-error(<string>)
 )

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

…ack options Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

bazsi force-pushed the scl-pgsql-parser branch 2 times, most recently from af83c5c to 072dd64 Compare August 12, 2023 04:53

alltilla reviewed Aug 21, 2023

View reviewed changes

scl/pgsql/pgsql.conf Outdated Show resolved Hide resolved

scl/pgsql/pgsql.conf Show resolved Hide resolved

scl/pgsql/pgsql.conf Outdated Show resolved Hide resolved

OverOrion self-requested a review September 19, 2023 09:08

alltilla added this to the syslog-ng 4.4 milestone Sep 19, 2023

OverOrion requested a review from alltilla September 21, 2023 13:57

OverOrion force-pushed the scl-pgsql-parser branch 9 times, most recently from bde74f2 to 8173514 Compare September 22, 2023 15:18

alltilla reviewed Sep 25, 2023

View reviewed changes

scl/pgsql/pgsql.conf Show resolved Hide resolved

scl/pgsql/pgsql.conf Outdated Show resolved Hide resolved

alltilla removed this from the syslog-ng 4.4 milestone Sep 25, 2023

MrAnno added this to the syslog-ng 4.5 milestone Oct 2, 2023

OverOrion force-pushed the scl-pgsql-parser branch from 8173514 to 47e1886 Compare November 3, 2023 15:59

OverOrion force-pushed the scl-pgsql-parser branch from 47e1886 to eda8da6 Compare November 3, 2023 16:07

OverOrion force-pushed the scl-pgsql-parser branch 6 times, most recently from 05e9413 to 299fac3 Compare November 6, 2023 13:27

OverOrion requested a review from alltilla November 6, 2023 14:30

alltilla reviewed Nov 8, 2023

View reviewed changes

OverOrion force-pushed the scl-pgsql-parser branch from 299fac3 to 50c749f Compare November 8, 2023 10:51

alltilla reviewed Nov 9, 2023

View reviewed changes

modules/csvparser/csvparser.c Outdated Show resolved Hide resolved

modules/csvparser/csvparser.c Outdated Show resolved Hide resolved

modules/csvparser/csvparser.c Show resolved Hide resolved

OverOrion force-pushed the scl-pgsql-parser branch from 50c749f to f1322d6 Compare November 10, 2023 12:18

bazsi commented Nov 11, 2023

View reviewed changes

OverOrion force-pushed the scl-pgsql-parser branch from f1322d6 to 57427eb Compare November 13, 2023 12:57

OverOrion force-pushed the scl-pgsql-parser branch from 57427eb to cc5b0ee Compare November 13, 2023 13:52

OverOrion force-pushed the scl-pgsql-parser branch 2 times, most recently from 6f68be1 to d315518 Compare November 13, 2023 14:06

alltilla reviewed Nov 24, 2023

View reviewed changes

modules/csvparser/csvparser.c Outdated Show resolved Hide resolved

modules/csvparser/csvparser.c Show resolved Hide resolved

OverOrion added 2 commits November 24, 2023 11:05

lib: decouple on_error from LogTemplateOptions

450d9b7

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

csv-parser: refactor column processing into function

2adda35

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

OverOrion force-pushed the scl-pgsql-parser branch from d315518 to 49fda7f Compare November 24, 2023 10:13

OverOrion and others added 5 commits November 24, 2023 13:47

csv-parser: add on-type-error()

c3117ba

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

csv-parser: refactor _iterate_columns to use various on-error() failb…

447e06a

…ack options Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

scl/pgsql: add support for PostgreSQL csvlog parsing

2abe385

Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com>

scl/pgsql: enhance PostgreSQL csvlog integration, add some tests

9c88938

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

scl/pgsql: add test case with extra fields

96b82cf

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>

OverOrion force-pushed the scl-pgsql-parser branch from 49fda7f to 96b82cf Compare November 24, 2023 12:49

alltilla approved these changes Nov 24, 2023

View reviewed changes

alltilla merged commit e34697d into syslog-ng:master Nov 24, 2023

SCL parser for PostgreSQL csvlog #4586

SCL parser for PostgreSQL csvlog #4586

Uh oh!

Conversation

bazsi commented Aug 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bazsi commented Oct 22, 2023

Uh oh!

OverOrion commented Nov 3, 2023

Uh oh!

kira-syslogng commented Nov 3, 2023

Uh oh!

alltilla Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

OverOrion Nov 8, 2023

Choose a reason for hiding this comment

Uh oh!

alltilla Nov 9, 2023

Choose a reason for hiding this comment

Uh oh!

bazsi Nov 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bazsi Nov 11, 2023

Choose a reason for hiding this comment

Uh oh!

bazsi Nov 11, 2023

Choose a reason for hiding this comment

Uh oh!

bazsi commented Nov 11, 2023

Uh oh!

OverOrion commented Nov 13, 2023

Uh oh!

kira-syslogng commented Nov 13, 2023

Uh oh!

kira-syslogng commented Nov 13, 2023

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This Pull Request introduces config grammar changes

Uh oh!

Uh oh!

bazsi commented Aug 10, 2023 •

edited

Loading

github-actions bot commented Nov 24, 2023 •

edited

Loading