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.

Sign up for GitHub

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

Jump to bottom

Convert Filebeat haproxy.log to ECS #9117

Merged
merged 6 commits into from
Nov 22, 2018
Merged

Convert Filebeat haproxy.log to ECS #9117

merged 6 commits into from
Nov 22, 2018

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Nov 15, 2018 •
edited
Loading

Caveats

  • haproxy.client.ip is not renamed. If it's an IP, it's copied to source.ip,
    otherwise copied to source.domain.
  • haproxy.source is the source's hostname, but Filebeat is already populating
    host.hostname, so leaving as haproxy.source.
  • In the httplog format, the HTTP request is not being parsed/extracted.
    This PR does not change this fact, as it's a translation to ECS, not general improvements.

Renames

  • haproxy.client.port => source.port
  • haproxy.process_name => process.name
  • haproxy.pid => process.pid
  • haproxy.destination.ip => destination.ip
  • haproxy.destination.port => destination.port
  • haproxy.geoip.* => source.geo.*

TODO

  • Convert pid and port to int
  • Update ECS-migration.yml file
  • Changelog
  • Create field aliases

@ruflin ruflin mentioned this pull request Nov 15, 2018
Closed
@webmat webmat requested a review from ruflin November 15, 2018 21:31
@webmat webmat self-assigned this Nov 15, 2018
@webmat webmat added in progress Pull request is currently in progress. module Filebeat Filebeat ecs labels Nov 15, 2018
ruflin
ruflin reviewed Nov 19, 2018
View reviewed changes
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget the ecs-migration.yml file.

filebeat/module/haproxy/log/ingest/pipeline.json
@@ -40,9 +40,19 @@
}
},
{
"geoip": {
"grok": {
"field": "haproxy.client.ip",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this assume in some cases this is actually not an ip?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In HAProxy's case, they're too performance-conscious for this to be a name resolved from a reverse DNS query. However this can be populated with text if the connection is coming from a Unix socket LOL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unix socket info seems to be one more new twist on the IP field :-(

@webmat webmat removed the in progress Pull request is currently in progress. label Nov 20, 2018
@webmat
Copy link
Contributor Author

webmat commented Nov 20, 2018

@ruflin Ready for review. Make sure to check out caveats.

@webmat webmat added the review label Nov 20, 2018
@webmat webmat changed the title [WIP] Convert Filebeat haproxy.log to ECS Convert Filebeat haproxy.log to ECS Nov 20, 2018
@ruflin
Copy link
Member

ruflin commented Nov 21, 2018

For the Caveats:

  • client.ip: SGTM for now
  • user_agent: See other PR
  • haproxy.source: I think it's ok that we overwrite the host.* object.
  • +1 on not making additional changes

@webmat
Copy link
Contributor Author

webmat commented Nov 21, 2018

@ruflin Looking at the test data we have, I would not use haproxy.source to overwrite host.hostname, after all. I don't want to overwrite a perfectly good hostname with 127.0.0.1 :-)

@ruflin
Copy link
Member

ruflin commented Nov 22, 2018

@webmat SGTM. I assume only thing holding this one back from merging is field aliases?

@webmat
Copy link
Contributor Author

webmat commented Nov 22, 2018

@ruflin Yes, aliases are the only thing missing. Before I go ahead and add them here, can you check out this comment: #9135 (comment)? I've tried creating aliases, and there are issues.

@ruflin
Copy link
Member

ruflin commented Nov 22, 2018

@webmat I assume our offline conversation resolved this. Let me know when this one is ready for an other round.

@webmat
Copy link
Contributor Author

webmat commented Nov 22, 2018 •
edited
Loading

This PR is ready for final review.

Note that on top of ECS migration, now pid and port fields are cast to int in event body (they were strings).

Mathieu Martin added 6 commits November 22, 2018 16:46
Perform all the straightforward renames at once.
d6860c6 
- haproxy.client.port => source.port
- haproxy.process_name => process.name
- haproxy.pid => process.pid
- haproxy.destination.ip => destination.ip
- haproxy.destination.port => destination.port

Add grok to conditionally extract `haproxy.client.ip` to `source.ip` (if an IP), or to `source.domain` otherwise.
Add a few missing integer conversions in one of the log formats
dd2d590
Shove all renames so far in ecs-migration.yml
e8e1e01
Changelog
b49dc4a
Move non-normalized field defs at the top, to promote sanity.
3b53bdf
client.ip is not actually aliased. Add note that it can be a Unix socket
d1c9830
@webmat webmat merged commit f6bbfd5 into elastic:master Nov 22, 2018
@webmat webmat deleted the ecs-haproxy-log branch November 22, 2018 21:59
@webmat webmat mentioned this pull request Dec 17, 2018
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin approved these changes

Assignees

@webmat webmat

Labels
ecs Filebeat Filebeat module review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@webmat @ruflin