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

Allow - in Apache access log byte count #3863

Merged
merged 2 commits into from
Mar 31, 2017

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Mar 30, 2017

Allows a - instead of a number for the byte sent part of the Apache access log, as described in http://httpd.apache.org/docs/current/mod/mod_log_config.html

In contrast, Nginx will always output 0 - so no need to change anything there.

There are a few more pieces in the Grok pattern that are not quite in line with what Apache can produce. The Elasticsearch grok processor inherited a list of patterns from Logstash. One of them being COMBINEDAPACHELOG, we might want to mirror that in Filebeat. I guess that might be a more intrusive change, while this PR as-is can do no harm and should be fully backward compatible.

@cwurm
Copy link
Contributor Author

cwurm commented Mar 30, 2017

This addresses #3833.

@exekias
Copy link
Contributor

exekias commented Mar 31, 2017

jenkins, retest it please

@exekias
Copy link
Contributor

exekias commented Mar 31, 2017

Could you please add CHANGELOG entry?

@cwurm
Copy link
Contributor Author

cwurm commented Mar 31, 2017

@exekias I guess, though what should it look like? Which version(s)? Most recent release is 5.3.0, and the CHANGELOG here on master doesn't even cover that.

@exekias
Copy link
Contributor

exekias commented Mar 31, 2017

Just add it in HEAD section, we will manage that in the backport

@exekias exekias added the needs_backport PR is waiting to be backported to other branches. label Mar 31, 2017
@cwurm
Copy link
Contributor Author

cwurm commented Mar 31, 2017

@exekias done

@tsg
Copy link
Contributor

tsg commented Mar 31, 2017

Thanks for the fix @cwurm. I didn't use the COMBINEDAPACHELOG because I want control over the names of the resulting fields. Perhaps another option would have been to do renames afterwards, not sure.

@exekias exekias merged commit a083077 into elastic:master Mar 31, 2017
@cwurm cwurm deleted the cwurm-access-grok branch March 31, 2017 14:09
@cwurm
Copy link
Contributor Author

cwurm commented Mar 31, 2017

@tsg We could take the grok pattern behind COMBINEDAPACHELOG and replace the variable names. Except for some for which we don't have a field Beats - but that's probably fine, as almost nobody uses ident anyway for example.

@tsg tsg added v5.3.1 and removed v5.3.0 labels Apr 4, 2017
tsg pushed a commit to tsg/beats that referenced this pull request Apr 4, 2017
* Allow  in Apache access log byte count

(cherry picked from commit a083077)
@tsg tsg removed the needs_backport PR is waiting to be backported to other branches. label Apr 4, 2017
monicasarbu pushed a commit that referenced this pull request Apr 4, 2017
* Allow  in Apache access log byte count

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

Successfully merging this pull request may close these issues.

5 participants