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

Introduce log.source.address and log.file.path for 7.x compatiblity #9435

Merged
merged 3 commits into from
Dec 12, 2018
Merged

Introduce log.source.address and log.file.path for 7.x compatiblity #9435

merged 3 commits into from
Dec 12, 2018

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Dec 7, 2018 •
edited
Loading

Related to #8902 but adding the fields instead of replacing

@ruflin ruflin added in progress Pull request is currently in progress. Filebeat Filebeat ecs labels Dec 7, 2018
@ruflin ruflin mentioned this pull request Dec 7, 2018
Closed
@ruflin ruflin added review and removed in progress Pull request is currently in progress. labels Dec 10, 2018
@ruflin ruflin self-assigned this Dec 10, 2018
@ruflin ruflin changed the title [WIP] Introduce log.source.ip and log.file.path for 7.x compatiblity Introduce log.source.ip and log.file.path for 7.x compatiblity Dec 10, 2018
@ruflin ruflin force-pushed the introduce-new-source-fields branch from 2cb7600 to a31733e Compare December 10, 2018 08:40
@ruflin ruflin added the Team:Integrations Label for the Integrations team label Dec 10, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/infrastructure

@ruflin ruflin mentioned this pull request Dec 10, 2018
Closed
@ruflin
Copy link
Contributor Author

ruflin commented Dec 11, 2018

This should not be merged before discussion in #9460 is resolved.

@ruflin ruflin changed the title Introduce log.source.ip and log.file.path for 7.x compatiblity Introduce log.source.source and log.file.path for 7.x compatiblity Dec 11, 2018
@ruflin ruflin changed the title Introduce log.source.source and log.file.path for 7.x compatiblity Introduce log.source.address and log.file.path for 7.x compatiblity Dec 11, 2018
@ruflin ruflin force-pushed the introduce-new-source-fields branch from 83a9f46 to edfd916 Compare December 11, 2018 20:45
@ruflin
Copy link
Contributor Author

ruflin commented Dec 11, 2018

PR was changed to use log.source.address

webmat
webmat suggested changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

A few minor details, then we're good:

  • Changelog rebase fun
  • One missing log.source.ip => log.source.address rename

Question, not a problem: should we make file reusable in ECS (ref log.file.*)?

CHANGELOG.asciidoc Show resolved Hide resolved
filebeat/_meta/fields.common.yml Show resolved Hide resolved
filebeat/_meta/fields.common.yml Show resolved Hide resolved
filebeat/input/udp/input.go Outdated Show resolved Hide resolved
@ruflin
Introduce log.source.address and log.file.path for 7.x compatiblity
8750ba1 
Related to elastic#8902 but adding the fields instead of replacing
@ruflin ruflin force-pushed the introduce-new-source-fields branch from 9b39084 to 8750ba1 Compare December 12, 2018 09:55
@ruflin ruflin added the v6.6.0 label Dec 12, 2018
webmat
webmat reviewed Dec 12, 2018
View reviewed changes
x-pack/filebeat/module/suricata/eve/test/eve-alerts.log-expected.json Outdated
@@ -17,6 +17,7 @@
"http.request.method": "GET",
"http.response.status_code": "200",
"input.type": "log",
"log.file.path": "/Users/ruflin/Dev/gopath/src/github.com/elastic/beats/x-pack/filebeat/module/suricata/eve/test/eve-alerts.log",
Copy link
Contributor

Choose a reason for hiding this comment

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

These paths are still present in x-pack

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will update these files too and push again.

webmat
webmat suggested changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

One last thing: the golden files in the x-pack directory still have your the log file path.

Then we're good 👍

webmat
webmat approved these changes Dec 12, 2018
View reviewed changes
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM

filebeat/module/icinga/startup/test/test.log-expected.json
@@ -1,6 +1,6 @@
[
{
"@timestamp": "2018-12-11T08:08:07.894Z",
"@timestamp": "2018-12-12T11:22:05.182Z",
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think it's a problem, but why are these timestamps still changing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed that with #9506. It's not an issue for CI.

@ruflin ruflin merged commit f81831f into elastic:6.x Dec 12, 2018
@ruflin ruflin deleted the introduce-new-source-fields branch December 12, 2018 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webmat webmat webmat approved these changes

Assignees

@ruflin ruflin

Labels
ecs Filebeat Filebeat review Team:Integrations Label for the Integrations team v6.6.0
Projects
None yet
Milestone
No milestone
Development

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