-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Filebeat HAproxy Default log format added #8428
Filebeat HAproxy Default log format added #8428
Conversation
@@ -40,5 +40,23 @@ | |||
description: raw_request_line is the complete HTTP request line, including the method, request and HTTP version string. | |||
type: text | |||
|
|||
|
|||
- name: default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't namespace fields depending on the matching pattern, some fields can be common between them, like probably frontend_name
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TCP format PR is also open and waiting and I think I did a better naming there. Let's move on to finish and close this two and then I'll quickly open a new one to write proper naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After opening a new PR with both PRs, what prevents us on using a proper naming? I'd do it before merging.
"@timestamp": "2018-09-20T15:42:59.000Z", | ||
"fileset.module": "haproxy", | ||
"fileset.name": "log", | ||
"haproxy.client_ip": "127.0.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using local IPs can be a good trick to mitigate #8400 🙂 but also will avoid the presence of geoip fields, and I think that we want at least to test that they are present 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify this point, we may want to use a public IP here, what would be more similar to a real event. Using a public IP will fill the geoip data (using a local IP won't, and this is why I said that it wouldn't be then affected by #8400).
e50caaf
to
b02c8ca
Compare
This branch will need an update. |
b02c8ca
to
fd9e9d2
Compare
Continues here #8637 |
Refer to this Issue for more details elastic/integrations#3250
Integration tests are passing and I'm uploading after a
make update
and amake filebeat.test
but removing whatever is written in the HAProxy http expectedjson
that makes the tests fail