docker autodiscover not ECS ready

[roncohen]

since we moved docker fields from docker.container.* to container.* as part of ECS, we need to update docker autodiscover to use the new fields.

[ruflin]

Leaving comments on the investigation in case someone picks up later:

The data seems to come form these two lines: https://github.com/elastic/beats/blob/master/libbeat/autodiscover/providers/docker/docker.go#L153 and https://github.com/elastic/beats/blob/master/libbeat/autodiscover/providers/docker/docker.go#L170 It's not clear to me yet how this data makes it into the events themself.

[ruflin]

The meta fields from autodiscovery are added through multiple hops to the DynamicFields in the ClientConfig: https://github.com/elastic/beats/blob/master/libbeat/beat/pipeline.go#L60 Based on this they enrich all events on this Client.

The structure in autodiscovery for the meta but also the hints fields must be adjusted. This will break existing hints configurations like here for example:

https://github.com/elastic/apm-integration-testing/blob/93b405996b05b3389fdd48ff5655909551b38da8/docker/filebeat/filebeat.yml#L29

It means these have to be changed from "${data.docker.container.id}" to "${data.container.id}".

@exekias @jsoriano As the enrichment of the autodiscovery events does not use the same code base as the add_host_metadata processor we should find a way to make sure the file structure and fields we provide with the two stay in sync.

[ruflin]

I put up a first WIP PR with a potential fix but still some things need clarification to make sure it doesn't break anything else: #10758

[jsoriano]

I have been talking with @kaiyan-sheng about this, for the record I am transcribing here some of the things we talked:

Autodiscover metadata is only used internally for the communication between autodiscover providers and the autodiscover framework, and to resolve the conditions and templates of autodiscover configuration. This data is collected by autodiscover providers and [afaik] in principle it is not used in events sent to the outputs. Kaiyan also mentioned that she saw that changing the autodiscover metadata didn't change the fields in events, what would confirm that.

Migrating this metadata to ECS-like fields would have its benefits: fields available would match fields stored following ECS, and some configurations could be the same with docker and kubernetes autodiscover. But I think it is important to remember that this metadata doesn't affect the stored events.

I also think it'd be important here to keep backwards compatibility even in 7.X. We cannot use aliases or other things we have used for ECS migration, but I think it can be easily done here by just duplicating the info sent in autodiscover events. This would prevent lots of headaches in upgrades, it is already a bit tricky to debug some problems with autodiscover.

Other things to take into account:

• Hints generators also use this configuration to create provider-agnostic payloads for the templates in the hints, we should also adapt them (nothing to do if we keep backwards compatibility).
• There are other places where container fields are generated, we should review if we want them migrated for consistency:
  • add_docker_metadata (already migrated)
  • add_kubernetes_metadata
  • kubernetes module
  • docker module
• Also remember that we are not migrating labels fields because we cannot create aliases and we don't have a clear strategy for upgrade and backwards compatibility (see this conversation for more info).

[kaiyan-sheng]

@roncohen Is it too late to migrate docker and kubernetes module to ECS since we are already in feature freeze?

[roncohen]

thanks @jsoriano. I think there might be some confusion around what the ECS migration entails and what the current state of things are.

current state:
If i use autodiscover for docker with the newest 7.0 filebeat, my events have docker.container.id fields instead of the ECS fields.

Here's the version i used:

2019-02-19T16:42:12.699Z	INFO	[beat]	instance/beat.go:818	Build info	{"system_info": {"build": {"commit": "b1fd1f413c86ba5b8d1d5664864e62f3ad0b07ae", "libbeat": "7.0.0", "time": "2019-02-18T17:31:33.000Z", "version": "7.0.0"}}}

Here's a screenshot showing the non-ECS fields being written:

required state:
In 7.0 we need to support ECS. This means we write to the ECS fields in events.

There's an opt-in option to create backwards compatible aliases. That would be an alias pointing from docker.container.id to container.id for example.

I don't think it's particularly important to change the meta data used internally for the communication between autodiscover providers and the autodiscover framework, but it's very important to change the resulting events to be ECS compatible, so we need to prioritize that.

Not supporting ECS is a bug for 7.0 so we can change it after feature freeze.

Let me know if there's something i misunderstood.

[kaiyan-sheng]

Thanks @roncohen for your explanation. Sorry I was confused earlier. This is a bug for docker autodiscover with filebeat then. I created a PR to experiment with how to fix this: #10836

[roncohen]

thanks @kaiyan-sheng. There's also this one which may be useful: #10758