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

enable netinfo host metadata #15347

Closed
wants to merge 2 commits into from
Closed

enable netinfo host metadata #15347

wants to merge 2 commits into from

Conversation

DanRoscigno
Copy link
Contributor

The Observability solution expects netinfo.enabled to be true:

processors:
  - add_host_metadata:
      netinfo.enabled: true

When the defaults (false) are set launching from Uptime to Logs or Metrics fails (the filter gets set to host.ip: <ip addr> and by default host.ip is not set.

@kaiyan-sheng
Copy link
Contributor

Does this consider a breaking change? @exekias @jsoriano WDYT?

@jsoriano
Copy link
Member

I wouldn't say that this is a breaking change. It would add some extra fields by default, what I think is ok. We should confirm that the added fields don't cause any conflict, but they are ECS fields (host.ip and host.mac), so I wouldn't expect problems because of that.

But if we do this change, we should do it also in the code, not only in docs and the reference config. We should change the default in https://github.com/elastic/beats/blob/v7.5.2/libbeat/processors/add_host_metadata/config.go#L36

This would also require a changelog entry.

@DanRoscigno
Copy link
Contributor Author

But if we do this change, we should do it also in the code, not only in docs and the reference config. We should change the default in https://github.com/elastic/beats/blob/v7.5.2/libbeat/processors/add_host_metadata/config.go#L36

This would also require a changelog entry.

@jsoriano , do you want me to make that change, or are you going to do it?

@jsoriano
Copy link
Member

jsoriano commented Feb 4, 2020

But if we do this change, we should do it also in the code, not only in docs and the reference config. We should change the default in https://github.com/elastic/beats/blob/v7.5.2/libbeat/processors/add_host_metadata/config.go#L36
This would also require a changelog entry.

@jsoriano , do you want me to make that change, or are you going to do it?

@DanRoscigno as you prefer, the main change would be only to change this false to true in https://github.com/elastic/beats/blob/v7.5.2/libbeat/processors/add_host_metadata/config.go#L36, but we may also need to update the tests.

Then we will also need to review this value in docs and reference configs and add a changelog entry.

@DanRoscigno
Copy link
Contributor Author

I think it is risky for me to modify the tests, can you do it Jaime?

@jsoriano
Copy link
Member

jsoriano commented Feb 4, 2020

Ok, no probl, I'll do it!

@jsoriano jsoriano self-assigned this Feb 4, 2020
@jsoriano
Copy link
Member

jsoriano commented Feb 4, 2020

I have opened #16077 to continue with this. Thanks @DanRoscigno!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Filebeat Filebeat Metricbeat Metricbeat review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants