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

Add cache.ttl to add_host_metadata #9359

Merged
merged 4 commits into from
Dec 4, 2018
Merged

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Dec 3, 2018

Despite what is says in the documentation (NOTE: The host information is refreshed every 5 minutes.) the add_host_metadata processor does not actually ever refresh most of its information (it does the IPs and MACs if netinfo.enabled: true, but not the base host info).

This PR changes it to call sysinfo.Host() every time its cache expires.

It also adds a cache.ttl parameter to configure how often that happens. The default stays at 5m, and negative values disable caching altogether (most likely at some cost to performance).

I did not add the new config parameter to all the *beat.reference.yml files - I noticed they didn't contain everything (e.g. add_kubernetes_metadata) and I think this parameter is not something most users need to configure. Happy to put it in though if it makes sense.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. But it needs a CHANGELOG entry.

}

data := host.MapHostInfo(p.info)
Copy link
Member

Choose a reason for hiding this comment

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

IIUC the problem was here b/c it never fetched updated info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Code LGTM. See comment Andrew.

@ruflin ruflin added the needs_backport PR is waiting to be backported to other branches. label Dec 4, 2018
@ruflin
Copy link
Member

ruflin commented Dec 4, 2018

I added needs_backport as I think we should backport this fix.

@cwurm
Copy link
Contributor Author

cwurm commented Dec 4, 2018

I've added two changelog entries (one under Bugfix, one under Added) - let me know if it's good to merge please.

@cwurm cwurm merged commit 30dfa05 into elastic:master Dec 4, 2018
cwurm pushed a commit to cwurm/beats that referenced this pull request Dec 4, 2018
Fix: Refresh host info when cache expires.

Add config parameter `cache.ttl` to control cache expiration (default 5m).

(cherry picked from commit 30dfa05)
@cwurm cwurm added v6.6.0 and removed needs_backport PR is waiting to be backported to other branches. review labels Dec 4, 2018
cwurm pushed a commit that referenced this pull request Dec 4, 2018
Fix: Refresh host info when cache expires.

Add config parameter `cache.ttl` to control cache expiration (default 5m).

(cherry picked from commit 30dfa05)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants