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

Prepare docker autodiscover in 6.7 for ECS and dedotting #10899

Merged
merged 16 commits into from
Feb 26, 2019

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Feb 22, 2019

Partial backport of #10898 to 6.7

Summary of backported changes:

  • ECS fields are added as autodiscover selectors
  • Labels dedotting added, disabled by default

Co-Authored-By: kaiyan-sheng kaiyan.sheng@elastic.co
Co-Authored-By: Nicolas Ruflin spam@ruflin.com

@jsoriano jsoriano added Filebeat Filebeat libbeat containers Related to containers use case ecs Team:Integrations Label for the Integrations team v6.7.0 labels Feb 22, 2019
@jsoriano jsoriano self-assigned this Feb 22, 2019
@jsoriano jsoriano requested a review from roncohen February 22, 2019 16:59
@jsoriano jsoriano requested a review from a team February 22, 2019 17:41
func (d *Provider) emitContainer(event bus.Event, flag string) {
container, meta := d.generateMetaDocker(event)
var host string
if len(container.IPAddresses) > 0 {
Copy link
Contributor

@roncohen roncohen Feb 25, 2019

Choose a reason for hiding this comment

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

Looks like if the condition here is hit, we'll return nil for container and this line will panic

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, this also affects 7.0 branch that I already merged 🙁 I will open another PR to fix it in master.

Copy link
Member Author

@jsoriano jsoriano Feb 25, 2019

Choose a reason for hiding this comment

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

Fixed here and opened PR for master/7.0 #10928

@roncohen
Copy link
Contributor

thanks for taking this on @jsoriano.
Will you also take a note to make sure it's documented that people can use the new meta fields already in 6.7?

@jsoriano
Copy link
Member Author

@roncohen thanks for the review! I have created #10924 to document all this once everything is merged.

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM

@jsoriano jsoriano merged commit 7dbf4cc into elastic:6.7 Feb 26, 2019
@jsoriano jsoriano deleted the autodiscovery-ecs-docker-6.7 branch February 26, 2019 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
containers Related to containers use case ecs Filebeat Filebeat libbeat review Team:Integrations Label for the Integrations team v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants