support for vhost:port in common/combined log formats #2688

nambrosch · 2019-04-19T13:55:34Z

This patch will add support for vhost & port in common & combined apache logs as discussed @ #2670. Here is that format -

foo.com:443 1.2.3.4 - - [15/Apr/2019:14:30:16 -0400] "GET /bar.html HTTP/2.0" 500 - "https://foo.com/referer.html" "Mozilla/5.0 ..."

Below is my use case that I'm using on a test server, using the ${.apache.vhost} macro to create a directory then logging using combined format -

apache:

LogFormat "%v:%p %h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vcombined
CustomLog "|/usr/bin/logger -S 65536 -t apache -p info -d -n 127.0.0.1 -P 5141" vcombined

syslog-ng:

source s_apache_access {
  network(ip(127.0.0.1) port(5141) transport(udp) flags(syslog-protocol));
};

destination d_apache_access {
  file (
    "/var/log/httpd/${.apache.vhost}/access_log_${YEAR}${MONTH}${DAY}"
    template("${.apache.clientip} ${.apache.ident} ${.apache.auth} [${.apache.timestamp}] \"${.apache.rawrequest}\" ${.apache.response} ${.apache.bytes} \"${.apache.referrer}\" \"${.apache.agent}\"\n")
  );
};

log {
  source(s_apache_access);
  parser { apache-accesslog-parser(); };
  destination(d_apache_access);
};

kira-syslogng · 2019-04-19T13:55:37Z

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
"add to whitelist" add author of a Pull Request to whitelist (globally, be careful, it means this user can trigger kira for any PR)
do nothing -> CI won't start)

kira-syslogng · 2019-04-19T13:55:37Z

