Skip to content

Conversation

Kokan
Copy link
Collaborator

@Kokan Kokan commented Dec 20, 2019

Opposed to parsers that by default uses MESSAGE and that could be further specialised with template() options, the geoip2 always uses template. That is enforced with the grammar rule always requiring a template in a form of string: geoip2("template-format").

The PR includes the following changes:

  • Extend the geoip2 grammer with general parse_opt rules (currently only includes template()
  • In init phase enforces the existence of template
  • Changed UT according to always using template() opposed to using MESSAGE field, that works for any parser, but never actually used in case of geoip2

As a results both are possible configuration option:

 geoip2("$HOST");
 geoip2(template("$HOST"));

 # geoip2();  # this is still not valid

Right now only `template()` is included with the parser_opt rule, but this also allows any other future enhancements.

Signed-off-by: Kokan <[email protected]>
After this patch providing a format string after opening bracket became optional, and the following are equal:
```
geoip2("format-string");
geoip2(format-string);
geoip2(template("format-string"));
```

Signed-off-by: Kokan <[email protected]>
@Kokan Kokan changed the title Geoip template opt geoip2: extend with template option Dec 20, 2019
@kira-syslogng
Copy link
Contributor

Build SUCCESS

Copy link
Collaborator

@furiel furiel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. It should have been implemented originally with the template option.

@Kokan
Copy link
Collaborator Author

Kokan commented Dec 30, 2019

It should have been implemented originally with the template option.

@furiel do you think marking geoip2("format"); version deprecated would be okay ? I was thinking about that, but in the end for my original goal it was not required.

Copy link
Collaborator

@alltilla alltilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome :)

@alltilla alltilla merged commit 40689b6 into syslog-ng:master Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants