From 94dc9cb58989326823da765783dd1850512367a5 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 20 Sep 2016 09:03:53 +0000 Subject: [PATCH] Review feedback --- probe/endpoint/dns_snooper_linux_amd64.go | 3 --- probe/endpoint/reporter.go | 8 ++++---- render/container.go | 2 +- render/process.go | 10 ++++++---- render/theinternet.go | 10 +++++----- 5 files changed, 16 insertions(+), 17 deletions(-) diff --git a/probe/endpoint/dns_snooper_linux_amd64.go b/probe/endpoint/dns_snooper_linux_amd64.go index e4750d16c5..2c5b9dc760 100644 --- a/probe/endpoint/dns_snooper_linux_amd64.go +++ b/probe/endpoint/dns_snooper_linux_amd64.go @@ -48,9 +48,6 @@ func newPcapHandle() (*pcap.Handle, error) { return nil, err } defer inactive.CleanUp() - if err = inactive.SetPromisc(true); err != nil { - return nil, err - } // pcap timeout blackmagic copied from Weave Net to reduce CPU consumption // see https://github.com/weaveworks/weave/commit/025315363d5ea8b8265f1b3ea800f24df2be51a4 if err = inactive.SetTimeout(time.Duration(math.MaxInt64)); err != nil { diff --git a/probe/endpoint/reporter.go b/probe/endpoint/reporter.go index 5443112365..f357e6efd6 100644 --- a/probe/endpoint/reporter.go +++ b/probe/endpoint/reporter.go @@ -21,6 +21,7 @@ const ( Conntracked = "conntracked" Procspied = "procspied" ReverseDNSNames = "reverse_dns_names" + SnoopedDNSNames = "snooped_dns_names" ) // Reporter generates Reports containing the Endpoint topology. @@ -195,11 +196,10 @@ func (r *Reporter) makeEndpointNode(namespaceID string, addr string, port uint16 node := report.MakeNodeWith( report.MakeEndpointNodeID(r.hostID, namespaceID, addr, portStr), map[string]string{Addr: addr, Port: portStr}) - names := r.dnsSnooper.CachedNamesForIP(addr) - if resolvedNames, err := r.reverseResolver.get(addr); err == nil { - names = append(names, resolvedNames...) + if names := r.dnsSnooper.CachedNamesForIP(addr); len(names) > 0 { + node = node.WithSet(SnoopedDNSNames, report.MakeStringSet(names...)) } - if len(names) > 0 { + if names, err := r.reverseResolver.get(addr); err == nil && len(names) > 0 { node = node.WithSet(ReverseDNSNames, report.MakeStringSet(names...)) } if extra != nil { diff --git a/render/container.go b/render/container.go index 3a2705b32f..eec54e8727 100644 --- a/render/container.go +++ b/render/container.go @@ -112,7 +112,7 @@ func ShortLivedConnectionJoin(r Renderer, toIPs func(report.Node) []string) Rend return report.Nodes{} } if ip := net.ParseIP(addr); ip != nil && !local.Contains(ip) { - node := toInternetNode(m) + node := externalNode(m) return report.Nodes{node.ID: node} } diff --git a/render/process.go b/render/process.go index bc344eba74..059ad8bfbc 100644 --- a/render/process.go +++ b/render/process.go @@ -97,7 +97,7 @@ func MapEndpoint2Pseudo(n report.Node, local report.Networks) report.Nodes { if ip := net.ParseIP(addr); ip != nil && !local.Contains(ip) { // If the dstNodeAddr is not in a network local to this report, we emit an // external pseudoNode - node = toInternetNode(n) + node = externalNode(n) } else { // due to https://github.com/weaveworks/scope/issues/1323 we are dropping // all non-internet pseudo nodes for now. @@ -157,11 +157,13 @@ func MapProcess2Name(n report.Node, _ report.Networks) report.Nodes { return report.Nodes{name: node} } -func toInternetNode(m report.Node) report.Node { +func externalNode(m report.Node) report.Node { // First, check if it's a known service and emit a // a specific node if it is - hostnames, _ := m.Sets.Lookup(endpoint.ReverseDNSNames) - for _, hostname := range hostnames { + snoopedHostnames, _ := m.Sets.Lookup(endpoint.SnoopedDNSNames) + reverseHostnames, _ := m.Sets.Lookup(endpoint.ReverseDNSNames) + // Intentionally prioritize snooped hostnames + for _, hostname := range append(snoopedHostnames, reverseHostnames...) { if isKnownService(hostname) { return NewDerivedPseudoNode(ServiceNodeIDPrefix+hostname, m) } diff --git a/render/theinternet.go b/render/theinternet.go index 7546f93a2b..7417af1027 100644 --- a/render/theinternet.go +++ b/render/theinternet.go @@ -9,17 +9,17 @@ import ( ) var ( - // ServiceNodeIDPrefix is how the ID all service pseudo nodes begin + // ServiceNodeIDPrefix is how the ID of all service pseudo nodes begin ServiceNodeIDPrefix = "service-" - knownServicesMatchers = []*regexp.Regexp{ + knownServiceMatchers = []*regexp.Regexp{ // See http://docs.aws.amazon.com/general/latest/gr/rande.html for fainer grained // details regexp.MustCompile(`^.+\.amazonaws\.com$`), regexp.MustCompile(`^.+\.googleapis\.com$`), } - knownServicesExcluder = []*regexp.Regexp{ + knownServiceExcluders = []*regexp.Regexp{ // We exclude ec2 machines because they are too generic // and having separate nodes for them makes visualizations worse regexp.MustCompile(`^ec2.*\.amazonaws\.com$`), @@ -28,7 +28,7 @@ var ( func isKnownService(hostname string) bool { foundMatch := false - for _, matcher := range knownServicesMatchers { + for _, matcher := range knownServiceMatchers { if matcher.MatchString(hostname) { foundMatch = true break @@ -38,7 +38,7 @@ func isKnownService(hostname string) bool { return false } - for _, excluder := range knownServicesExcluder { + for _, excluder := range knownServiceExcluders { if excluder.MatchString(hostname) { return false }