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 autodiscover for docker in filebeat #10862

Closed
wants to merge 10 commits into from
Closed

Fix autodiscover for docker in filebeat #10862

wants to merge 10 commits into from

Conversation

kaiyan-sheng
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng commented Feb 21, 2019

This PR is based on #10758

Results from autodiscover for docker with filebeat have docker.container.id fields instead of the ECS fields container.id.

Turned out the container information in filebeat output came from docker autodiscover. Without dedot in autodiscover, labels in output json will not be dedotted. If processor add_docker_metadata is enabled, then the labels in json output will be duplicated: one set of labels without dedotting (from autodiscover) and another set of labels after dedotting (from add_docker_metadata).

Please see results in pictures below.

  1. With add_docker_metadata labels.dedot=true but no dedot for autodiscover (docker.autodiscover labels.dedot=false):

screen shot 2019-02-21 at 2 15 50 pm

You can see same labels showed up twice, once with dots and the other without.
  1. With docker.autodiscover labels.dedot=true:

screen shot 2019-02-21 at 2 08 43 pm

You can see there is no duplicate labels.

@kaiyan-sheng
Copy link
Contributor Author

@jsoriano I think this is ready for review :-) Seems like we do need to add dedot in docker autodiscover after all. Otherwise when https://github.com/elastic/beats/blob/master/libbeat/processors/add_docker_metadata/add_docker_metadata.go#L188 happens, the labels will get both the dedotted version and the original version with dots. Please correct me if this doesn't make sense to you at all.... I'm still trying to figure out the inner connections :-) Thanks!

@jsoriano
Copy link
Member

@kaiyan-sheng thanks for doing this, I have been testing it and I found that dedotting was being applied to selectors, so conditions on labels with dots stopped working, as well as hints with dots. I am continuing with #10898 to address these issues.

I have opened also a backport (#10899) so dedotting of autodiscover labels is available also in 6.7, but disabled by default.

jsoriano added a commit that referenced this pull request Feb 25, 2019
Fields injected by docker autodiscover provider were being placed
in alias fields introduced for ECS, change them to the new location
and add selectors accordingly. 

This PR includes #10862 and #10758

As a summary:
    * Autodiscover selectors using ECS structure are added to
      autodiscover events, old selectors are kept for backwards compatibility
    * Autodiscover generated metadata follows ECS
    * Dedotting of labels is added, enabled by default, will be backported for 6.7,
      but disabled

`docker.containers.labels` is not migrated, as it wasn't for `add_docker_metadata`
(see #9412)

Fixes #10757

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>
Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>
@jsoriano
Copy link
Member

Merged as part of #10898

@jsoriano jsoriano closed this Feb 25, 2019
jsoriano added a commit to jsoriano/beats that referenced this pull request Feb 25, 2019
Fields injected by docker autodiscover provider were being placed
in alias fields introduced for ECS, change them to the new location
and add selectors accordingly. 

This PR includes elastic#10862 and elastic#10758

As a summary:
    * Autodiscover selectors using ECS structure are added to
      autodiscover events, old selectors are kept for backwards compatibility
    * Autodiscover generated metadata follows ECS
    * Dedotting of labels is added, enabled by default, will be backported for 6.7,
      but disabled

`docker.containers.labels` is not migrated, as it wasn't for `add_docker_metadata`
(see elastic#9412)

Fixes elastic#10757

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>
Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>
(cherry picked from commit 1bf8087)
jsoriano added a commit that referenced this pull request Feb 27, 2019
Fields injected by docker autodiscover provider were being placed
in alias fields introduced for ECS, change them to the new location
and add selectors accordingly. 

This PR includes #10862 and #10758

As a summary:
    * Autodiscover selectors using ECS structure are added to
      autodiscover events, old selectors are kept for backwards compatibility
    * Autodiscover generated metadata follows ECS
    * Dedotting of labels is added, enabled by default, will be backported for 6.7,
      but disabled

`docker.containers.labels` is not migrated, as it wasn't for `add_docker_metadata`
(see #9412)

Fixes #10757

(cherry picked from commit 1bf8087)

Co-Authored-By: kaiyan-sheng <kaiyan.sheng@elastic.co>
Co-Authored-By: Nicolas Ruflin <spam@ruflin.com>
@ainiml
Copy link

ainiml commented Jun 18, 2019

@jsoriano I think this is ready for review :-) Seems like we do need to add dedot in docker autodiscover after all

Both labels.dedot and templates are required for autodiscovery to work, but the template should not be needed if you just want to gather all docker containers, and want to use autodiscover to add the docker metadata

 filebeat.autodiscover:
   providers:
     - type: docker
       labels.dedot: true
       templates:
         - condition:
           contains:
             docker.container.name: "*"
           config:
             - type: docker
               containers.ids:
                 - "${data.docker.container.id}"

@jsoriano
Copy link
Member

@ainiml for this case you can use hints-based autodiscover, with the advantage of being able to add fine-grained configuration to specific pods and containers if you need it.

@ainiml
Copy link

ainiml commented Jun 18, 2019

@jsoriano thanks! Going with the least effort was the initial goal, just to get enough information into logstash to do some filtering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants