From 9dc50b5202511e5037fd76415f8f3ac2a6b899d5 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 00:01:36 +0100 Subject: [PATCH 1/9] refactor: hide "empty set" constants They are an implementation detail. --- extras/generate_latest_map | 3 +- probe/docker/container.go | 4 +-- probe/docker/container_test.go | 4 +-- probe/docker/registry_test.go | 4 +-- probe/docker/reporter.go | 4 +-- probe/docker/tagger.go | 4 +-- probe/host/reporter.go | 2 +- probe/host/tagger.go | 2 +- probe/kubernetes/reporter.go | 8 ++--- probe/kubernetes/reporter_test.go | 2 +- probe/overlay/weave.go | 2 +- render/container.go | 2 +- render/detailed/metadata_test.go | 2 +- render/detailed/parents_test.go | 2 +- render/detailed/tables_test.go | 2 +- render/short_lived_connections_test.go | 8 ++--- render/theinternet_test.go | 4 +-- report/counters.go | 7 ++-- report/counters_internal_test.go | 40 ++++++++++----------- report/edge_metadatas.go | 7 ++-- report/edge_metadatas_internal_test.go | 38 ++++++++++---------- report/id_list.go | 3 +- report/latest_map_generated.go | 10 +++--- report/latest_map_internal_test.go | 50 +++++++++++++------------- report/node.go | 16 ++++----- report/node_set.go | 7 ++-- report/node_set_test.go | 26 +++++++------- report/sets.go | 12 +++---- report/sets_internal_test.go | 4 +-- report/sets_test.go | 14 ++++---- report/string_set.go | 5 ++- test/fixture/report_fixture.go | 24 ++++++------- 32 files changed, 157 insertions(+), 165 deletions(-) diff --git a/extras/generate_latest_map b/extras/generate_latest_map index e73891925..0532b9f51 100755 --- a/extras/generate_latest_map +++ b/extras/generate_latest_map @@ -35,7 +35,7 @@ function generate_latest_map() { local lowercase_data_type="${data_type,}" local entry_type="${lowercase_data_type}LatestEntry" local latest_map_type="${uppercase_data_type}LatestMap" - local empty_latest_map_variable="Empty${latest_map_type}" + local empty_latest_map_variable="empty${latest_map_type}" local make_function="Make${latest_map_type}" # shellcheck disable=SC2016 @@ -63,7 +63,6 @@ function generate_latest_map() { // ${latest_map_type} holds latest ${data_type} instances. type ${latest_map_type} struct { ps.Map } - // ${empty_latest_map_variable} is an empty ${latest_map_type}. Start with this. var ${empty_latest_map_variable} = ${latest_map_type}{ps.NewMap()} // ${make_function} makes an empty ${latest_map_type}. diff --git a/probe/docker/container.go b/probe/docker/container.go index 19973bcf3..bc10f6bfd 100644 --- a/probe/docker/container.go +++ b/probe/docker/container.go @@ -303,7 +303,7 @@ func (c *container) NetworkInfo(localAddrs []net.IP) report.Sets { // Treat all Docker IPs as local scoped. ipsWithScopes := addScopeToIPs(c.hostID, ipv4s) - return report.EmptySets. + return report.MakeSets(). Add(ContainerNetworks, report.MakeStringSet(networks...)). Add(ContainerPorts, c.ports(localAddrs)). Add(ContainerIPs, report.MakeStringSet(ipv4s...)). @@ -388,7 +388,7 @@ func (c *container) getBaseNode() report.Node { ContainerCommand: c.getSanitizedCommand(), ImageID: c.Image(), ContainerHostname: c.Hostname(), - }).WithParents(report.EmptySets. + }).WithParents(report.MakeSets(). Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.Image()))), ) result = result.AddPrefixPropertyList(LabelPrefix, c.container.Config.Labels) diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index b042c70c9..728a997ad 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -86,7 +86,7 @@ func TestContainer(t *testing.T) { ).WithMetrics(report.Metrics{ "docker_cpu_total_usage": report.MakeMetric(nil), "docker_memory_usage": report.MakeSingletonMetric(now, 12345).WithMax(45678), - }).WithParents(report.EmptySets. + }).WithParents(report.MakeSets(). Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID("baz"))), ) @@ -102,7 +102,7 @@ func TestContainer(t *testing.T) { } { - want := report.EmptySets. + want := report.MakeSets(). Add("docker_container_ports", report.MakeStringSet("1.2.3.4:80->80/tcp", "81/tcp")). Add("docker_container_networks", nil). Add("docker_container_ips", report.MakeStringSet("1.2.3.4")). diff --git a/probe/docker/registry_test.go b/probe/docker/registry_test.go index 0fd6bcaa2..a77e55cb9 100644 --- a/probe/docker/registry_test.go +++ b/probe/docker/registry_test.go @@ -71,7 +71,7 @@ func (c *mockContainer) GetNode() report.Node { docker.ContainerID: c.c.ID, docker.ContainerName: c.c.Name, docker.ImageID: c.c.Image, - }).WithParents(report.EmptySets. + }).WithParents(report.MakeSets(). Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(c.c.Image))), ) } @@ -80,7 +80,7 @@ func (c *mockContainer) NetworkMode() (string, bool) { return "", false } func (c *mockContainer) NetworkInfo([]net.IP) report.Sets { - return report.EmptySets + return report.MakeSets() } func (c *mockContainer) Container() *client.Container { diff --git a/probe/docker/reporter.go b/probe/docker/reporter.go index db51eeb4d..a20aa7381 100644 --- a/probe/docker/reporter.go +++ b/probe/docker/reporter.go @@ -218,7 +218,7 @@ func (r *Reporter) containerTopology(localAddrs []net.IP) report.Topology { // namespaces & deal with containers in the host net namespace. This // is recursive to deal with people who decide to be clever. { - hostNetworkInfo := report.EmptySets + hostNetworkInfo := report.MakeSets() if hostIPs, err := getLocalIPs(); err == nil { hostIPsWithScopes := addScopeToIPs(r.hostID, hostIPs) hostNetworkInfo = hostNetworkInfo. @@ -230,7 +230,7 @@ func (r *Reporter) containerTopology(localAddrs []net.IP) report.Topology { networkInfo = func(prefix string) (ips report.Sets, isInHostNamespace bool) { container, ok := r.registry.GetContainerByPrefix(prefix) if !ok { - return report.EmptySets, false + return report.MakeSets(), false } networkMode, ok := container.NetworkMode() diff --git a/probe/docker/tagger.go b/probe/docker/tagger.go index 3bdf04f55..066b9ee97 100644 --- a/probe/docker/tagger.go +++ b/probe/docker/tagger.go @@ -117,7 +117,7 @@ func (t *Tagger) tag(tree process.Tree, topology *report.Topology) { node := report.MakeNodeWith(nodeID, map[string]string{ ContainerID: c.ID(), - }).WithParents(report.EmptySets. + }).WithParents(report.MakeSets(). Add(report.Container, report.MakeStringSet(report.MakeContainerNodeID(c.ID()))), ) @@ -125,7 +125,7 @@ func (t *Tagger) tag(tree process.Tree, topology *report.Topology) { image, ok := t.registry.GetContainerImage(c.Image()) if ok && len(image.RepoTags) > 0 { imageName := ImageNameWithoutVersion(image.RepoTags[0]) - node = node.WithParents(report.EmptySets. + node = node.WithParents(report.MakeSets(). Add(report.ContainerImage, report.MakeStringSet(report.MakeContainerImageNodeID(imageName))), ) } diff --git a/probe/host/reporter.go b/probe/host/reporter.go index 1f0ab5ce2..7c4d44de1 100644 --- a/probe/host/reporter.go +++ b/probe/host/reporter.go @@ -133,7 +133,7 @@ func (r *Reporter) Report() (report.Report, error) { Uptime: uptime.String(), ScopeVersion: r.version, }). - WithSets(report.EmptySets. + WithSets(report.MakeSets(). Add(LocalNetworks, report.MakeStringSet(localCIDRs...)), ). WithMetrics(metrics). diff --git a/probe/host/tagger.go b/probe/host/tagger.go index 2896e9215..88eee393a 100644 --- a/probe/host/tagger.go +++ b/probe/host/tagger.go @@ -26,7 +26,7 @@ func (Tagger) Name() string { return "Host" } func (t Tagger) Tag(r report.Report) (report.Report, error) { var ( metadata = map[string]string{report.HostNodeID: t.hostNodeID} - parents = report.EmptySets.Add(report.Host, report.MakeStringSet(t.hostNodeID)) + parents = report.MakeSets().Add(report.Host, report.MakeStringSet(t.hostNodeID)) ) // Explicitly don't tag Endpoints, Addresses and Overlay nodes - These topologies include pseudo nodes, diff --git a/probe/kubernetes/reporter.go b/probe/kubernetes/reporter.go index a0db79273..f0ca9a15d 100644 --- a/probe/kubernetes/reporter.go +++ b/probe/kubernetes/reporter.go @@ -197,9 +197,9 @@ func (r *Reporter) Tag(rpt report.Report) (report.Report, error) { n = n.WithLatest(report.DoesNotMakeConnections, mtime.Now(), "") } - rpt.Container.Nodes[id] = n.WithParents(report.EmptySets.Add( + rpt.Container.Nodes[id] = n.WithParents(report.MakeSets().Add( report.Pod, - report.EmptyStringSet.Add(report.MakePodNodeID(uid)), + report.MakeStringSet().Add(report.MakePodNodeID(uid)), )) } return rpt, nil @@ -266,12 +266,12 @@ func (r *Reporter) serviceTopology() (report.Topology, []Service, error) { // connections for which we don't have a robust solution // (see https://github.com/weaveworks/scope/issues/1491) func (r *Reporter) hostTopology(services []Service) report.Topology { - localNetworks := report.EmptyStringSet + localNetworks := report.MakeStringSet() for _, service := range services { localNetworks = localNetworks.Add(service.ClusterIP() + "/32") } node := report.MakeNode(report.MakeHostNodeID(r.hostID)) - node = node.WithSets(report.EmptySets. + node = node.WithSets(report.MakeSets(). Add(host.LocalNetworks, localNetworks)) return report.MakeTopology().AddNode(node) } diff --git a/probe/kubernetes/reporter_test.go b/probe/kubernetes/reporter_test.go index 4abc62c60..9172a2662 100644 --- a/probe/kubernetes/reporter_test.go +++ b/probe/kubernetes/reporter_test.go @@ -264,7 +264,7 @@ func TestTagger(t *testing.T) { } have, ok := rpt.Container.Nodes["container1"].Parents.Lookup(report.Pod) - want := report.EmptyStringSet.Add(report.MakePodNodeID("123456")) + want := report.MakeStringSet().Add(report.MakePodNodeID("123456")) if !ok || !reflect.DeepEqual(have, want) { t.Errorf("Expected container to have pod parent %v %v", have, want) } diff --git a/probe/overlay/weave.go b/probe/overlay/weave.go index 1840cc4e6..4c18e1508 100644 --- a/probe/overlay/weave.go +++ b/probe/overlay/weave.go @@ -472,7 +472,7 @@ func (w *Weave) addCurrentPeerInfo(latests map[string]string, node report.Node) latests[WeavePluginDriver] = "weave" } node = node.AddPrefixMulticolumnTable(WeaveConnectionsMulticolumnTablePrefix, getConnectionsTable(w.statusCache.Router)) - node = node.WithParents(report.EmptySets.Add(report.Host, report.MakeStringSet(w.hostID))) + node = node.WithParents(report.MakeSets().Add(report.Host, report.MakeStringSet(w.hostID))) return latests, node } diff --git a/render/container.go b/render/container.go index e50673ad1..74f4e3d58 100644 --- a/render/container.go +++ b/render/container.go @@ -365,7 +365,7 @@ func MapContainerImage2Name(n report.Node, _ report.Networks) report.Nodes { n.ID = report.MakeContainerImageNodeID(imageNameWithoutVersion) if imageID, ok := report.ParseContainerImageNodeID(n.ID); ok { - n.Sets = n.Sets.Add(docker.ImageID, report.EmptyStringSet.Add(imageID)) + n.Sets = n.Sets.Add(docker.ImageID, report.MakeStringSet().Add(imageID)) } return report.Nodes{n.ID: n} diff --git a/render/detailed/metadata_test.go b/render/detailed/metadata_test.go index 7698e9693..ae7715cb3 100644 --- a/render/detailed/metadata_test.go +++ b/render/detailed/metadata_test.go @@ -23,7 +23,7 @@ func TestNodeMetadata(t *testing.T) { docker.ContainerID: fixture.ClientContainerID, docker.LabelPrefix + "label1": "label1value", docker.ContainerStateHuman: docker.StateRunning, - }).WithTopology(report.Container).WithSets(report.EmptySets. + }).WithTopology(report.Container).WithSets(report.MakeSets(). Add(docker.ContainerIPs, report.MakeStringSet("10.10.10.0/24", "10.10.10.1/24")), ), want: []report.MetadataRow{ diff --git a/render/detailed/parents_test.go b/render/detailed/parents_test.go index 6cc79b0b2..b4d18bb23 100644 --- a/render/detailed/parents_test.go +++ b/render/detailed/parents_test.go @@ -22,7 +22,7 @@ func TestParents(t *testing.T) { { name: "Node accidentally tagged with itself", node: render.HostRenderer.Render(fixture.Report, nil)[fixture.ClientHostNodeID].WithParents( - report.EmptySets.Add(report.Host, report.MakeStringSet(fixture.ClientHostNodeID)), + report.MakeSets().Add(report.Host, report.MakeStringSet(fixture.ClientHostNodeID)), ), want: nil, }, diff --git a/render/detailed/tables_test.go b/render/detailed/tables_test.go index ab847d276..1ae83afd4 100644 --- a/render/detailed/tables_test.go +++ b/render/detailed/tables_test.go @@ -28,7 +28,7 @@ func TestNodeTables(t *testing.T) { docker.ContainerID: fixture.ClientContainerID, docker.LabelPrefix + "label1": "label1value", docker.ContainerState: docker.StateRunning, - }).WithTopology(report.Container).WithSets(report.EmptySets. + }).WithTopology(report.Container).WithSets(report.MakeSets(). Add(docker.ContainerIPs, report.MakeStringSet("10.10.10.0/24", "10.10.10.1/24")), ), want: []report.Table{ diff --git a/render/short_lived_connections_test.go b/render/short_lived_connections_test.go index 77740da15..629d90897 100644 --- a/render/short_lived_connections_test.go +++ b/render/short_lived_connections_test.go @@ -79,7 +79,7 @@ var ( docker.ContainerName: container1Name, report.HostNodeID: serverHostNodeID, }). - WithSets(report.EmptySets. + WithSets(report.MakeSets(). Add(docker.ContainerIPs, report.MakeStringSet(container1IP)). Add(docker.ContainerIPsWithScopes, report.MakeStringSet(report.MakeAddressNodeID("", container1IP))). Add(docker.ContainerPorts, report.MakeStringSet(fmt.Sprintf("%s:%s->%s/tcp", serverIP, serverPort, serverPort))), @@ -89,7 +89,7 @@ var ( docker.ContainerName: container2Name, report.HostNodeID: serverHostNodeID, }). - WithSets(report.EmptySets. + WithSets(report.MakeSets(). Add(docker.ContainerIPs, report.MakeStringSet(container2IP)). Add(docker.ContainerIPsWithScopes, report.MakeStringSet(report.MakeAddressNodeID("", container2IP))), ).WithTopology(report.Container), @@ -98,7 +98,7 @@ var ( docker.ContainerName: pauseContainerName, report.HostNodeID: serverHostNodeID, }). - WithSets(report.EmptySets. + WithSets(report.MakeSets(). Add(docker.ContainerIPs, report.MakeStringSet(pauseContainerIP)). Add(docker.ContainerIPsWithScopes, report.MakeStringSet(report.MakeAddressNodeID("", pauseContainerIP))), ).WithTopology(report.Container).WithLatest(report.DoesNotMakeConnections, mtime.Now(), ""), @@ -109,7 +109,7 @@ var ( serverHostNodeID: report.MakeNodeWith(serverHostNodeID, map[string]string{ report.HostNodeID: serverHostNodeID, }). - WithSets(report.EmptySets. + WithSets(report.MakeSets(). Add(host.LocalNetworks, report.MakeStringSet("192.168.0.0/16")), ).WithTopology(report.Host), }, diff --git a/render/theinternet_test.go b/render/theinternet_test.go index 1cb540cd1..b91a7fcab 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -15,7 +15,7 @@ func TestReportLocalNetworks(t *testing.T) { Host: report.Topology{ Nodes: report.Nodes{ "nonets": report.MakeNode("nonets"), - "foo": report.MakeNode("foo").WithSets(report.EmptySets. + "foo": report.MakeNode("foo").WithSets(report.MakeSets(). Add(host.LocalNetworks, report.MakeStringSet( "10.0.0.1/8", "192.168.1.1/24", "10.0.0.1/8", "badnet/33")), ), @@ -23,7 +23,7 @@ func TestReportLocalNetworks(t *testing.T) { }, Overlay: report.Topology{ Nodes: report.Nodes{ - "router": report.MakeNode("router").WithSets(report.EmptySets. + "router": report.MakeNode("router").WithSets(report.MakeSets(). Add(host.LocalNetworks, report.MakeStringSet("10.32.0.1/12")), ), }, diff --git a/report/counters.go b/report/counters.go index 660c32af4..f75493dbe 100644 --- a/report/counters.go +++ b/report/counters.go @@ -14,12 +14,11 @@ type Counters struct { psMap ps.Map } -// EmptyCounters is the set of empty counters. -var EmptyCounters = Counters{ps.NewMap()} +var emptyCounters = Counters{ps.NewMap()} // MakeCounters returns EmptyCounters func MakeCounters() Counters { - return EmptyCounters + return emptyCounters } // Copy is a noop @@ -30,7 +29,7 @@ func (c Counters) Copy() Counters { // Add value to the counter 'key' func (c Counters) Add(key string, value int) Counters { if c.psMap == nil { - c = EmptyCounters + c = emptyCounters } if existingValue, ok := c.psMap.Lookup(key); ok { value += existingValue.(int) diff --git a/report/counters_internal_test.go b/report/counters_internal_test.go index 731a85a68..36fe42a1b 100644 --- a/report/counters_internal_test.go +++ b/report/counters_internal_test.go @@ -11,7 +11,7 @@ import ( ) func TestCountersAdd(t *testing.T) { - have := EmptyCounters. + have := MakeCounters(). Add("foo", 1). Add("foo", 2) if v, ok := have.Lookup("foo"); !ok || v != 3 { @@ -23,14 +23,14 @@ func TestCountersAdd(t *testing.T) { } func TestCountersDeepEquals(t *testing.T) { - want := EmptyCounters. + want := MakeCounters(). Add("foo", 3) - have := EmptyCounters. + have := MakeCounters(). Add("foo", 3) if !reflect.DeepEqual(want, have) { t.Errorf(test.Diff(want, have)) } - notequal := EmptyCounters. + notequal := MakeCounters(). Add("foo", 4) if reflect.DeepEqual(want, notequal) { t.Errorf(test.Diff(want, have)) @@ -59,34 +59,34 @@ func TestCountersMerge(t *testing.T) { a, b, want Counters }{ "Empty a": { - a: EmptyCounters, - b: EmptyCounters. + a: MakeCounters(), + b: MakeCounters(). Add("foo", 1), - want: EmptyCounters. + want: MakeCounters(). Add("foo", 1), }, "Empty b": { - a: EmptyCounters. + a: MakeCounters(). Add("foo", 1), - b: EmptyCounters, - want: EmptyCounters. + b: MakeCounters(), + want: MakeCounters(). Add("foo", 1), }, "Disjoin a & b": { - a: EmptyCounters. + a: MakeCounters(). Add("foo", 1), - b: EmptyCounters. + b: MakeCounters(). Add("bar", 2), - want: EmptyCounters. + want: MakeCounters(). Add("foo", 1). Add("bar", 2), }, "Overlapping a & b": { - a: EmptyCounters. + a: MakeCounters(). Add("foo", 1), - b: EmptyCounters. + b: MakeCounters(). Add("foo", 2), - want: EmptyCounters. + want: MakeCounters(). Add("foo", 3), }, } { @@ -97,7 +97,7 @@ func TestCountersMerge(t *testing.T) { } func TestCountersEncoding(t *testing.T) { - want := EmptyCounters. + want := MakeCounters(). Add("foo", 1). Add("bar", 2) @@ -111,7 +111,7 @@ func TestCountersEncoding(t *testing.T) { encoder := codec.NewEncoder(buf, h) want.CodecEncodeSelf(encoder) decoder := codec.NewDecoder(buf, h) - have := EmptyCounters + have := MakeCounters() have.CodecDecodeSelf(decoder) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -132,7 +132,7 @@ func TestCountersString(t *testing.T) { } { - have := EmptyCounters.String() + have := MakeCounters().String() want := `{}` if want != have { t.Errorf("Expected: %s, Got %s", want, have) @@ -140,7 +140,7 @@ func TestCountersString(t *testing.T) { } { - have := EmptyCounters. + have := MakeCounters(). Add("foo", 1). Add("bar", 2).String() diff --git a/report/edge_metadatas.go b/report/edge_metadatas.go index 24b206401..5300fa143 100644 --- a/report/edge_metadatas.go +++ b/report/edge_metadatas.go @@ -15,12 +15,11 @@ type EdgeMetadatas struct { psMap ps.Map } -// EmptyEdgeMetadatas is the set of empty EdgeMetadatas. -var EmptyEdgeMetadatas = EdgeMetadatas{ps.NewMap()} +var emptyEdgeMetadatas = EdgeMetadatas{ps.NewMap()} // MakeEdgeMetadatas returns EmptyEdgeMetadatas func MakeEdgeMetadatas() EdgeMetadatas { - return EmptyEdgeMetadatas + return emptyEdgeMetadatas } // Copy is a noop @@ -31,7 +30,7 @@ func (c EdgeMetadatas) Copy() EdgeMetadatas { // Add value to the counter 'key' func (c EdgeMetadatas) Add(key string, value EdgeMetadata) EdgeMetadatas { if c.psMap == nil { - c = EmptyEdgeMetadatas + c = emptyEdgeMetadatas } if existingValue, ok := c.psMap.Lookup(key); ok { value = value.Merge(existingValue.(EdgeMetadata)) diff --git a/report/edge_metadatas_internal_test.go b/report/edge_metadatas_internal_test.go index b9e36d6a3..094998658 100644 --- a/report/edge_metadatas_internal_test.go +++ b/report/edge_metadatas_internal_test.go @@ -11,7 +11,7 @@ import ( ) func TestEdgeMetadatasAdd(t *testing.T) { - have := EmptyEdgeMetadatas. + have := MakeEdgeMetadatas(). Add("foo", EdgeMetadata{ EgressPacketCount: newu64(1), @@ -41,12 +41,12 @@ func TestEdgeMetadatasAddNil(t *testing.T) { } func TestEdgeMetadatasDeepEquals(t *testing.T) { - want := EmptyEdgeMetadatas. + want := MakeEdgeMetadatas(). Add("foo", EdgeMetadata{ EgressPacketCount: newu64(3), }) - have := EmptyEdgeMetadatas. + have := MakeEdgeMetadatas(). Add("foo", EdgeMetadata{ EgressPacketCount: newu64(3), @@ -66,27 +66,27 @@ func TestEdgeMetadatasMerge(t *testing.T) { want: EdgeMetadatas{}, }, "Empty a": { - a: EmptyEdgeMetadatas, - b: EmptyEdgeMetadatas. + a: MakeEdgeMetadatas(), + b: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(1), }), - want: EmptyEdgeMetadatas. + want: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(1), }), }, "Empty b": { - a: EmptyEdgeMetadatas. + a: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(12), EgressByteCount: newu64(999), }), - b: EmptyEdgeMetadatas, - want: EmptyEdgeMetadatas. + b: MakeEdgeMetadatas(), + want: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(12), @@ -94,19 +94,19 @@ func TestEdgeMetadatasMerge(t *testing.T) { }), }, "Disjoint a & b": { - a: EmptyEdgeMetadatas. + a: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(12), EgressByteCount: newu64(500), }), - b: EmptyEdgeMetadatas. + b: MakeEdgeMetadatas(). Add("hostQ|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(1), EgressByteCount: newu64(2), }), - want: EmptyEdgeMetadatas. + want: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(12), @@ -119,20 +119,20 @@ func TestEdgeMetadatasMerge(t *testing.T) { }), }, "Overlapping a & b": { - a: EmptyEdgeMetadatas. + a: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(12), EgressByteCount: newu64(1000), }), - b: EmptyEdgeMetadatas. + b: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(1), IngressByteCount: newu64(123), EgressByteCount: newu64(2), }), - want: EmptyEdgeMetadatas. + want: MakeEdgeMetadatas(). Add("hostA|:192.168.1.1:12345|:192.168.1.2:80", EdgeMetadata{ EgressPacketCount: newu64(13), @@ -168,7 +168,7 @@ func TestEdgeMetadataFlatten(t *testing.T) { // Test an EdgeMetadatas flatten to the correct value (should // just sum) { - have := EmptyEdgeMetadatas. + have := MakeEdgeMetadatas(). Add("foo", EdgeMetadata{ EgressPacketCount: newu64(1), }). @@ -206,7 +206,7 @@ func TestEdgeMetadataReversed(t *testing.T) { } func TestEdgeMetadatasEncoding(t *testing.T) { - want := EmptyEdgeMetadatas. + want := MakeEdgeMetadatas(). Add("foo", EdgeMetadata{ EgressPacketCount: newu64(1), }). @@ -223,7 +223,7 @@ func TestEdgeMetadatasEncoding(t *testing.T) { encoder := codec.NewEncoder(buf, h) want.CodecEncodeSelf(encoder) decoder := codec.NewDecoder(buf, h) - have := EmptyEdgeMetadatas + have := MakeEdgeMetadatas() have.CodecDecodeSelf(decoder) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -245,7 +245,7 @@ func TestEdgeMetadatasEncodingNil(t *testing.T) { encoder := codec.NewEncoder(buf, h) want.CodecEncodeSelf(encoder) decoder := codec.NewDecoder(buf, h) - have := EmptyEdgeMetadatas + have := MakeEdgeMetadatas() have.CodecDecodeSelf(decoder) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/report/id_list.go b/report/id_list.go index c9778bbcd..b285d5dbf 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -3,8 +3,7 @@ package report // IDList is a list of string IDs, which are always sorted and unique. type IDList StringSet -// EmptyIDList is an Empty ID List. -var EmptyIDList = IDList(EmptyStringSet) +var emptyIDList = IDList(MakeStringSet()) // MakeIDList makes a new IDList. func MakeIDList(ids ...string) IDList { diff --git a/report/latest_map_generated.go b/report/latest_map_generated.go index c17b318e6..7e1c2b53b 100644 --- a/report/latest_map_generated.go +++ b/report/latest_map_generated.go @@ -30,12 +30,11 @@ func (e *stringLatestEntry) Equal(e2 *stringLatestEntry) bool { // StringLatestMap holds latest string instances. type StringLatestMap struct{ ps.Map } -// EmptyStringLatestMap is an empty StringLatestMap. Start with this. -var EmptyStringLatestMap = StringLatestMap{ps.NewMap()} +var emptyStringLatestMap = StringLatestMap{ps.NewMap()} // MakeStringLatestMap makes an empty StringLatestMap. func MakeStringLatestMap() StringLatestMap { - return EmptyStringLatestMap + return emptyStringLatestMap } // Copy is a noop, as StringLatestMaps are immutable. @@ -170,12 +169,11 @@ func (e *nodeControlDataLatestEntry) Equal(e2 *nodeControlDataLatestEntry) bool // NodeControlDataLatestMap holds latest NodeControlData instances. type NodeControlDataLatestMap struct{ ps.Map } -// EmptyNodeControlDataLatestMap is an empty NodeControlDataLatestMap. Start with this. -var EmptyNodeControlDataLatestMap = NodeControlDataLatestMap{ps.NewMap()} +var emptyNodeControlDataLatestMap = NodeControlDataLatestMap{ps.NewMap()} // MakeNodeControlDataLatestMap makes an empty NodeControlDataLatestMap. func MakeNodeControlDataLatestMap() NodeControlDataLatestMap { - return EmptyNodeControlDataLatestMap + return emptyNodeControlDataLatestMap } // Copy is a noop, as NodeControlDataLatestMaps are immutable. diff --git a/report/latest_map_internal_test.go b/report/latest_map_internal_test.go index 653ba3bf7..8615fe1d4 100644 --- a/report/latest_map_internal_test.go +++ b/report/latest_map_internal_test.go @@ -14,7 +14,7 @@ import ( func TestLatestMapAdd(t *testing.T) { now := time.Now() - have := EmptyStringLatestMap. + have := MakeStringLatestMap(). Set("foo", now.Add(-1), "Baz"). Set("foo", now, "Bar") if v, ok := have.Lookup("foo"); !ok || v != "Bar" { @@ -37,7 +37,7 @@ func TestLatestMapLookupEntry(t *testing.T) { Value interface{} } entry := LatestEntry{Timestamp: now, Value: "Bar"} - have := EmptyStringLatestMap.Set("foo", entry.Timestamp, entry.Value.(string)) + have := MakeStringLatestMap().Set("foo", entry.Timestamp, entry.Value.(string)) if got, timestamp, ok := have.LookupEntry("foo"); !ok || got != entry.Value || !timestamp.Equal(entry.Timestamp) { t.Errorf("got: %#v %v != expected %#v", got, timestamp, entry) } @@ -56,14 +56,14 @@ func TestLatestMapAddNil(t *testing.T) { func TestLatestMapDeepEquals(t *testing.T) { now := time.Now() - want := EmptyStringLatestMap. + want := MakeStringLatestMap(). Set("foo", now, "Bar") - have := EmptyStringLatestMap. + have := MakeStringLatestMap(). Set("foo", now, "Bar") if !reflect.DeepEqual(want, have) { t.Errorf(test.Diff(want, have)) } - notequal := EmptyStringLatestMap. + notequal := MakeStringLatestMap(). Set("foo", now, "Baz") if reflect.DeepEqual(want, notequal) { t.Errorf(test.Diff(want, have)) @@ -72,8 +72,8 @@ func TestLatestMapDeepEquals(t *testing.T) { func TestLatestMapDelete(t *testing.T) { now := time.Now() - want := EmptyStringLatestMap - have := EmptyStringLatestMap. + want := MakeStringLatestMap() + have := MakeStringLatestMap(). Set("foo", now, "Baz"). Delete("foo") if !reflect.DeepEqual(want, have) { @@ -90,7 +90,7 @@ func TestLatestMapDeleteNil(t *testing.T) { } func nilStringLatestMap() StringLatestMap { - m := EmptyStringLatestMap + m := MakeStringLatestMap() m.Map = nil return m } @@ -108,34 +108,34 @@ func TestLatestMapMerge(t *testing.T) { want: nilStringLatestMap(), }, "Empty a": { - a: EmptyStringLatestMap, - b: EmptyStringLatestMap. + a: MakeStringLatestMap(), + b: MakeStringLatestMap(). Set("foo", now, "bar"), - want: EmptyStringLatestMap. + want: MakeStringLatestMap(). Set("foo", now, "bar"), }, "Empty b": { - a: EmptyStringLatestMap. + a: MakeStringLatestMap(). Set("foo", now, "bar"), - b: EmptyStringLatestMap, - want: EmptyStringLatestMap. + b: MakeStringLatestMap(), + want: MakeStringLatestMap(). Set("foo", now, "bar"), }, "Disjoint a & b": { - a: EmptyStringLatestMap. + a: MakeStringLatestMap(). Set("foo", now, "bar"), - b: EmptyStringLatestMap. + b: MakeStringLatestMap(). Set("baz", now, "bop"), - want: EmptyStringLatestMap. + want: MakeStringLatestMap(). Set("foo", now, "bar"). Set("baz", now, "bop"), }, "Common a & b": { - a: EmptyStringLatestMap. + a: MakeStringLatestMap(). Set("foo", now, "bar"), - b: EmptyStringLatestMap. + b: MakeStringLatestMap(). Set("foo", then, "baz"), - want: EmptyStringLatestMap. + want: MakeStringLatestMap(). Set("foo", now, "bar"), }, } { @@ -147,8 +147,8 @@ func TestLatestMapMerge(t *testing.T) { func BenchmarkLatestMapMerge(b *testing.B) { var ( - left = EmptyStringLatestMap - right = EmptyStringLatestMap + left = MakeStringLatestMap() + right = MakeStringLatestMap() now = time.Now() ) @@ -169,7 +169,7 @@ func BenchmarkLatestMapMerge(b *testing.B) { func TestLatestMapEncoding(t *testing.T) { now := time.Now() - want := EmptyStringLatestMap. + want := MakeStringLatestMap(). Set("foo", now, "bar"). Set("bar", now, "baz") @@ -181,7 +181,7 @@ func TestLatestMapEncoding(t *testing.T) { encoder := codec.NewEncoder(buf, h) want.CodecEncodeSelf(encoder) decoder := codec.NewDecoder(buf, h) - have := EmptyStringLatestMap + have := MakeStringLatestMap() have.CodecDecodeSelf(decoder) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) @@ -201,7 +201,7 @@ func TestLatestMapEncodingNil(t *testing.T) { encoder := codec.NewEncoder(buf, h) want.CodecEncodeSelf(encoder) decoder := codec.NewDecoder(buf, h) - have := EmptyStringLatestMap + have := MakeStringLatestMap() have.CodecDecodeSelf(decoder) if !reflect.DeepEqual(want, have) { t.Error(test.Diff(want, have)) diff --git a/report/node.go b/report/node.go index 3ad71c6d3..db22873fc 100644 --- a/report/node.go +++ b/report/node.go @@ -28,15 +28,15 @@ type Node struct { func MakeNode(id string) Node { return Node{ ID: id, - Counters: EmptyCounters, - Sets: EmptySets, - Adjacency: EmptyIDList, - Edges: EmptyEdgeMetadatas, + Counters: MakeCounters(), + Sets: MakeSets(), + Adjacency: MakeIDList(), + Edges: MakeEdgeMetadatas(), Controls: MakeNodeControls(), - LatestControls: EmptyNodeControlDataLatestMap, - Latest: EmptyStringLatestMap, + LatestControls: MakeNodeControlDataLatestMap(), + Latest: MakeStringLatestMap(), Metrics: Metrics{}, - Parents: EmptySets, + Parents: MakeSets(), } } @@ -170,7 +170,7 @@ func (n Node) WithParents(parents Sets) Node { // PruneParents returns a fresh copy of n, without any parents. func (n Node) PruneParents() Node { - n.Parents = EmptySets + n.Parents = MakeSets() return n } diff --git a/report/node_set.go b/report/node_set.go index e7a1f9219..1baf2152c 100644 --- a/report/node_set.go +++ b/report/node_set.go @@ -17,12 +17,11 @@ type NodeSet struct { psMap ps.Map } -// EmptyNodeSet is the empty set of nodes. -var EmptyNodeSet = NodeSet{ps.NewMap()} +var emptyNodeSet = NodeSet{ps.NewMap()} // MakeNodeSet makes a new NodeSet with the given nodes. func MakeNodeSet(nodes ...Node) NodeSet { - return EmptyNodeSet.Add(nodes...) + return emptyNodeSet.Add(nodes...) } // Add adds the nodes to the NodeSet. Add is the only valid way to grow a @@ -49,7 +48,7 @@ func (n NodeSet) Delete(ids ...string) NodeSet { result = result.Delete(id) } if result.Size() == 0 { - return EmptyNodeSet + return emptyNodeSet } return NodeSet{result} } diff --git a/report/node_set_test.go b/report/node_set_test.go index 28c396c5e..42f88bfab 100644 --- a/report/node_set_test.go +++ b/report/node_set_test.go @@ -76,9 +76,9 @@ func TestNodeSetAdd(t *testing.T) { want: report.NodeSet{}, }, { - input: report.EmptyNodeSet, + input: report.MakeNodeSet(), nodes: []report.Node{}, - want: report.EmptyNodeSet, + want: report.MakeNodeSet(), }, { input: report.MakeNodeSet(report.MakeNode("a")), @@ -86,7 +86,7 @@ func TestNodeSetAdd(t *testing.T) { want: report.MakeNodeSet(report.MakeNode("a")), }, { - input: report.EmptyNodeSet, + input: report.MakeNodeSet(), nodes: []report.Node{report.MakeNode("a")}, want: report.MakeNodeSet(report.MakeNode("a")), }, @@ -146,7 +146,7 @@ func TestNodeSetAdd(t *testing.T) { } func BenchmarkNodeSetAdd(b *testing.B) { - n := report.EmptyNodeSet + n := report.MakeNodeSet() for i := 0; i < 600; i++ { n = n.Add( report.MakeNodeWith(fmt.Sprint(i), map[string]string{ @@ -181,9 +181,9 @@ func TestNodeSetDelete(t *testing.T) { want: report.NodeSet{}, }, { - input: report.EmptyNodeSet, + input: report.MakeNodeSet(), nodes: []string{}, - want: report.EmptyNodeSet, + want: report.MakeNodeSet(), }, { input: report.MakeNodeSet(report.MakeNode("a")), @@ -191,19 +191,19 @@ func TestNodeSetDelete(t *testing.T) { want: report.MakeNodeSet(report.MakeNode("a")), }, { - input: report.EmptyNodeSet, + input: report.MakeNodeSet(), nodes: []string{"a"}, - want: report.EmptyNodeSet, + want: report.MakeNodeSet(), }, { input: report.MakeNodeSet(report.MakeNode("a")), nodes: []string{"a"}, - want: report.EmptyNodeSet, + want: report.MakeNodeSet(), }, { input: report.MakeNodeSet(report.MakeNode("b")), nodes: []string{"a", "b"}, - want: report.EmptyNodeSet, + want: report.MakeNodeSet(), }, { input: report.MakeNodeSet(report.MakeNode("a")), @@ -233,14 +233,14 @@ func TestNodeSetMerge(t *testing.T) { want report.NodeSet }{ {input: report.NodeSet{}, other: report.NodeSet{}, want: report.NodeSet{}}, - {input: report.EmptyNodeSet, other: report.EmptyNodeSet, want: report.EmptyNodeSet}, + {input: report.MakeNodeSet(), other: report.MakeNodeSet(), want: report.MakeNodeSet()}, { input: report.MakeNodeSet(report.MakeNode("a")), - other: report.EmptyNodeSet, + other: report.MakeNodeSet(), want: report.MakeNodeSet(report.MakeNode("a")), }, { - input: report.EmptyNodeSet, + input: report.MakeNodeSet(), other: report.MakeNodeSet(report.MakeNode("a")), want: report.MakeNodeSet(report.MakeNode("a")), }, diff --git a/report/sets.go b/report/sets.go index 59e67270c..d73d5d7bf 100644 --- a/report/sets.go +++ b/report/sets.go @@ -14,11 +14,11 @@ type Sets struct { } // EmptySets is an empty Sets. Starts with this. -var EmptySets = Sets{ps.NewMap()} +var emptySets = Sets{ps.NewMap()} // MakeSets returns EmptySets func MakeSets() Sets { - return EmptySets + return emptySets } // Keys returns the keys for this set @@ -32,7 +32,7 @@ func (s Sets) Keys() []string { // Add the given value to the Sets. func (s Sets) Add(key string, value StringSet) Sets { if s.psMap == nil { - s = EmptySets + s = emptySets } if existingValue, ok := s.psMap.Lookup(key); ok { value = value.Merge(existingValue.(StringSet)) @@ -45,7 +45,7 @@ func (s Sets) Add(key string, value StringSet) Sets { // Delete the given set from the Sets. func (s Sets) Delete(key string) Sets { if s.psMap == nil { - return EmptySets + return emptySets } return Sets{ psMap: s.psMap.Delete(key), @@ -55,12 +55,12 @@ func (s Sets) Delete(key string) Sets { // Lookup returns the sets stored under key. func (s Sets) Lookup(key string) (StringSet, bool) { if s.psMap == nil { - return EmptyStringSet, false + return MakeStringSet(), false } if value, ok := s.psMap.Lookup(key); ok { return value.(StringSet), true } - return EmptyStringSet, false + return MakeStringSet(), false } // Size returns the number of elements diff --git a/report/sets_internal_test.go b/report/sets_internal_test.go index 4c4253a04..eef3e9235 100644 --- a/report/sets_internal_test.go +++ b/report/sets_internal_test.go @@ -7,12 +7,12 @@ import ( ) func TestSets(t *testing.T) { - sets := EmptySets.Add("foo", MakeStringSet("bar")) + sets := MakeSets().Add("foo", MakeStringSet("bar")) if v, _ := sets.Lookup("foo"); !reflect.DeepEqual(v, MakeStringSet("bar")) { t.Fatal(v) } - sets = sets.Merge(EmptySets.Add("foo", MakeStringSet("baz"))) + sets = sets.Merge(MakeSets().Add("foo", MakeStringSet("baz"))) if v, _ := sets.Lookup("foo"); !reflect.DeepEqual(v, MakeStringSet("bar", "baz")) { t.Fatal(v) } diff --git a/report/sets_test.go b/report/sets_test.go index 31980ffa1..c2ac164fb 100644 --- a/report/sets_test.go +++ b/report/sets_test.go @@ -12,20 +12,20 @@ func TestSetsMerge(t *testing.T) { a, b report.Sets want map[string][]string }{ - {report.EmptySets, report.EmptySets, map[string][]string{}}, + {report.MakeSets(), report.MakeSets(), map[string][]string{}}, { - report.EmptySets, - report.EmptySets.Add("a", report.MakeStringSet("b")), + report.MakeSets(), + report.MakeSets().Add("a", report.MakeStringSet("b")), map[string][]string{"a": {"b"}}, }, { - report.EmptySets, - report.EmptySets.Add("a", report.MakeStringSet("b", "c")), + report.MakeSets(), + report.MakeSets().Add("a", report.MakeStringSet("b", "c")), map[string][]string{"a": {"b", "c"}}, }, { - report.EmptySets.Add("a", report.MakeStringSet("1")).Add("b", report.MakeStringSet("2")), - report.EmptySets.Add("c", report.MakeStringSet("3")).Add("b", report.MakeStringSet("3")), + report.MakeSets().Add("a", report.MakeStringSet("1")).Add("b", report.MakeStringSet("2")), + report.MakeSets().Add("c", report.MakeStringSet("3")).Add("b", report.MakeStringSet("3")), map[string][]string{"a": {"1"}, "b": {"2", "3"}, "c": {"3"}}, }, } { diff --git a/report/string_set.go b/report/string_set.go index 42f02cea8..d0b63a50f 100644 --- a/report/string_set.go +++ b/report/string_set.go @@ -8,8 +8,7 @@ import ( // method to add strings. type StringSet []string -// EmptyStringSet is an empty string set. -var EmptyStringSet StringSet +var emptyStringSet StringSet // MakeStringSet makes a new StringSet with the given strings. func MakeStringSet(strs ...string) StringSet { @@ -37,7 +36,7 @@ func (s StringSet) Contains(str string) bool { // Intersection returns the intersections of a and b func (s StringSet) Intersection(b StringSet) StringSet { - result, i, j := EmptyStringSet, 0, 0 + result, i, j := emptyStringSet, 0, 0 for i < len(s) && j < len(b) { if s[i] == b[j] { result = result.Add(s[i]) diff --git a/test/fixture/report_fixture.go b/test/fixture/report_fixture.go index 106536005..982bc13fa 100644 --- a/test/fixture/report_fixture.go +++ b/test/fixture/report_fixture.go @@ -199,7 +199,7 @@ var ( docker.ContainerID: ClientContainerID, report.HostNodeID: ClientHostNodeID, }). - WithTopology(report.Process).WithParents(report.EmptySets. + WithTopology(report.Process).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ClientHostNodeID)). Add("container", report.MakeStringSet(ClientContainerNodeID)), ).WithMetrics(report.Metrics{ @@ -212,7 +212,7 @@ var ( docker.ContainerID: ClientContainerID, report.HostNodeID: ClientHostNodeID, }). - WithTopology(report.Process).WithParents(report.EmptySets. + WithTopology(report.Process).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ClientHostNodeID)). Add("container", report.MakeStringSet(ClientContainerNodeID)), ), @@ -222,7 +222,7 @@ var ( docker.ContainerID: ServerContainerID, report.HostNodeID: ServerHostNodeID, }). - WithTopology(report.Process).WithParents(report.EmptySets. + WithTopology(report.Process).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ServerHostNodeID)). Add("container", report.MakeStringSet(ServerContainerNodeID)), ), @@ -231,7 +231,7 @@ var ( process.Name: NonContainerName, report.HostNodeID: ServerHostNodeID, }). - WithTopology(report.Process).WithParents(report.EmptySets. + WithTopology(report.Process).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ServerHostNodeID)), ), }, @@ -254,7 +254,7 @@ var ( docker.ContainerState: docker.StateRunning, docker.ContainerStateHuman: docker.StateRunning, }). - WithTopology(report.Container).WithParents(report.EmptySets. + WithTopology(report.Container).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ClientHostNodeID)). Add("container_image", report.MakeStringSet(ClientContainerImageNodeID)). Add("pod", report.MakeStringSet(ClientPodNodeID)), @@ -279,7 +279,7 @@ var ( docker.LabelPrefix + TestLabelKey2: ApplicationLabelValue2, kubernetes.Namespace: KubernetesNamespace, }). - WithTopology(report.Container).WithParents(report.EmptySets. + WithTopology(report.Container).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ServerHostNodeID)). Add("container_image", report.MakeStringSet(ServerContainerImageNodeID)). Add("pod", report.MakeStringSet(ServerPodNodeID)), @@ -298,7 +298,7 @@ var ( docker.ImageName: ClientContainerImageName, report.HostNodeID: ClientHostNodeID, }). - WithParents(report.EmptySets. + WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ClientHostNodeID)), ).WithTopology(report.ContainerImage), ServerContainerImageNodeID: report.MakeNodeWith(ServerContainerImageNodeID, map[string]string{ @@ -308,7 +308,7 @@ var ( docker.LabelPrefix + "foo1": "bar1", docker.LabelPrefix + "foo2": "bar2", }). - WithParents(report.EmptySets. + WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ServerHostNodeID)), ).WithTopology(report.ContainerImage), }, @@ -323,7 +323,7 @@ var ( "os": "Linux", report.HostNodeID: ClientHostNodeID, }). - WithTopology(report.Host).WithSets(report.EmptySets. + WithTopology(report.Host).WithSets(report.MakeSets(). Add(host.LocalNetworks, report.MakeStringSet("10.10.10.0/24")), ).WithMetrics(report.Metrics{ host.CPUUsage: ClientHostCPUMetric, @@ -337,7 +337,7 @@ var ( "os": "Linux", report.HostNodeID: ServerHostNodeID, }). - WithTopology(report.Host).WithSets(report.EmptySets. + WithTopology(report.Host).WithSets(report.MakeSets(). Add(host.LocalNetworks, report.MakeStringSet("10.10.10.0/24")), ).WithMetrics(report.Metrics{ host.CPUUsage: ServerHostCPUMetric, @@ -356,7 +356,7 @@ var ( kubernetes.Namespace: KubernetesNamespace, report.HostNodeID: ClientHostNodeID, }). - WithTopology(report.Pod).WithParents(report.EmptySets. + WithTopology(report.Pod).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ClientHostNodeID)). Add("service", report.MakeStringSet(ServiceNodeID)), ), @@ -367,7 +367,7 @@ var ( kubernetes.State: "running", report.HostNodeID: ServerHostNodeID, }). - WithTopology(report.Pod).WithParents(report.EmptySets. + WithTopology(report.Pod).WithParents(report.MakeSets(). Add("host", report.MakeStringSet(ServerHostNodeID)). Add("service", report.MakeStringSet(ServiceNodeID)), ), From b5f3aa68ae0a08a655c397f575892a921e1f127e Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 00:32:35 +0100 Subject: [PATCH 2/9] refactor: optimise "empty" case in report set constructors --- report/id_list.go | 6 ++++++ report/node_set.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/report/id_list.go b/report/id_list.go index b285d5dbf..d75df56c3 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -7,11 +7,17 @@ var emptyIDList = IDList(MakeStringSet()) // MakeIDList makes a new IDList. func MakeIDList(ids ...string) IDList { + if len(ids) == 0 { + return emptyIDList + } return IDList(MakeStringSet(ids...)) } // Add is the only correct way to add ids to an IDList. func (a IDList) Add(ids ...string) IDList { + if len(ids) == 0 { + return a + } return IDList(StringSet(a).Add(ids...)) } diff --git a/report/node_set.go b/report/node_set.go index 1baf2152c..3c4ba7141 100644 --- a/report/node_set.go +++ b/report/node_set.go @@ -27,6 +27,9 @@ func MakeNodeSet(nodes ...Node) NodeSet { // Add adds the nodes to the NodeSet. Add is the only valid way to grow a // NodeSet. Add returns the NodeSet to enable chaining. func (n NodeSet) Add(nodes ...Node) NodeSet { + if len(nodes) == 0 { + return n + } result := n.psMap if result == nil { result = ps.NewMap() From fd3fc6656da9a2a79aa74ea5568c6e55f0ee80b9 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 00:55:30 +0100 Subject: [PATCH 3/9] refactor: add some more empty report set objects for efficiency and consistency --- report/controls.go | 6 +++--- report/metrics.go | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/report/controls.go b/report/controls.go index 02e87a5b8..9f2815fbf 100644 --- a/report/controls.go +++ b/report/controls.go @@ -56,11 +56,11 @@ type NodeControls struct { Controls StringSet } +var emptyNodeControls = NodeControls{Controls: MakeStringSet()} + // MakeNodeControls makes a new NodeControls func MakeNodeControls() NodeControls { - return NodeControls{ - Controls: MakeStringSet(), - } + return emptyNodeControls } // Copy is a noop, as NodeControls is immutable diff --git a/report/metrics.go b/report/metrics.go index ea1122c92..036c0089e 100644 --- a/report/metrics.go +++ b/report/metrics.go @@ -65,11 +65,13 @@ func MakeSingletonMetric(t time.Time, v float64) Metric { } +var emptyMetric = Metric{} + // MakeMetric makes a new Metric from unique samples incrementally ordered in // time. func MakeMetric(samples []Sample) Metric { if len(samples) < 1 { - return Metric{} + return emptyMetric } var ( From 7119fb9de86f915fc39ec1f0f8260e7fa4dcaa30 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 01:01:16 +0100 Subject: [PATCH 4/9] refactor: rename report.NewNetworks to MakeNetworks for consistency - all the other report set constructors are called 'Make...' --- render/container_test.go | 2 +- render/theinternet.go | 2 +- render/theinternet_test.go | 2 +- report/networks.go | 6 +++--- report/networks_test.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/render/container_test.go b/render/container_test.go index f376f22ba..0eb5d66b9 100644 --- a/render/container_test.go +++ b/render/container_test.go @@ -45,7 +45,7 @@ type testcase struct { } func testMap(t *testing.T, f render.MapFunc, input testcase) { - localNetworks := report.NewNetworks() + localNetworks := report.MakeNetworks() if err := localNetworks.AddCIDR("1.2.3.0/16"); err != nil { t.Fatalf(err.Error()) } diff --git a/render/theinternet.go b/render/theinternet.go index 06996e49f..3e237b642 100644 --- a/render/theinternet.go +++ b/render/theinternet.go @@ -67,7 +67,7 @@ func isKnownService(hostname string) bool { // used to determine which nodes in the report are "remote", i.e. outside of // our infrastructure. func LocalNetworks(r report.Report) report.Networks { - networks := report.NewNetworks() + networks := report.MakeNetworks() for _, topology := range []report.Topology{r.Host, r.Overlay} { for _, md := range topology.Nodes { diff --git a/render/theinternet_test.go b/render/theinternet_test.go index b91a7fcab..dd650d163 100644 --- a/render/theinternet_test.go +++ b/render/theinternet_test.go @@ -29,7 +29,7 @@ func TestReportLocalNetworks(t *testing.T) { }, }, }) - want := report.NewNetworks() + want := report.MakeNetworks() for _, cidr := range []string{"10.0.0.1/8", "192.168.1.1/24", "10.32.0.1/12"} { if err := want.AddCIDR(cidr); err != nil { panic(err) diff --git a/report/networks.go b/report/networks.go index b4f1dede9..4cd9d4a9b 100644 --- a/report/networks.go +++ b/report/networks.go @@ -14,10 +14,10 @@ type Networks struct{ *critbitgo.Net } // as being host-scoped. // // TODO this design is broken, make it consistent with probe networks. -var LocalNetworks = NewNetworks() +var LocalNetworks = MakeNetworks() -// NewNetworks creates a datastructure representing a set of networks. -func NewNetworks() Networks { +// MakeNetworks creates a datastructure representing a set of networks. +func MakeNetworks() Networks { return Networks{critbitgo.NewNet()} } diff --git a/report/networks_test.go b/report/networks_test.go index dbc49de21..531a5ff0e 100644 --- a/report/networks_test.go +++ b/report/networks_test.go @@ -8,7 +8,7 @@ import ( ) func TestContains(t *testing.T) { - networks := report.NewNetworks() + networks := report.MakeNetworks() for _, cidr := range []string{"10.0.0.1/8", "192.168.1.1/24"} { if err := networks.AddCIDR(cidr); err != nil { panic(err) From 5b131b1ea6985c102de6aaeb310fc564e2b78e03 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 01:13:59 +0100 Subject: [PATCH 5/9] refactor: remove a couple of report set Remove() functions They were unused and their naming was inconsistent with similar operations (that are called Delete()). --- report/id_list.go | 5 ----- report/string_set.go | 15 --------------- report/topology_test.go | 21 --------------------- 3 files changed, 41 deletions(-) diff --git a/report/id_list.go b/report/id_list.go index d75df56c3..049dada0e 100644 --- a/report/id_list.go +++ b/report/id_list.go @@ -21,11 +21,6 @@ func (a IDList) Add(ids ...string) IDList { return IDList(StringSet(a).Add(ids...)) } -// Remove is the only correct way to remove IDs from an IDList. -func (a IDList) Remove(ids ...string) IDList { - return IDList(StringSet(a).Remove(ids...)) -} - // Copy returns a copy of the IDList. func (a IDList) Copy() IDList { return IDList(StringSet(a).Copy()) diff --git a/report/string_set.go b/report/string_set.go index d0b63a50f..f9193427b 100644 --- a/report/string_set.go +++ b/report/string_set.go @@ -67,21 +67,6 @@ func (s StringSet) Add(strs ...string) StringSet { return s } -// Remove removes the strings from the StringSet. Remove is the only valid way -// to shrink a StringSet. Remove returns the StringSet to enable chaining. -func (s StringSet) Remove(strs ...string) StringSet { - for _, str := range strs { - i := sort.Search(len(s), func(i int) bool { return s[i] >= str }) - if i >= len(s) || s[i] != str { - // The list does not have the element. - continue - } - // has the element, remove it. - s = append(s[:i], s[i+1:]...) - } - return s -} - // Merge combines the two StringSets and returns a new result. func (s StringSet) Merge(other StringSet) StringSet { switch { diff --git a/report/topology_test.go b/report/topology_test.go index dccb6a40f..08ca1a2b1 100644 --- a/report/topology_test.go +++ b/report/topology_test.go @@ -45,27 +45,6 @@ func TestStringSetAdd(t *testing.T) { } } -func TestStringSetRemove(t *testing.T) { - for _, testcase := range []struct { - input report.StringSet - strs []string - want report.StringSet - }{ - {input: report.StringSet(nil), strs: []string{}, want: report.StringSet(nil)}, - {input: report.MakeStringSet(), strs: []string{}, want: report.MakeStringSet()}, - {input: report.MakeStringSet("a"), strs: []string{}, want: report.MakeStringSet("a")}, - {input: report.MakeStringSet(), strs: []string{"a"}, want: report.MakeStringSet()}, - {input: report.MakeStringSet("a"), strs: []string{"a"}, want: report.StringSet{}}, - {input: report.MakeStringSet("b"), strs: []string{"a", "b"}, want: report.StringSet{}}, - {input: report.MakeStringSet("a"), strs: []string{"c", "b"}, want: report.MakeStringSet("a")}, - {input: report.MakeStringSet("a", "c"), strs: []string{"b", "b", "b"}, want: report.MakeStringSet("a", "c")}, - } { - if want, have := testcase.want, testcase.input.Remove(testcase.strs...); !reflect.DeepEqual(want, have) { - t.Errorf("%v - %v: want %#v, have %#v", testcase.input, testcase.strs, want, have) - } - } -} - func TestStringSetMerge(t *testing.T) { for _, testcase := range []struct { input report.StringSet From 430e74a80ab036a9f4600775d8a2a5b8ef8226a9 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 02:06:21 +0100 Subject: [PATCH 6/9] refactor: remove report latest map Delete() It wasn't used, and is problematic in any case since it introduces non-monotonicity. --- extras/generate_latest_map | 8 -------- probe/docker/container_test.go | 8 +++++--- report/latest_map_generated.go | 16 ---------------- report/latest_map_internal_test.go | 19 ------------------- 4 files changed, 5 insertions(+), 46 deletions(-) diff --git a/extras/generate_latest_map b/extras/generate_latest_map index 0532b9f51..8dfddc860 100755 --- a/extras/generate_latest_map +++ b/extras/generate_latest_map @@ -125,14 +125,6 @@ function generate_latest_map() { return ${latest_map_type}{m.Map.Set(key, &${entry_type}{Timestamp: timestamp, Value: value})} } - // Delete the value for the given key. - func (m ${latest_map_type}) Delete(key string) ${latest_map_type} { - if m.Map == nil { - return m - } - return ${latest_map_type}{m.Map.Delete(key)} - } - // ForEach executes fn on each key value pair in the map. func (m ${latest_map_type}) ForEach(fn func(k string, timestamp time.Time, v ${data_type})) { if m.Map != nil { diff --git a/probe/docker/container_test.go b/probe/docker/container_test.go index 728a997ad..95b502c90 100644 --- a/probe/docker/container_test.go +++ b/probe/docker/container_test.go @@ -92,11 +92,13 @@ func TestContainer(t *testing.T) { test.Poll(t, 100*time.Millisecond, want, func() interface{} { node := c.GetNode() - node.Latest.ForEach(func(k string, _ time.Time, v string) { - if v == "0" || v == "" { - node.Latest = node.Latest.Delete(k) + latest := report.MakeStringLatestMap() + node.Latest.ForEach(func(k string, t time.Time, v string) { + if v != "0" && v != "" { + latest = latest.Set(k, t, v) } }) + node.Latest = latest return node }) } diff --git a/report/latest_map_generated.go b/report/latest_map_generated.go index 7e1c2b53b..f7fee95a0 100644 --- a/report/latest_map_generated.go +++ b/report/latest_map_generated.go @@ -92,14 +92,6 @@ func (m StringLatestMap) Set(key string, timestamp time.Time, value string) Stri return StringLatestMap{m.Map.Set(key, &stringLatestEntry{Timestamp: timestamp, Value: value})} } -// Delete the value for the given key. -func (m StringLatestMap) Delete(key string) StringLatestMap { - if m.Map == nil { - return m - } - return StringLatestMap{m.Map.Delete(key)} -} - // ForEach executes fn on each key value pair in the map. func (m StringLatestMap) ForEach(fn func(k string, timestamp time.Time, v string)) { if m.Map != nil { @@ -231,14 +223,6 @@ func (m NodeControlDataLatestMap) Set(key string, timestamp time.Time, value Nod return NodeControlDataLatestMap{m.Map.Set(key, &nodeControlDataLatestEntry{Timestamp: timestamp, Value: value})} } -// Delete the value for the given key. -func (m NodeControlDataLatestMap) Delete(key string) NodeControlDataLatestMap { - if m.Map == nil { - return m - } - return NodeControlDataLatestMap{m.Map.Delete(key)} -} - // ForEach executes fn on each key value pair in the map. func (m NodeControlDataLatestMap) ForEach(fn func(k string, timestamp time.Time, v NodeControlData)) { if m.Map != nil { diff --git a/report/latest_map_internal_test.go b/report/latest_map_internal_test.go index 8615fe1d4..7fe2ef116 100644 --- a/report/latest_map_internal_test.go +++ b/report/latest_map_internal_test.go @@ -70,25 +70,6 @@ func TestLatestMapDeepEquals(t *testing.T) { } } -func TestLatestMapDelete(t *testing.T) { - now := time.Now() - want := MakeStringLatestMap() - have := MakeStringLatestMap(). - Set("foo", now, "Baz"). - Delete("foo") - if !reflect.DeepEqual(want, have) { - t.Errorf(test.Diff(want, have)) - } -} - -func TestLatestMapDeleteNil(t *testing.T) { - want := StringLatestMap{} - have := StringLatestMap{}.Delete("foo") - if !reflect.DeepEqual(want, have) { - t.Errorf(test.Diff(want, have)) - } -} - func nilStringLatestMap() StringLatestMap { m := MakeStringLatestMap() m.Map = nil From cfbbdf7bf01d7649e9ac8938c1ca4790e04a4413 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 03:19:52 +0100 Subject: [PATCH 7/9] refactor: optimise report.Sets.Delete() going empty Mainly for consistencty; we do something similar NodeSet.Delete(). --- report/sets.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/report/sets.go b/report/sets.go index d73d5d7bf..61d7ec71f 100644 --- a/report/sets.go +++ b/report/sets.go @@ -47,9 +47,11 @@ func (s Sets) Delete(key string) Sets { if s.psMap == nil { return emptySets } - return Sets{ - psMap: s.psMap.Delete(key), + psMap := s.psMap.Delete(key) + if psMap.IsNil() { + return emptySets } + return Sets{psMap: psMap} } // Lookup returns the sets stored under key. From 1f023890e4af7c1570f3c9bef62cac179a4ea282 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 07:54:32 +0100 Subject: [PATCH 8/9] refactor: optimise report.Map.Render() --- render/render.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/render/render.go b/render/render.go index da48658cc..8cb1678cb 100644 --- a/render/render.go +++ b/render/render.go @@ -110,9 +110,7 @@ func (m *Map) Render(rpt report.Report, dct Decorator) report.Nodes { for outNodeID, inAdjacency := range adjacencies { outAdjacency := report.MakeIDList() for _, inAdjacent := range inAdjacency { - for _, outAdjacent := range mapped[inAdjacent] { - outAdjacency = outAdjacency.Add(outAdjacent) - } + outAdjacency = outAdjacency.Merge(mapped[inAdjacent]) } outNode := output[outNodeID] outNode.Adjacency = outAdjacency From d12603c51600c1be5ab5314f7916ac3d15735384 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 3 Jul 2017 07:55:17 +0100 Subject: [PATCH 9/9] tiny refactor: use inline string set constructor in test --- probe/kubernetes/reporter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probe/kubernetes/reporter_test.go b/probe/kubernetes/reporter_test.go index 9172a2662..229c1e501 100644 --- a/probe/kubernetes/reporter_test.go +++ b/probe/kubernetes/reporter_test.go @@ -264,7 +264,7 @@ func TestTagger(t *testing.T) { } have, ok := rpt.Container.Nodes["container1"].Parents.Lookup(report.Pod) - want := report.MakeStringSet().Add(report.MakePodNodeID("123456")) + want := report.MakeStringSet(report.MakePodNodeID("123456")) if !ok || !reflect.DeepEqual(have, want) { t.Errorf("Expected container to have pod parent %v %v", have, want) }