This user does not have permission to start the build. Can one of the admins verify this patch and start the build?
(admin: you have the next options (make sure you checked the code):
"ok to test" to accept this pull request (and further changes) for testing
"test this please" for a one time test run
"add to whitelist" add author of a Pull Request to whitelist (globally, be careful, it means this user can trigger kira for any PR)
do nothing -> CI won't start)

Kokan · 2019-04-19T14:04:01Z

@kira-syslogng ok to test

kira-syslogng · 2019-04-19T14:26:43Z

Build SUCCESS

bazsi

This looks good to me. There are some white space only changes, maybe tab to space conversion, that could perhaps be removed, but it's ok this way, as that's only a few lines.

I didn't exlicitly check that this is indeed the Apache combined format but i trust you it is.

nambrosch · 2019-04-20T14:50:21Z

i used four spaces for indenting instead of tabs, what is your convention?

bazsi · 2019-04-21T06:04:10Z

4 spaces is much better than a tab. So this change is welcome.

…

On Sat, Apr 20, 2019, 16:50 nambrosch ***@***.*** wrote: i used four spaces for indenting instead of tabs, what is your convention? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2688 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFOK5SS4GU4XBF5GTDU7BDPRMUS5ANCNFSM4HHEHWWA> .

Signed-off-by: Nik Ambrosch <nik@ambrosch.com>

kira-syslogng · 2019-04-21T06:40:11Z

Build SUCCESS

furiel · 2019-04-21T13:55:44Z

The patch itself looks good to me. I just have one question for clarification.

Would it be possible to add support for the unofficial vcommon & vcombined formats to the apache parser?

Could you explain what do you mean by unofficial format? I tried to investigate, and here is what I found. According to the documentation, the official combined log format is

LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" combined

which is different than the introduced version.

I fired up an apache server on centos:7 in docker, and they use the official format above.
However, with ubuntu:bionic, they go with your version, except they call it vhost_combined instead of vcombined.

So what is happening here is that some distributions deviate from the official log format? And the goal here is to support those distributions too? Out of curiosity, which distribution do you use?

Btw, I do not have problem with that. I like the idea that syslog-ng should work out of the box, and support the deviations of major distributions.

furiel · 2019-04-21T14:03:49Z

scl/apache/apache.conf


+        # if traditional log format


From clean code point of view, instead of this comment, you could give a name to these parser blocks. Analogously to

block parser apache-accesslog-parser(prefix(".apache.") template("${MESSAGE}"))

you could extract these branches into apache-access-log-traditional-part and apache-access-log-combined-part. That would make the scl a little more readable. As maybe there are other distributions with different format, it would be easier to extend later on.

I wasn't aware that ubuntu has this LogFormat already available, thanks for pointing it out. Fedora/CentOS does not include this format. It might make more sense to reference the name as vhost_combined instead of vcombined.

Shifting from 1 -> 3 blocks is a good idea, I'll explore that tonight.

Using one block for each log format makes extending easier. Signed-off-by: Nik Ambrosch <nik@ambrosch.com>

nambrosch

@furiel split the blocks per your suggestion.

kira-syslogng · 2019-04-22T18:21:41Z

Build SUCCESS

bazsi · 2019-04-23T09:44:30Z

Hi, This looks better indeed @furiel. Another layer of refinement occured to me, but instead of describing it in email, here's the patch. I am sure @nambrosch is a bit fed up with all the improvements :) can you still apply this to your solution? Basically I've moved the filter to apache-accesslog-parser-vhost(), which will drop all messages that don't match the required format. I've also fixed a bug, so that template() option to apache-accesslog-parser-combined() actually uses that value, instead of hard-coding $1. I didn't test this this round, so it'd be appreciated if you could doublecheck that it still works for you. ``` diff --git a/scl/apache/apache.conf b/scl/apache/apache.conf index 440e64269..9046f2452 100644 --- a/scl/apache/apache.conf +++ b/scl/apache/apache.conf @@ -25,6 +25,10 @@ # LogFormat "%v:%p %h %l %u %t \"%r\" %>s %b" vhost_common block parser apache-accesslog-parser-vhost(prefix() template()) { channel { + rewrite { + set("`template`" value("1")); + }; + filter { match("^[A-Za-z0-9\-\._]+:[0-9]+ " value("1")); }; parser { csv-parser( dialect(escape-double-char) @@ -59,7 +63,7 @@ block parser apache-accesslog-parser-combined(prefix() template()) { dialect(escape-double-char) flags(strip-whitespace) delimiters(" ") - template($1) + template(`template`) quote-pairs('""[]') columns("clientip", "ident", "auth", "timestamp", "rawrequest", "response", @@ -69,14 +73,12 @@ block parser apache-accesslog-parser-combined(prefix() template()) { }; block parser apache-accesslog-parser(prefix(".apache.") template("${MESSAGE}")) { + # parse into a logstash-like schema + # https://github.com/elastic/logstash/blob/v1.4.2/patterns/grok-patterns#L90 channel { - rewrite { - set("`template`" value("1")); - }; # parser for formats including vhost:port if { - filter { match("^[A-Za-z0-9\-\._]+:[0-9]+ " value("1")); }; parser { apache-accesslog-parser-vhost(prefix(`prefix`) template("$1")); }; # parser for standard formats @@ -84,7 +86,8 @@ block parser apache-accesslog-parser(prefix(".apache.") template("${MESSAGE}")) parser { apache-accesslog-parser-combined(prefix(`prefix`) template("$1")); }; }; - # parser for all formats + # mungle values to match Kibana/elastic schema and common to all + # supported formats. parser { csv-parser( prefix(`prefix`) ```

…

On Mon, Apr 22, 2019 at 8:21 PM kira-syslogng ***@***.***> wrote: Build SUCCESS — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2688 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFOK5Q3LIEYE6RN62HYKV3PRX63NANCNFSM4HHEHWWA> .

bazsi · 2019-04-23T11:45:15Z

Here's that last branch as a git branch, feel free to integrate it into yours.

https://github.com/bazsi/syslog-ng/tree/apache-access-log-parser-improvements

nambrosch · 2019-04-23T17:46:53Z

@bazsi i'm still using $1 two more times in block parser apache-accesslog-parser so removing set("`template`" value("1")); causes it to break.

moving the filter and changing $1 -> template in block parser apache-accesslog-parser-combined is logical though.

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

bazsi · 2019-04-28T11:57:15Z

I have just added a few more patches on top of the initial patch and sent a PR to @nambrosch With those patches this has my approval and I think it would be great if we could merge them by 3.21

Alternatively, we could my branch directly, it contains both this patch and my improvements
https://github.com/bazsi/syslog-ng/tree/apache-access-log-parser-improvements

nambrosch · 2019-04-28T23:44:38Z

@bazsi Thanks, I'm happy with those changes - confirmed it works on a fedora host and merged the PR.

kira-syslogng · 2019-04-29T00:04:28Z

Build SUCCESS

bazsi · 2019-04-29T04:39:59Z

Btw, I implemented a template () option for match () to make these use cases easier to implement, but this can be merged as is, we could eventually make use of that in this parser as well. I hope I can publish that branch once I finished the tests. Bazsi

…

On Mon, Apr 29, 2019, 02:04 kira-syslogng ***@***.*** wrote: Build SUCCESS — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2688 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFOK5WUT2ZMZEMM24EYZQ3PSY3Q3ANCNFSM4HHEHWWA> .

bazsi

Approved. It would be nice if you could get rid off the merge commit within the branch, just make sure its a set of linear patches.

nambrosch · 2019-04-29T05:02:17Z

Approved. It would be nice if you could get rid off the merge commit within the branch, just make sure its a set of linear patches.

Thanks. Is it possible to remove that? I think trying to revert the merge commit in nambrosch/syslog-ng creates an additional PR, no?

Kokan · 2019-04-29T05:52:51Z

@nambrosch you should be fine to hard reset to @bazsi patch and force push it. (it should not create new PR)

Something like this: https://asciinema.org/a/zRtDlGZjId1VheJeiMoqP4qr9

nambrosch · 2019-04-29T06:05:49Z

ah ha, perfect, all set.

kira-syslogng · 2019-04-29T06:25:15Z

Build SUCCESS

add support for vhost:port in common/combined log formats

723d02a

bazsi previously approved these changes Apr 20, 2019

View reviewed changes

scl/apache: remove whitespace

a9e613e

Signed-off-by: Nik Ambrosch <nik@ambrosch.com>

nambrosch dismissed bazsi’s stale review via a9e613e April 21, 2019 06:17

furiel reviewed Apr 21, 2019

View reviewed changes

scl/apache: split into multiple blocks

5b5aa7f

Using one block for each log format makes extending easier. Signed-off-by: Nik Ambrosch <nik@ambrosch.com>

nambrosch commented Apr 22, 2019

View reviewed changes

gaborznagy added the community-driven-merge label Apr 24, 2019

bazsi added 2 commits April 28, 2019 13:50

apache: move the setting of $1 into apache-accesslog-parser-vhost()

5971628

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

apache: document all access.log formats supported

16978f8

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

bazsi previously approved these changes Apr 29, 2019

View reviewed changes

nambrosch dismissed bazsi’s stale review via 16978f8 April 29, 2019 06:02

nambrosch force-pushed the apache-scl-vhost branch from bf2efad to 16978f8 Compare April 29, 2019 06:02

bazsi approved these changes Apr 29, 2019

View reviewed changes

Kokan approved these changes Apr 29, 2019

View reviewed changes

Kokan merged commit 7e3b683 into syslog-ng:master Apr 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for vhost:port in common/combined log formats #2688

support for vhost:port in common/combined log formats #2688

nambrosch commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

Kokan commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

bazsi left a comment

nambrosch commented Apr 20, 2019

bazsi commented Apr 21, 2019 via email

kira-syslogng commented Apr 21, 2019

furiel commented Apr 21, 2019

furiel Apr 21, 2019

nambrosch Apr 21, 2019

nambrosch left a comment

kira-syslogng commented Apr 22, 2019

bazsi commented Apr 23, 2019 via email

bazsi commented Apr 23, 2019 •

edited

Loading

nambrosch commented Apr 23, 2019 •

edited

Loading

bazsi commented Apr 28, 2019

nambrosch commented Apr 28, 2019

kira-syslogng commented Apr 29, 2019

bazsi commented Apr 29, 2019 via email

bazsi left a comment

nambrosch commented Apr 29, 2019

Kokan commented Apr 29, 2019

nambrosch commented Apr 29, 2019

kira-syslogng commented Apr 29, 2019

support for vhost:port in common/combined log formats #2688

support for vhost:port in common/combined log formats #2688

Conversation

nambrosch commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

Kokan commented Apr 19, 2019

kira-syslogng commented Apr 19, 2019

bazsi left a comment

Choose a reason for hiding this comment

nambrosch commented Apr 20, 2019

bazsi commented Apr 21, 2019 via email

kira-syslogng commented Apr 21, 2019

furiel commented Apr 21, 2019

furiel Apr 21, 2019

Choose a reason for hiding this comment

nambrosch Apr 21, 2019

Choose a reason for hiding this comment

nambrosch left a comment

Choose a reason for hiding this comment

kira-syslogng commented Apr 22, 2019

bazsi commented Apr 23, 2019 via email

bazsi commented Apr 23, 2019 • edited Loading

nambrosch commented Apr 23, 2019 • edited Loading

bazsi commented Apr 28, 2019

nambrosch commented Apr 28, 2019

kira-syslogng commented Apr 29, 2019

bazsi commented Apr 29, 2019 via email

bazsi left a comment

Choose a reason for hiding this comment

nambrosch commented Apr 29, 2019

Kokan commented Apr 29, 2019

nambrosch commented Apr 29, 2019

kira-syslogng commented Apr 29, 2019

bazsi commented Apr 23, 2019 •

edited

Loading

nambrosch commented Apr 23, 2019 •

edited

Loading