Add support for http hostname in nginx filebeat module #14505
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsoriano
added
module
review
Filebeat
jsoriano
commented
Nov 13, 2019
| "service.type": "nginx", | ||
| "source.address": "localhost, localhost", | ||
| "source.address": "localhost", |
There was a problem hiding this comment.
This was being incorrectly parsed, this is also fixed as part of this PR.
jsoriano
commented
Nov 13, 2019
| "lang": "painless", | ||
| "source": "boolean isPrivate(def dot, def ip) { try { StringTokenizer tok = new StringTokenizer(ip, dot); int firstByte = Integer.parseInt(tok.nextToken()); int secondByte = Integer.parseInt(tok.nextToken()); if (firstByte == 10) { return true; } if (firstByte == 192 && secondByte == 168) { return true; } if (firstByte == 172 && secondByte >= 16 && secondByte <= 31) { return true; } if (firstByte == 127) { return true; } return false; } catch (Exception e) { return false; } } try { ctx.source.ip = null; if (ctx.nginx.access.remote_ip_list == null) { return; } def found = false; for (def item : ctx.nginx.access.remote_ip_list) { if (!isPrivate(params.dot, item)) { ctx.source.ip = item; found = true; break; } } if (!found) { ctx.source.ip = ctx.nginx.access.remote_ip_list[0]; }} catch (Exception e) { ctx.source.ip = null; }", | ||
| "source": "boolean isPrivate(def dot, def ip) { try { StringTokenizer tok = new StringTokenizer(ip, dot); int firstByte = Integer.parseInt(tok.nextToken()); int secondByte = Integer.parseInt(tok.nextToken()); if (firstByte == 10) { return true; } if (firstByte == 192 && secondByte == 168) { return true; } if (firstByte == 172 && secondByte >= 16 && secondByte <= 31) { return true; } if (firstByte == 127) { return true; } return false; } catch (Exception e) { return false; } } try { ctx.source.address = null; if (ctx.nginx.access.remote_ip_list == null) { return; } def found = false; for (def item : ctx.nginx.access.remote_ip_list) { if (!isPrivate(params.dot, item)) { ctx.source.address = item; found = true; break; } } if (!found) { ctx.source.address = ctx.nginx.access.remote_ip_list[0]; }} catch (Exception e) { ctx.source.address = null; }", |
There was a problem hiding this comment.
Change in this script is a replacement of source.ip with source.address.
jsoriano
added
needs_backport
|
LGTM |
exekias
reviewed
Nov 14, 2019
| @@ -286,8 +289,12 @@ | |||
| "http.version": "1.1", | |||
| "input.type": "log", | |||
| "log.offset": 1398, | |||
| "nginx.access.remote_ip_list": [ | |||
| "localhost", | |||
| "localhost" | |||
There was a problem hiding this comment.
why is localhost repeated here?
There was a problem hiding this comment.
It appears twice in the example log line:
localhost, localhost - - [29/May/2017:19:02:48 +0000] "GET /test2 HTTP/1.1" 200 612 "-" "Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120716 Firefox/15.0a2" "-"
Not sure how this log was generated.
exekias
approved these changes
Nov 14, 2019
jsoriano
added a commit
to jsoriano/beats
that referenced
this pull request
Nov 20, 2019
It is common to modify the default log format in nginx to include the http host used by the client to make the logged request. This is similar to what we already did for Apache in elastic#12778. It also fixes an issue found on source address parsing when source addresses are not IPs. For that, the pipeline works with addresses as strings instead of IPs, and it only tries to convert to IP to finally store the it in `source.ip` if it parses as an IP. Co-authored-by: poma <Semenov.Roman@mail.ru> (cherry picked from commit 1967bc9)
jsoriano
added
test-plan
jsoriano
added a commit
that referenced
this pull request
Nov 26, 2019
…ebeat module (#14654) It is common to modify the default log format in nginx to include the http host used by the client to make the logged request. This is similar to what we already did for Apache in #12778. It also fixes an issue found on source address parsing when source addresses are not IPs. For that, the pipeline works with addresses as strings instead of IPs, and it only tries to convert to IP to finally store the it in `source.ip` if it parses as an IP. (cherry picked from commit 1967bc9) Co-authored-by: poma <Semenov.Roman@mail.ru>
|
is it available on 7.x build? |
It is available starting on 7.6.0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is common to modify the default log format in nginx to include
the http host used by the client to make the logged request.
This continues with #13223 to add support for that, and is
similar to what we already did for Apache in #12778.
Fixes #13223.
It also fixes an issue found on source address parsing when
source addresses are not IPs. For that, the pipeline works
with addresses as strings instead of IPs, and it only tries to
convert to IP to finally store the it in
source.ipif it parsesas an IP.
Co-authored-by: poma Semenov.Roman@mail.ru