Review feedback

probe/endpoint/dns_snooper.go → probe/endpoint/dns_snooper_linux_amd64.go

@@ -43,20 +43,11 @@ func NewDNSSnooper() (*DNSSnooper, error) {
 }
 
 func newPcapHandle() (*pcap.Handle, error) {
-	// TODO: use specific interfaces instead?
 	inactive, err := pcap.NewInactiveHandle("any")
 	if err != nil {
 		return nil, err
 	}
 	defer inactive.CleanUp()
-	if err = inactive.SetPromisc(true); err != nil {
-		return nil, err
-	}
-	// TODO: reduce the size of packets being copied? maybe an overoptimization
-	// if err = inactive.SetSnapLen(snaplen); err != nil {
-	//	return
-	// }
-
 	// 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 {
@@ -132,7 +123,7 @@ func (s *DNSSnooper) run() {
 		sll           layers.LinuxSLL
 	)
 
-	// assumes that the "any" interface in being used (see https://wiki.wireshark.org/SLL)
+	// assumes that the "any" interface is being used (see https://wiki.wireshark.org/SLL)
 	packetParser := gopacket.NewDecodingLayerParser(layers.LayerTypeLinuxSLL, &sll, &eth, &ip4, &ip6, &udp, &tcp, &dns)
 
 	for {

probe/endpoint/dns_snooper_others.go

@@ -0,0 +1,25 @@
+// +build darwin arm
+
+// Cross-compiling the snooper requires having pcap binaries,
+// let's disable it for now.
+// See http://stackoverflow.com/questions/31648793/go-programming-cross-compile-for-revel-framework
+
+package endpoint
+
+// DNSSnooper is a snopper of DNS queries
+type DNSSnooper struct{}
+
+// NewDNSSnooper creates a new snooper of DNS queries
+func NewDNSSnooper() (*DNSSnooper, error) {
+	return nil, nil
+}
+
+// CachedNamesForIP obtains the domains associated to an IP,
+// obtained while snooping A-record queries
+func (s *DNSSnooper) CachedNamesForIP(ip string) []string {
+	return []string{}
+}
+
+// Stop makes the snooper stop inspecting DNS communications
+func (s *DNSSnooper) Stop() {
+}

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 {

prog/probe.go

@@ -145,10 +145,9 @@ func probeMain(flags probeFlags) {
 		p.AddReporter(process.NewReporter(processCache, hostID, process.GetDeltaTotalJiffies))
 	}
 
-	// TODO: make the snooper optional
 	dnsSnooper, err := endpoint.NewDNSSnooper()
 	if err != nil {
-		log.Errorf("Fail to start DNS snooper: nodes for external services will be less accurate: %s", err)
+		log.Errorf("Failed to start DNS snooper: nodes for external services will be less accurate: %s", err)
 	} else {
 		defer dnsSnooper.Stop()
 	}

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}
 		}
 

render/process.go

@@ -2,6 +2,7 @@ package render
 
 import (
 	"net"
+	"sort"
 
 	"github.com/weaveworks/scope/probe/docker"
 	"github.com/weaveworks/scope/probe/endpoint"
@@ -97,7 +98,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 +158,16 @@ 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)
+	// Sort the names to make the lookup more deterministic
+	sort.StringSlice(snoopedHostnames).Sort()
+	sort.StringSlice(reverseHostnames).Sort()
+	// Intentionally prioritize snooped hostnames
+	for _, hostname := range append(snoopedHostnames, reverseHostnames...) {
 		if isKnownService(hostname) {
 			return NewDerivedPseudoNode(ServiceNodeIDPrefix+hostname, m)
 		}

render/theinternet.go

@@ -9,26 +9,27 @@ 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$`),
 	}
 )
 
+// TODO: Make it user-customizable https://github.com/weaveworks/scope/issues/1876
 func isKnownService(hostname string) bool {
 	foundMatch := false
-	for _, matcher := range knownServicesMatchers {
+	for _, matcher := range knownServiceMatchers {
 		if matcher.MatchString(hostname) {
 			foundMatch = true
 			break
@@ -38,7 +39,7 @@ func isKnownService(hostname string) bool {
 		return false
 	}
 
-	for _, excluder := range knownServicesExcluder {
+	for _, excluder := range knownServiceExcluders {
 		if excluder.MatchString(hostname) {
 			return false
 		}