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

Don't collect empty ip addresses in docker container metricset #11247

Merged
merged 3 commits into from
Mar 18, 2019

Conversation

jsoriano
Copy link
Member

Docker containers can have empty ip addresses if they are running in
host network mode or if they are stopped. Collecting lists with empty
addresses can make type mapping to fail when trying to store them as ip
addresses.

It only affects 7.0 because before these ip addresses were of type
keyword.

Fix #11225

Docker containers can have empty ip addresses if they are running in
host network mode or if they are stopped. Collecting lists with empty
addresses can make type mapping to fail when trying to store them as ip
addresses.
@jsoriano jsoriano added bug module review Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. containers Related to containers use case v7.0.0 labels Mar 14, 2019
@jsoriano jsoriano self-assigned this Mar 14, 2019
@jsoriano jsoriano requested a review from a team as a code owner March 14, 2019 11:04
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.

LGTM

@@ -71,7 +71,9 @@ func eventMapping(r mb.ReporterV2, cont *types.Container, dedot bool) {
func extractIPAddresses(networks *types.SummaryNetworkSettings) []string {
ipAddresses := make([]string, 0, len(networks.Networks))
Copy link
Member

Choose a reason for hiding this comment

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

With this change ipAddresses is now not anymore len(networks.Networks). Is it mainly in testing that it might be empty or a common case? Thinking which use case we should optimise for.

Copy link
Member Author

Choose a reason for hiding this comment

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

I though about this and I think that in most of the cases ipAddresses is still going to be len(networks.Networks).
The most common case where this is not going to be true is when the container is using the host network, in that case we would be creating an array of capacity one that we are not going to use.

@jsoriano jsoriano merged commit 258c1c8 into elastic:master Mar 18, 2019
@jsoriano jsoriano deleted the docker-empty-addresses branch March 18, 2019 18:04
jsoriano added a commit to jsoriano/beats that referenced this pull request Mar 18, 2019
…ic#11247)

Docker containers can have empty ip addresses if they are running in
host network mode or if they are stopped. Collecting lists with empty
addresses can make type mapping to fail when trying to store them as ip
addresses.

Fix elastic#11225

(cherry picked from commit 258c1c8)
@jsoriano jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Mar 18, 2019
jsoriano added a commit that referenced this pull request Mar 19, 2019
… container metricset (#11293)

Docker containers can have empty ip addresses if they are running in
host network mode or if they are stopped. Collecting lists with empty
addresses can make type mapping to fail when trying to store them as ip
addresses.

Fix #11225

(cherry picked from commit 258c1c8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug containers Related to containers use case Metricbeat Metricbeat module review v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants