Skip to content

Commit

Permalink
Review Feedback
Browse files Browse the repository at this point in the history
Squash of:
- including children in topologies_test.go
- report.Node.Prune should prune children also
- rewrote ShortLivedInternetConnections test to express its intent
- adding tests for detail Summary rendering
  • Loading branch information
paulbellamy committed Mar 29, 2016
1 parent 2c6b6e6 commit fe6203f
Show file tree
Hide file tree
Showing 18 changed files with 553 additions and 235 deletions.
3 changes: 2 additions & 1 deletion experimental/graphviz/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (
"os"
"os/exec"

"github.com/weaveworks/scope/render/detailed"
"github.com/weaveworks/scope/report"
)

func dot(w io.Writer, t report.Nodes) {
func dot(w io.Writer, t detailed.NodeSummaries) {
fmt.Fprintf(w, "digraph G {\n")
fmt.Fprintf(w, "\toutputorder=edgesfirst;\n")
fmt.Fprintf(w, "\toverlap=scale;\n")
Expand Down
7 changes: 4 additions & 3 deletions experimental/graphviz/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"fmt"

"github.com/weaveworks/scope/render"
"github.com/weaveworks/scope/render/detailed"
"github.com/weaveworks/scope/report"
)

func renderTo(rpt report.Report, topology string) (report.Nodes, error) {
func renderTo(rpt report.Report, topology string) (detailed.NodeSummaries, error) {
renderer, ok := map[string]render.Renderer{
"processes": render.FilterUnconnected(render.ProcessWithContainerNameRenderer),
"processes-by-name": render.FilterUnconnected(render.ProcessNameRenderer),
Expand All @@ -16,7 +17,7 @@ func renderTo(rpt report.Report, topology string) (report.Nodes, error) {
"hosts": render.HostRenderer,
}[topology]
if !ok {
return report.Nodes{}, fmt.Errorf("unknown topology %v", topology)
return detailed.NodeSummaries{}, fmt.Errorf("unknown topology %v", topology)
}
return renderer.Render(rpt), nil
return detailed.Summaries(renderer.Render(rpt)), nil
}
2 changes: 1 addition & 1 deletion render/detailed/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func (m *MetricRow) CodecDecodeSelf(decoder *codec.Decoder) {
}

// NodeMetrics produces a table (to be consumed directly by the UI) based on
// an origin ID, which is (optimistically) a node ID in one of our topologies.
// an a report.Node, which is (hopefully) a node in one of our topologies.
func NodeMetrics(n report.Node) []MetricRow {
if _, ok := n.Counters.Lookup(n.Topology); ok {
// This is a group of nodes, so no metrics!
Expand Down
11 changes: 7 additions & 4 deletions render/detailed/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,14 @@ var (
func children(n report.Node) []NodeSummaryGroup {
summaries := map[string][]NodeSummary{}
n.Children.ForEach(func(child report.Node) {
if child.ID != n.ID {
if summary, ok := MakeNodeSummary(child); ok {
summaries[child.Topology] = append(summaries[child.Topology], summary.SummarizeMetrics())
}
if child.ID == n.ID {
return
}
summary, ok := MakeNodeSummary(child)
if !ok {
return
}
summaries[child.Topology] = append(summaries[child.Topology], summary.SummarizeMetrics())
})

nodeSummaryGroups := []NodeSummaryGroup{}
Expand Down
2 changes: 1 addition & 1 deletion render/detailed/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestMakeDetailedContainerNode(t *testing.T) {
{ID: "docker_image_id", Value: fixture.ServerContainerImageID},
},
DockerLabels: []detailed.MetadataRow{
{ID: "label_" + render.AmazonECSContainerNameLabel, Value: `server`},
{ID: "label_" + detailed.AmazonECSContainerNameLabel, Value: `server`},
{ID: "label_foo1", Value: `bar1`},
{ID: "label_foo2", Value: `bar2`},
{ID: "label_io.kubernetes.pod.name", Value: "ping/pong-b"},
Expand Down
3 changes: 1 addition & 2 deletions render/detailed/parents.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/weaveworks/scope/probe/docker"
"github.com/weaveworks/scope/probe/host"
"github.com/weaveworks/scope/probe/kubernetes"
"github.com/weaveworks/scope/render"
"github.com/weaveworks/scope/report"
)

Expand Down Expand Up @@ -55,7 +54,7 @@ func Parents(r report.Report, n report.Node) (result []Parent) {
}

func containerParent(n report.Node) Parent {
label, _ := render.GetRenderableContainerName(n)
label := getRenderableContainerName(n)
containerID, _ := n.Latest.Lookup(docker.ContainerID)
return Parent{
ID: report.MakeContainerNodeID(containerID),
Expand Down
61 changes: 46 additions & 15 deletions render/detailed/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ const (
Heptagon = "heptagon"
Hexagon = "hexagon"
Cloud = "cloud"

ImageNameNone = "<none>"

// Keys we use to render container names
AmazonECSContainerNameLabel = "com.amazonaws.ecs.container-name"
KubernetesContainerNameLabel = "io.kubernetes.container.name"
)

// NodeSummaryGroup is a topology-typed group of children for a Node.
Expand Down Expand Up @@ -88,9 +94,7 @@ func MakeNodeSummary(n report.Node) (NodeSummary, bool) {
report.Host: hostNodeSummary,
}
if renderer, ok := renderers[n.Topology]; ok {
if summary, ok := baseNodeSummary(NodeSummary{}, n); ok {
return renderer(summary, n)
}
return renderer(baseNodeSummary(n), n)
}
return NodeSummary{}, false
}
Expand Down Expand Up @@ -129,15 +133,16 @@ func (n NodeSummary) Copy() NodeSummary {
return result
}

func baseNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
base.ID = n.ID
base.Shape = Circle
base.Linkable = true
base.Metadata = NodeMetadata(n)
base.DockerLabels = NodeDockerLabels(n)
base.Metrics = NodeMetrics(n)
base.Adjacency = n.Adjacency.Copy()
return base, true
func baseNodeSummary(n report.Node) NodeSummary {
return NodeSummary{
ID: n.ID,
Shape: Circle,
Linkable: true,
Metadata: NodeMetadata(n),
DockerLabels: NodeDockerLabels(n),
Metrics: NodeMetrics(n),
Adjacency: n.Adjacency.Copy(),
}
}

func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
Expand All @@ -158,7 +163,8 @@ func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
// try rendering it as an uncontained node
if strings.HasPrefix(n.ID, render.MakePseudoNodeID(render.UncontainedID)) {
base.Label = render.UncontainedMajor
base.Shape = Circle
base.Shape = Square
base.Stack = true
base.LabelMinor = report.ExtractHostID(n)
return base, true
}
Expand Down Expand Up @@ -202,7 +208,7 @@ func processNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
}

func containerNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) {
base.Label, _ = render.GetRenderableContainerName(n)
base.Label = getRenderableContainerName(n)

if c, ok := n.Counters.Lookup(report.Container); ok {
base.Stack = true
Expand Down Expand Up @@ -235,7 +241,7 @@ func containerImageNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bo
base.Shape = Hexagon
base.Stack = true

if base.Label == "<none>" {
if base.Label == ImageNameNone {
base.Label, _ = n.Latest.Lookup(docker.ImageID)
if len(base.Label) > 12 {
base.Label = base.Label[:12]
Expand Down Expand Up @@ -356,3 +362,28 @@ func (n NodeSummaries) Copy() NodeSummaries {
}
return result
}

// getRenderableContainerName obtains a user-friendly container name, to render in the UI
func getRenderableContainerName(nmd report.Node) string {
for _, key := range []string{
// Amazon's ecs-agent produces huge Docker container names, destructively
// derived from mangling Container Definition names in Task
// Definitions.
//
// However, the ecs-agent provides a label containing the original Container
// Definition name.
docker.LabelPrefix + AmazonECSContainerNameLabel,
// Kubernetes also mangles its Docker container names and provides a
// label with the original container name. However, note that this label
// is only provided by Kubernetes versions >= 1.2 (see
// https://github.com/kubernetes/kubernetes/pull/17234/ )
docker.LabelPrefix + KubernetesContainerNameLabel,
docker.ContainerName,
docker.ContainerHostname,
} {
if label, ok := nmd.Latest.Lookup(key); ok {
return label
}
}
return ""
}
186 changes: 186 additions & 0 deletions render/detailed/summary_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
package detailed_test

import (
"sort"
"testing"
"time"

"github.com/weaveworks/scope/common/mtime"
"github.com/weaveworks/scope/probe/docker"
"github.com/weaveworks/scope/probe/host"
"github.com/weaveworks/scope/probe/process"
"github.com/weaveworks/scope/render"
"github.com/weaveworks/scope/render/detailed"
"github.com/weaveworks/scope/render/expected"
"github.com/weaveworks/scope/report"
"github.com/weaveworks/scope/test"
"github.com/weaveworks/scope/test/fixture"
"github.com/weaveworks/scope/test/reflect"
)

func TestSummaries(t *testing.T) {
{
// Just a convenient source of some rendered nodes
have := detailed.Summaries(render.ProcessRenderer.Render(fixture.Report))
// The ids of the processes rendered above
expectedIDs := []string{
fixture.ClientProcess1NodeID,
fixture.ClientProcess2NodeID,
fixture.ServerProcessNodeID,
fixture.NonContainerProcessNodeID,
expected.UnknownPseudoNode1ID,
expected.UnknownPseudoNode2ID,
render.IncomingInternetID,
render.OutgoingInternetID,
}
sort.Strings(expectedIDs)

// It should summarize each node
ids := []string{}
for id := range have {
ids = append(ids, id)
}
sort.Strings(ids)
if !reflect.DeepEqual(expectedIDs, ids) {
t.Fatalf("Expected Summaries to have summarized every node in the process renderer: %v, but got %v", expectedIDs, ids)
}
}

// It should summarize nodes' metrics
{
t1, t2 := mtime.Now().Add(-1*time.Minute), mtime.Now()
metric := report.MakeMetric().Add(t1, 1).Add(t2, 2)
input := fixture.Report.Copy()

input.Process.Nodes[fixture.ClientProcess1NodeID] = input.Process.Nodes[fixture.ClientProcess1NodeID].WithMetrics(report.Metrics{process.CPUUsage: metric})
have := detailed.Summaries(render.ProcessRenderer.Render(input))

node, ok := have[fixture.ClientProcess1NodeID]
if !ok {
t.Fatalf("Expected output to have the node we added the metric to")
}

var row detailed.MetricRow
ok = false
for _, metric := range node.Metrics {
if metric.ID == process.CPUUsage {
row = metric
ok = true
break
}
}
if !ok {
t.Fatalf("Expected node to have the metric we added")
}

// Our summarized MetricRow
want := detailed.MetricRow{
ID: process.CPUUsage,
Format: "percent",
Value: 2,
Metric: &report.Metric{
Samples: nil,
Min: metric.Min,
Max: metric.Max,
First: metric.First,
Last: metric.Last,
},
}
if !reflect.DeepEqual(want, row) {
t.Fatalf("Expected to have summarized the node's metrics: %s", test.Diff(want, row))
}
}
}

func TestMakeNodeSummary(t *testing.T) {
testcases := []struct {
name string
input report.Node
ok bool
want detailed.NodeSummary
}{
{
name: "single process rendering",
input: expected.RenderedProcesses[fixture.ClientProcess1NodeID],
ok: true,
want: detailed.NodeSummary{
ID: fixture.ClientProcess1NodeID,
Label: fixture.Client1Name,
LabelMinor: "client.hostname.com (10001)",
Rank: fixture.Client1Name,
Shape: "square",
Metadata: []detailed.MetadataRow{
{ID: process.PID, Value: fixture.Client1PID, Prime: true, Datatype: "number"},
},
Metrics: []detailed.MetricRow{},
Adjacency: report.MakeIDList(fixture.ServerProcessNodeID),
},
},
{
name: "single container rendering",
input: expected.RenderedContainers[fixture.ClientContainerNodeID],
ok: true,
want: detailed.NodeSummary{
ID: fixture.ClientContainerNodeID,
Label: fixture.ClientContainerName,
LabelMinor: fixture.ClientHostName,
Rank: fixture.ClientContainerImageName,
Shape: "hexagon",
Linkable: true,
Metadata: []detailed.MetadataRow{
{ID: docker.ContainerID, Value: fixture.ClientContainerID, Prime: true},
},
Metrics: []detailed.MetricRow{},
Adjacency: report.MakeIDList(fixture.ServerContainerNodeID),
},
},
{
name: "single container image rendering",
input: expected.RenderedContainerImages[fixture.ClientContainerImageNodeID],
ok: true,
want: detailed.NodeSummary{
ID: fixture.ClientContainerImageNodeID,
Label: fixture.ClientContainerImageName,
LabelMinor: "1 container",
Rank: fixture.ClientContainerImageName,
Shape: "hexagon",
Linkable: true,
Stack: true,
Metadata: []detailed.MetadataRow{
{ID: docker.ImageID, Value: fixture.ClientContainerImageID, Prime: true},
{ID: report.Container, Value: "1", Prime: true, Datatype: "number"},
},
Adjacency: report.MakeIDList(fixture.ServerContainerImageNodeID),
},
},
{
name: "single host rendering",
input: expected.RenderedHosts[fixture.ClientHostNodeID],
ok: true,
want: detailed.NodeSummary{
ID: fixture.ClientHostNodeID,
Label: "client",
LabelMinor: "hostname.com",
Rank: "hostname.com",
Shape: "circle",
Linkable: true,
Metadata: []detailed.MetadataRow{
{ID: host.HostName, Value: fixture.ClientHostName, Prime: false},
},
Metrics: []detailed.MetricRow{},
Adjacency: report.MakeIDList(fixture.ServerHostNodeID),
},
},
}
for _, testcase := range testcases {
have, ok := detailed.MakeNodeSummary(testcase.input)
if ok != testcase.ok {
t.Errorf("%s: MakeNodeSummary failed: expected ok value to be: %v", testcase.name, testcase.ok)
continue
}

if !reflect.DeepEqual(testcase.want, have) {
t.Errorf("%s: Node Summary did not match: %s", testcase.name, test.Diff(testcase.want, have))
}
}
}
Loading

0 comments on commit fe6203f

Please sign in to comment.