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

Fix field alias for nginx.access.remote_ip #11512

Merged
merged 3 commits into from
Mar 29, 2019

Conversation

webmat
Copy link
Contributor

@webmat webmat commented Mar 28, 2019

  • It was missing in the breaking changes doc (generated from ecs-migration.yml)
  • The actual field alias was incorrectly pointing to source.ip, this has been
    adjusted to source.address
  • Re-generating the documentation file also updated the breaking changes to
    include a change introduced in Adding categorization fields for the system/auth module #11334

This should be backported to 7.0.

Closes #11510

Mathieu Martin added 3 commits March 28, 2019 12:33
@webmat webmat requested review from a team as code owners March 28, 2019 16:44
@webmat webmat self-assigned this Mar 28, 2019
@webmat webmat added ecs needs_backport PR is waiting to be backported to other branches. labels Mar 28, 2019
@EthanStrider
Copy link

Will this update the filebeat nginx dashboards as well, as I'm fairly certain there are some visualizations that still point to nginx.access.remote_ip.

Screen Shot 2019-03-28 at 12 56 30 PM

@webmat
Copy link
Contributor Author

webmat commented Mar 28, 2019

@EthanStrider Ah indeed. The updates to kibana objects have also been done with the file that was missing this entry. I'll update this as well.

@EthanStrider
Copy link

I don't know if you want to try to address this issue: #11517 in this PR, but mentioning it in case you do.

@webmat
Copy link
Contributor Author

webmat commented Mar 28, 2019

@EthanStrider Ok, the source of this job is here: https://github.com/elastic/kibana/blob/7.0/x-pack/plugins/ml/server/models/data_recognizer/modules/nginx_ecs/ml/source_ip_request_rate_ecs.json

It appears like it's been adjusted to use the ECS fields directly in January (elastic/kibana#29383), so this should be in 7.0-rc1. How did you get this ML content installed, are you using 7.0-rc1?

@webmat
Copy link
Contributor Author

webmat commented Mar 28, 2019

Wait, I take that back. It may be fine over in that repo.

But when playing with Beats locally, the wrong file seems to be pulled in, referencing the old field name.

@webmat
Copy link
Contributor Author

webmat commented Mar 28, 2019

jenkins, test this

Copy link
Contributor

@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.

LGTM. Will directly merge and open backport. CI is failing because of missing Kafka.

@ruflin ruflin merged commit 692ef9e into elastic:master Mar 29, 2019
ruflin pushed a commit to ruflin/beats that referenced this pull request Mar 29, 2019
- It was missing in the breaking changes doc (generated from ecs-migration.yml)
- The actual field alias was incorrectly pointing to source.ip, this has been
  adjusted to source.address
- Re-generating the documentation file also updated the breaking changes to
  include a change introduced in elastic#11334

This should be backported to 7.0.

Closes elastic#11510

(cherry picked from commit 692ef9e)
@ruflin ruflin added v7.0.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 29, 2019
ruflin added a commit that referenced this pull request Mar 29, 2019
- It was missing in the breaking changes doc (generated from ecs-migration.yml)
- The actual field alias was incorrectly pointing to source.ip, this has been
  adjusted to source.address
- Re-generating the documentation file also updated the breaking changes to
  include a change introduced in #11334

This should be backported to 7.0.

Closes #11510

(cherry picked from commit 692ef9e)
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.

3 participants