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

Update migration file to new structure #9381

Merged
merged 1 commit into from
Dec 5, 2018

Conversation

ruflin
Copy link
Collaborator

@ruflin ruflin commented Dec 4, 2018

The new structure makes it clear which alias have to happen in 6.x and which ones in 7.x

@ruflin ruflin added review ecs Team:Integrations Label for the Integrations team labels Dec 4, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/infrastructure

@ruflin ruflin force-pushed the migration-file-update branch from 160bda9 to 388e342 Compare December 4, 2018 14:50
@ruflin ruflin requested review from webmat and graphaelli December 4, 2018 14:51
@@ -6,44 +6,46 @@
#
# - from: source-field-in-6.x
# to: target-filed-in-ECS
# # Alias field is useful for fields where there is a 1-1 mapping from old to new
# alias: true-if-alias-is-required-in-6x
# # Alias field is useful for fields where many-11 mapping from new to old are needed
Copy link
Member

Choose a reason for hiding this comment

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

many-1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see that fix. Have you pushed? 😂

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. Just a nit about adding a comment for the meaning when alias6 is absent

Copy link
Collaborator Author

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

@webmat would be great if you could go through the fields to make sure there are no mistakes inside.

@@ -6,44 +6,46 @@
#
# - from: source-field-in-6.x
# to: target-filed-in-ECS
# # Alias field is useful for fields where there is a 1-1 mapping from old to new
# alias: true-if-alias-is-required-in-6x
# # Alias field is useful for fields where many-11 mapping from new to old are needed
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@webmat
Copy link
Contributor

webmat commented Dec 4, 2018

Yeah I did read through quickly. Will pore through it one more time.

The new structure makes it clear which alias have to happen in 6.x and which ones in 7.x
@ruflin ruflin force-pushed the migration-file-update branch from 388e342 to 782a491 Compare December 4, 2018 15:10
alias: true
copy_to: false

- from: beat.hostname
to: agent.hostname
Copy link
Contributor

Choose a reason for hiding this comment

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

@MikePaquette is pushing back on agent.hostname, btw.

Agent should always be running on the host itself. If the monitoring agent is outside of the host, it should be represented in observer.hostname (used to be called device.hostname, but device is being renamed to observer).

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be addressed in another PR, though.

@webmat webmat self-requested a review December 4, 2018 15:20
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.

I went through the file again. The mappings look good, but I think the Auditbeat mapping should be removed.

There should be a pull request to address Auditbeat in its entirety.

@ruflin ruflin merged commit 2aca783 into elastic:master Dec 5, 2018
@ruflin ruflin mentioned this pull request Dec 5, 2018
@ruflin ruflin deleted the migration-file-update branch December 5, 2018 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ecs review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants