Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change grok pattern to fetch correct IP from X-Forwarded-For list #4351

Merged
merged 3 commits into from
May 29, 2017

Conversation

sepal
Copy link
Contributor

@sepal sepal commented May 18, 2017

This PR changes the grok pattern for the nginx access logs ingest file in a way, that it retrieves the correct IP if the X-Forwarded-For header was logged into instead of the remote_addr variable.

The X-Forwarded-For header is a non standard header which creates a lists of IPs through which proxies the request has passed as well as the original clients IP and looks basically like this:

X-Forwarded-For: client1, proxy1, proxy2

which results in a log line like this:

192.228.32.190, 108.162.246.21, 127.0.0.1 - - [15/May/2017:12:16:27 +0200] "GET /jobs/24237/it-back-end HTTP/1.1" 301 5 "-" "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"

The new pattern retrieves the first IP, which is the important one, and matches non or all succeeding IPs that are concatinated with a comma and a space.

@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@exekias
Copy link
Contributor

exekias commented May 18, 2017

jenkins, test it

@exekias
Copy link
Contributor

exekias commented May 18, 2017

LGTM

@ruflin
Copy link
Contributor

ruflin commented May 19, 2017

LGTM. Could you add a changelog entry? I would put it under "Added" as this is not really a bugfix but a new feature.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@sepal
Copy link
Contributor Author

sepal commented May 22, 2017

I added the change to the changelog as requested.

@tsg
Copy link
Contributor

tsg commented May 23, 2017

@sepal Can you add a test case (test.log and test.log-expected.json) by the model we have in apache2: test.log and test.log-expected.json. The tests will automatically pick it up and check that it all works fine. Happy to also do that myself in a follow up PR if you don't have the time.

@sepal
Copy link
Contributor Author

sepal commented May 23, 2017

Yes I can, I'll look into that tomorrow or on thursday.

@ruflin
Copy link
Contributor

ruflin commented May 26, 2017

jenkins, test it

@tsg tsg merged commit b6194b4 into elastic:master May 29, 2017
@tsg tsg added needs_backport PR is waiting to be backported to other branches. v5.5.0 labels May 29, 2017
@sepal sepal deleted the nginx_fetch_ip_proxy branch May 29, 2017 14:13
tsg pushed a commit to tsg/beats that referenced this pull request Jul 19, 2017
…astic#4351)

* Change grok pattern to fetch correct IP from X-Forwarded-For list.

* Document change to the nginx module in the changelog.

* Add tests for nginx access log.

(cherry picked from commit b6194b4)
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Jul 19, 2017
andrewkroh pushed a commit that referenced this pull request Jul 19, 2017
)

* Change grok pattern to fetch correct IP from X-Forwarded-For list.

* Document change to the nginx module in the changelog.

* Add tests for nginx access log.

(cherry picked from commit b6194b4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants