Fix node scoping rules

We only want to scope (i.e. prefix with hostID) those addresses that are
deemed loopback, to disambiguate them. Otherwise, we want to leave
addresses in unscoped form, so they can be matched, and links between
communicating nodes properly made.

So, we make the isLoopback check in MakeAddressID, and omit hostID if
the address isn't loopback. So far so good.

But this breaks topology rendering, as we were relying on extracting
hostID from adjacency node IDs, to populate origin hosts in the rendered
node output. So we need another way to get origin host from an arbitrary
node.

A survey revealed no reliable way to get that information from IDs in
their new form. However, we have access to node metadata, so this
changeset introduces the OriginHostTagger, which tags each node with its
origin host, via the foreign-key semantics we'll use going forward.

probe/main.go

@@ -66,7 +66,13 @@ func main() {
 	}
 	defer publisher.Close()
 
-	taggers := []tag.Tagger{tag.NewTopologyTagger()}
+	var (
+		hostName = hostname()
+		hostID   = hostName // TODO: we should sanitize the hostname
+	)
+
+	taggers := []tag.Tagger{tag.NewTopologyTagger(), tag.NewOriginHostTagger(hostID)}
+
 	var dockerTagger *tag.DockerTagger
 	if *dockerEnabled && runtime.GOOS == linux {
 		var err error
@@ -84,11 +90,9 @@ func main() {
 	defer close(quit)
 	go func() {
 		var (
-			hostName = hostname()
-			hostID   = hostName // TODO: we should sanitize the hostname
-			pubTick  = time.Tick(*publishInterval)
-			spyTick  = time.Tick(*spyInterval)
-			r        = report.MakeReport()
+			pubTick = time.Tick(*publishInterval)
+			spyTick = time.Tick(*spyInterval)
+			r       = report.MakeReport()
 		)
 
 		for {

probe/tag/origin_host_tagger.go

@@ -0,0 +1,29 @@
+package tag
+
+import (
+	"github.com/weaveworks/scope/report"
+)
+
+type originHostTagger struct{ hostNodeID string }
+
+// NewOriginHostTagger tags each node with a foreign key linking it to its
+// origin host in the host topology.
+func NewOriginHostTagger(hostID string) Tagger {
+	return &originHostTagger{hostNodeID: report.MakeHostNodeID(hostID)}
+}
+
+func (t originHostTagger) Tag(r report.Report) report.Report {
+	for _, topology := range []*report.Topology{
+		&(r.Endpoint),
+		&(r.Address),
+		&(r.Process),
+		&(r.Container),
+		&(r.Host),
+	} {
+		md := report.NodeMetadata{report.HostNodeID: t.hostNodeID}
+		for nodeID := range topology.NodeMetadatas {
+			(*topology).NodeMetadatas[nodeID].Merge(md)
+		}
+	}
+	return r
+}

probe/tag/origin_host_tagger_test.go

@@ -0,0 +1,25 @@
+package tag_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/weaveworks/scope/probe/tag"
+	"github.com/weaveworks/scope/report"
+)
+
+func TestOriginHostTagger(t *testing.T) {
+	var (
+		hostID         = "foo"
+		endpointNodeID = report.MakeEndpointNodeID(hostID, "1.2.3.4", "56789") // hostID ignored
+		nodeMetadata   = report.NodeMetadata{"foo": "bar"}
+	)
+
+	r := report.MakeReport()
+	r.Endpoint.NodeMetadatas[endpointNodeID] = nodeMetadata
+	want := nodeMetadata.Merge(report.NodeMetadata{report.HostNodeID: report.MakeHostNodeID(hostID)})
+	have := tag.NewOriginHostTagger(hostID).Tag(r).Endpoint.NodeMetadatas[endpointNodeID].Copy()
+	if !reflect.DeepEqual(want, have) {
+		t.Errorf("\nwant %+v\nhave %+v", want, have)
+	}
+}

probe/tag/topology_tagger.go

@@ -13,8 +13,11 @@ func NewTopologyTagger() Tagger {
 
 func (topologyTagger) Tag(r report.Report) report.Report {
 	for val, topology := range map[string]*report.Topology{
-		"endpoint": &(r.Endpoint),
-		"address":  &(r.Address),
+		"endpoint":  &(r.Endpoint),
+		"address":   &(r.Address),
+		"process":   &(r.Process),
+		"container": &(r.Container),
+		"host":      &(r.Host),
 	} {
 		md := report.NodeMetadata{"topology": val}
 		for nodeID := range topology.NodeMetadatas {

report/id.go

@@ -27,7 +27,7 @@ func MakeAdjacencyID(srcNodeID string) string {
 	return ">" + srcNodeID
 }
 
-// ParseAdjacencyID produces a node id from an adjancency id
+// ParseAdjacencyID produces a node ID from an adjancency ID.
 func ParseAdjacencyID(adjacencyID string) (string, bool) {
 	if !strings.HasPrefix(adjacencyID, ">") {
 		return "", false
@@ -56,6 +56,10 @@ func MakeEndpointNodeID(hostID, address, port string) string {
 
 // MakeAddressNodeID produces an address node ID from its composite parts.
 func MakeAddressNodeID(hostID, address string) string {
+	if !isLoopback(address) {
+		// Only loopback addresses get scoped by hostID.
+		hostID = ""
+	}
 	return hostID + ScopeDelim + address
 }
 
@@ -77,8 +81,9 @@ func MakeContainerNodeID(hostID, containerID string) string {
 	return hostID + ScopeDelim + containerID
 }
 
-// ParseNodeID produces the scope and remainder from a node ID
-func ParseNodeID(nodeID string) (string, string, bool) {
+// ParseNodeID produces the host ID and remainder (typically an address) from
+// a node ID. Note that hostID may be blank.
+func ParseNodeID(nodeID string) (hostID string, remainder string, ok bool) {
 	fields := strings.SplitN(nodeID, ScopeDelim, 2)
 	if len(fields) != 2 {
 		return "", "", false
@@ -120,3 +125,8 @@ func AddressIDAddresser(id string) net.IP {
 func PanicIDAddresser(id string) net.IP {
 	panic(fmt.Sprintf("PanicIDAddresser called on %q", id))
 }
+
+func isLoopback(address string) bool {
+	ip := net.ParseIP(address)
+	return ip != nil && ip.IsLoopback()
+}

report/report.go

@@ -33,6 +33,13 @@ type Report struct {
 	Host Topology
 }
 
+const (
+	// HostNodeID is a metadata foreign key, linking a node in any topology to
+	// a node in the host topology. That host node is the origin host, where
+	// the node was originally detected.
+	HostNodeID = "host_node_id"
+)
+
 // RenderableNode is the data type that's yielded to the JavaScript layer as
 // an element of a topology. It should contain information that's relevant
 // to rendering a node when there are many nodes visible at once.

report/topology.go

@@ -88,16 +88,19 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod
 	// Build a set of RenderableNodes for all non-pseudo probes, and an
 	// addressID to nodeID lookup map. Multiple addressIDs can map to the same
 	// RenderableNodes.
-	address2mapped := map[string]string{}
+	var (
+		source2mapped = map[string]string{} // source node ID -> mapped node ID
+		source2host   = map[string]string{} // source node ID -> origin host ID
+	)
 	for nodeID, metadata := range t.NodeMetadatas {
 		mapped, ok := mapFunc(nodeID, metadata)
 		if !ok {
 			continue
 		}
 
-		// mapped.ID needs not be unique over all addressIDs. If not, we just overwrite
-		// the existing data, on the assumption that the MapFunc returns the same
-		// data.
+		// mapped.ID needs not be unique over all addressIDs. If not, we just
+		// overwrite the existing data, on the assumption that the MapFunc
+		// returns the same data.
 		nodes[mapped.ID] = RenderableNode{
 			ID:         mapped.ID,
 			LabelMajor: mapped.Major,
@@ -107,24 +110,26 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod
 			Origins:    IDList{nodeID},
 			Metadata:   AggregateMetadata{}, // later
 		}
-		address2mapped[nodeID] = mapped.ID
+		source2mapped[nodeID] = mapped.ID
+		source2host[nodeID] = metadata[HostNodeID]
 	}
 
 	// Walk the graph and make connections.
 	for src, dsts := range t.Adjacency {
 		var (
-			srcNodeID, ok1          = ParseAdjacencyID(src)
-			srcOriginHostID, _, ok2 = ParseNodeID(srcNodeID)
-			srcRenderableID         = address2mapped[srcNodeID] // must exist
-			srcRenderableNode       = nodes[srcRenderableID]    // must exist
+			srcNodeID, ok = ParseAdjacencyID(src)
+			//srcOriginHostID, _, ok2 = ParseNodeID(srcNodeID)
+			srcHostNodeID     = source2host[srcNodeID]
+			srcRenderableID   = source2mapped[srcNodeID] // must exist
+			srcRenderableNode = nodes[srcRenderableID]   // must exist
 		)
-		if !ok1 || !ok2 {
+		if !ok {
 			log.Printf("bad adjacency ID %q", src)
 			continue
 		}
 
 		for _, dstNodeID := range dsts {
-			dstRenderableID, ok := address2mapped[dstNodeID]
+			dstRenderableID, ok := source2mapped[dstNodeID]
 			if !ok {
 				pseudoNode, ok := pseudoFunc(srcNodeID, srcRenderableNode, dstNodeID)
 				if !ok {
@@ -138,11 +143,11 @@ func (t Topology) RenderBy(mapFunc MapFunc, pseudoFunc PseudoFunc) RenderableNod
 					Pseudo:     true,
 					Metadata:   AggregateMetadata{}, // populated below - or not?
 				}
-				address2mapped[dstNodeID] = dstRenderableID
+				source2mapped[dstNodeID] = dstRenderableID
 			}
 
 			srcRenderableNode.Adjacency = srcRenderableNode.Adjacency.Add(dstRenderableID)
-			srcRenderableNode.Origins = srcRenderableNode.Origins.Add(MakeHostNodeID(srcOriginHostID))
+			srcRenderableNode.Origins = srcRenderableNode.Origins.Add(srcHostNodeID)
 			srcRenderableNode.Origins = srcRenderableNode.Origins.Add(srcNodeID)
 			edgeID := MakeEdgeID(srcNodeID, dstNodeID)
 			if md, ok := t.EdgeMetadatas[edgeID]; ok {

report/topology_test.go

@@ -19,6 +19,10 @@ var (
 	randomHostID  = "random.hostname.com"
 	unknownHostID = ""
 
+	clientHostNodeID = MakeHostNodeID(clientHostID)
+	serverHostNodeID = MakeHostNodeID(serverHostID)
+	randomHostNodeID = MakeHostNodeID(randomHostID)
+
 	client54001    = MakeEndpointNodeID(clientHostID, "10.10.10.20", "54001") // curl (1)
 	client54002    = MakeEndpointNodeID(clientHostID, "10.10.10.20", "54002") // curl (2)
 	unknownClient1 = MakeEndpointNodeID(serverHostID, "10.10.10.10", "54010") // we want to ensure two unknown clients, connnected
@@ -45,19 +49,22 @@ var (
 				// care to test into the fixture. Just be sure to include the bits
 				// that the mapping funcs extract :)
 				client54001: NodeMetadata{
-					"name":   "curl",
-					"domain": "client-54001-domain",
-					"pid":    "10001",
+					"name":     "curl",
+					"domain":   "client-54001-domain",
+					"pid":      "10001",
+					HostNodeID: clientHostNodeID,
 				},
 				client54002: NodeMetadata{
-					"name":   "curl",                // should be same as above!
-					"domain": "client-54002-domain", // may be different than above
-					"pid":    "10001",               // should be same as above!
+					"name":     "curl",                // should be same as above!
+					"domain":   "client-54002-domain", // may be different than above
+					"pid":      "10001",               // should be same as above!
+					HostNodeID: clientHostNodeID,
 				},
 				server80: NodeMetadata{
-					"name":   "apache",
-					"domain": "server-80-domain",
-					"pid":    "215",
+					"name":     "apache",
+					"domain":   "server-80-domain",
+					"pid":      "215",
+					HostNodeID: serverHostNodeID,
 				},
 			},
 			EdgeMetadatas: EdgeMetadatas{
@@ -107,13 +114,16 @@ var (
 			},
 			NodeMetadatas: NodeMetadatas{
 				clientIP: NodeMetadata{
-					"name": "client.hostname.com", // hostname
+					"name":     "client.hostname.com", // hostname
+					HostNodeID: clientHostNodeID,
 				},
 				randomIP: NodeMetadata{
-					"name": "random.hostname.com", // hostname
+					"name":     "random.hostname.com", // hostname
+					HostNodeID: randomHostNodeID,
 				},
 				serverIP: NodeMetadata{
-					"name": "server.hostname.com", // hostname
+					"name":     "server.hostname.com", // hostname
+					HostNodeID: serverHostNodeID,
 				},
 			},
 			EdgeMetadatas: EdgeMetadatas{