Only set host.name if not skipped

[simitt]

fixes elastic/apm-server#1846

[ruflin]

@simitt In general I like this change but I'm a bit concerned on the potential side effects. The reason we initially introduced this was because of older Logstash versions which added host if host.name was not set (I think it was in 6.3). Since then I worry quite a few things started to depend on this field to always be there, not only of add_host_metadata processor is there.

Also see discussion I started in #10698

Let's make sure if we merge that, that we don't break things on the way.

[simitt]

Since then I worry quite a few things started to depend on this field to always be there, not only of add_host_metadata processor is there.
...
Let's make sure if we merge that, that we don't break things on the way.

Could you be a bit more specific on what to look out for? Happy to check out everything works, but I'd need some pointers, as I assumed to be good as long as tests are green.

[ruflin]

3 specific things I worry about:

• What happens when sending data through different versions of LS (6.0, 6.7, 7.0) and not having add_host_metadata enabled and also sending data directly to ES?
• What happens to Infra UI if add_host_metadata is not enabled
• What happens to the system dashboards if add_host_metadata is enabled

But the one I'm worried about most is the unknown one :-)

[andrewkroh]

@simitt What about something similar to what APM added for skipping the agent.* fields? Like this:

andrewkroh@7007d51

This gives users of libbeat more freedom to define what their data looks like. If you choose to skip the built-in metadata then you must be sure to add back the fields like ecs.version if you need them.

[simitt]

@andrewkroh I first wanted to add a SkipAddHostName similar to what you pointed out. However, as I would see setting host.name closely related to the host_metadata_processor I decided to combine it there.

If you prefer a dedicated skip option I am happy to change it, no strong preference the one or the other way.

[andrewkroh]

Actually upon further thought these two changes are not mutually exclusive, I can still add a SkipBuiltInMetadata later. So no need to change anything on my account.

[ruflin]

I've been thinking a bit more about this. It will benefit apm-server, heartbeat and also the use cases where metricbeat monitors an external service. We should move forward on this but also test the above cases. It's very easy to reintroduce it again if things don't go as expected. Removing it will tell us the most on which things break.

The way I would like to see the add_host_metadata processor working is that if there is any host.* field in an event, it will not add further metadata. If no host.* field exists, it will add all of it. There could be a config overwrite: true that also adds host.* data if already a host.* field exists in the event.

[ruflin]

@andrewvc You will be interested in this PR / change.

[simitt]

After some more discussion on this I went with the approach on simply introducing an option to skip adding host.name as it fulfills the need, while other options would have larger implications that should be discussed in a larger context.