From 286453b9a99b9e674b4f1bcd98cfe7da0961e3fa Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 27 Dec 2017 16:22:52 +0000 Subject: [PATCH] refactor: move RenderContext where it belongs It shouldn't be in 'report' because that is shared between the probe and the app, and RenderContext, as the name suggests, is only needed in the app. --- app/api_topologies.go | 2 -- app/api_topologies_test.go | 2 +- app/api_topology.go | 15 +++++++++++++-- app/benchmark_internal_test.go | 2 +- app/collector.go | 11 +---------- render/detailed/node.go | 10 ++++++++-- render/detailed/node_test.go | 8 ++++---- render/detailed/summary.go | 4 ++-- render/detailed/summary_test.go | 14 +++++++------- report/report.go | 6 ------ 10 files changed, 37 insertions(+), 37 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index 5aaff9d18..1d205ce60 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -566,8 +566,6 @@ func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { } } -type rendererHandler func(context.Context, render.Renderer, render.Transformer, report.RenderContext, http.ResponseWriter, *http.Request) - func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 2dbfeb6a5..4ad448b84 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -183,7 +183,7 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.Render(fixture.Report, renderer, filter).Nodes), nil + return detailed.Summaries(detailed.RenderContext{Report: fixture.Report}, render.Render(fixture.Report, renderer, filter).Nodes), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { diff --git a/app/api_topology.go b/app/api_topology.go index 145b5aa85..8111885c1 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -28,15 +28,26 @@ type APINode struct { Node detailed.Node `json:"node"` } +// RenderContextForReporter creates the rendering context for the given reporter. +func RenderContextForReporter(rep Reporter, r report.Report) detailed.RenderContext { + rc := detailed.RenderContext{Report: r} + if wrep, ok := rep.(WebReporter); ok { + rc.MetricsGraphURL = wrep.MetricsGraphURL + } + return rc +} + +type rendererHandler func(context.Context, render.Renderer, render.Transformer, detailed.RenderContext, http.ResponseWriter, *http.Request) + // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ Nodes: detailed.Summaries(rc, render.Render(rc.Report, renderer, transformer).Nodes), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, transformer render.Transformer, rc detailed.RenderContext, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] diff --git a/app/benchmark_internal_test.go b/app/benchmark_internal_test.go index 72b26cd0f..afc61e63f 100644 --- a/app/benchmark_internal_test.go +++ b/app/benchmark_internal_test.go @@ -137,7 +137,7 @@ func BenchmarkRenderProcessNames(b *testing.B) { func benchmarkSummarizeTopology(b *testing.B, topologyID string) { r := getReport(b) - rc := report.RenderContext{Report: r} + rc := detailed.RenderContext{Report: r} nodes := renderForTopology(b, topologyID, r) b.ResetTimer() for i := 0; i < b.N; i++ { diff --git a/app/collector.go b/app/collector.go index 8477be4da..93e71ab2b 100644 --- a/app/collector.go +++ b/app/collector.go @@ -37,21 +37,12 @@ type Reporter interface { // WebReporter is a reporter that creates reports whose data is eventually // displayed on websites. It carries fields that will be forwarded to the -// report.RenderContext +// detailed.RenderContext type WebReporter struct { Reporter MetricsGraphURL string } -// RenderContextForReporter creates the rendering context for the given reporter. -func RenderContextForReporter(rep Reporter, r report.Report) report.RenderContext { - rc := report.RenderContext{Report: r} - if wrep, ok := rep.(WebReporter); ok { - rc.MetricsGraphURL = wrep.MetricsGraphURL - } - return rc -} - // Adder is something that can accept reports. It's a convenient interface for // parts of the app, and several experimental components. It takes the following // arguments: diff --git a/render/detailed/node.go b/render/detailed/node.go index f18a9752c..e0e972694 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -78,9 +78,15 @@ func (c *ControlInstance) CodecDecodeSelf(decoder *codec.Decoder) { } } +// RenderContext carries contextual data that is needed when rendering parts of the report. +type RenderContext struct { + report.Report + MetricsGraphURL string +} + // MakeNode transforms a renderable node to a detailed node. It uses // aggregate metadata, plus the set of origin node IDs, to produce tables. -func MakeNode(topologyID string, rc report.RenderContext, ns report.Nodes, n report.Node) Node { +func MakeNode(topologyID string, rc RenderContext, ns report.Nodes, n report.Node) Node { summary, _ := MakeNodeSummary(rc, n) return Node{ NodeSummary: summary, @@ -181,7 +187,7 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(rc report.RenderContext, n report.Node) []NodeSummaryGroup { +func children(rc RenderContext, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index 1a43fc6f6..11fafc6ed 100644 --- a/render/detailed/node_test.go +++ b/render/detailed/node_test.go @@ -18,7 +18,7 @@ import ( ) func child(t *testing.T, r render.Renderer, id string) detailed.NodeSummary { - s, ok := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, r.Render(fixture.Report).Nodes[id]) + s, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, r.Render(fixture.Report).Nodes[id]) if !ok { t.Fatalf("Expected node %s to be summarizable, but wasn't", id) } @@ -32,7 +32,7 @@ func connectionID(nodeID string, addr string) string { func TestMakeDetailedHostNode(t *testing.T) { renderableNodes := render.HostRenderer.Render(fixture.Report).Nodes renderableNode := renderableNodes[fixture.ClientHostNodeID] - have := detailed.MakeNode("hosts", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) + have := detailed.MakeNode("hosts", detailed.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) @@ -185,7 +185,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("containers", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) + have := detailed.MakeNode("containers", detailed.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) serverProcessNodeSummary.Linkable = true @@ -317,7 +317,7 @@ func TestMakeDetailedPodNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("pods", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) + have := detailed.MakeNode("pods", detailed.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) containerNodeSummary := child(t, render.ContainerWithImageNameRenderer, fixture.ServerContainerNodeID) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) diff --git a/render/detailed/summary.go b/render/detailed/summary.go index d309c2835..4217ee494 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -143,7 +143,7 @@ func MakeBasicNodeSummary(r report.Report, n report.Node) (BasicNodeSummary, boo } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool) { +func MakeNodeSummary(rc RenderContext, n report.Node) (NodeSummary, bool) { base, ok := MakeBasicNodeSummary(rc.Report, n) if !ok { return NodeSummary{}, false @@ -412,7 +412,7 @@ func (s nodeSummariesByID) Less(i, j int) bool { return s[i].ID < s[j].ID } type NodeSummaries map[string]NodeSummary // Summaries converts RenderableNodes into a set of NodeSummaries -func Summaries(rc report.RenderContext, rns report.Nodes) NodeSummaries { +func Summaries(rc RenderContext, rns report.Nodes) NodeSummaries { result := NodeSummaries{} for id, node := range rns { diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index 05f9c2b73..2044723ca 100644 --- a/render/detailed/summary_test.go +++ b/render/detailed/summary_test.go @@ -21,7 +21,7 @@ import ( func TestSummaries(t *testing.T) { { // Just a convenient source of some rendered nodes - have := detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.ProcessRenderer.Render(fixture.Report).Nodes) + have := detailed.Summaries(detailed.RenderContext{Report: fixture.Report}, render.ProcessRenderer.Render(fixture.Report).Nodes) // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -53,7 +53,7 @@ func TestSummaries(t *testing.T) { processNode.Metrics = processNode.Metrics.Copy() processNode.Metrics[process.CPUUsage] = metric input.Process.Nodes[fixture.ClientProcess1NodeID] = processNode - have := detailed.Summaries(report.RenderContext{Report: input}, render.ProcessRenderer.Render(input).Nodes) + have := detailed.Summaries(detailed.RenderContext{Report: input}, render.ProcessRenderer.Render(input).Nodes) node, ok := have[fixture.ClientProcess1NodeID] if !ok { @@ -196,7 +196,7 @@ func TestMakeNodeSummary(t *testing.T) { }, } for _, testcase := range testcases { - have, ok := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, testcase.input) + have, ok := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, testcase.input) if ok != testcase.ok { t.Errorf("%s: MakeNodeSummary failed: expected ok value to be: %v", testcase.name, testcase.ok) continue @@ -228,7 +228,7 @@ func TestMakeNodeSummaryNoMetadata(t *testing.T) { report.Overlay: report.MakeOverlayNodeID("", "3e:ca:14:ca:12:5c"), processNameTopology: "/home/weave/scope", } { - summary, b := detailed.MakeNodeSummary(report.RenderContext{}, report.MakeNode(id).WithTopology(topology)) + summary, b := detailed.MakeNodeSummary(detailed.RenderContext{}, report.MakeNode(id).WithTopology(topology)) switch { case !b: t.Errorf("Node Summary missing for topology %s, id %s", topology, id) @@ -270,7 +270,7 @@ func TestNodeMetadata(t *testing.T) { }, } for _, input := range inputs { - summary, _ := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, input.node) + summary, _ := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, input.node) have := summary.Metadata if !reflect.DeepEqual(input.want, have) { t.Errorf("%s: %s", input.name, test.Diff(input.want, have)) @@ -371,7 +371,7 @@ func TestNodeMetrics(t *testing.T) { }, } for _, input := range inputs { - summary, _ := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, input.node) + summary, _ := detailed.MakeNodeSummary(detailed.RenderContext{Report: fixture.Report}, input.node) have := summary.Metrics if !reflect.DeepEqual(input.want, have) { t.Errorf("%s: %s", input.name, test.Diff(input.want, have)) @@ -468,7 +468,7 @@ func TestNodeTables(t *testing.T) { }, } for _, input := range inputs { - summary, _ := detailed.MakeNodeSummary(report.RenderContext{Report: input.rpt}, input.node) + summary, _ := detailed.MakeNodeSummary(detailed.RenderContext{Report: input.rpt}, input.node) have := summary.Tables if !reflect.DeepEqual(input.want, have) { t.Errorf("%s: %s", input.name, test.Diff(input.want, have)) diff --git a/report/report.go b/report/report.go index 19bf9e780..aeb8adef0 100644 --- a/report/report.go +++ b/report/report.go @@ -168,12 +168,6 @@ type Report struct { ID string `deepequal:"skip"` } -// RenderContext carries contextual data that is needed when rendering parts of the report. -type RenderContext struct { - Report - MetricsGraphURL string -} - // MakeReport makes a clean report, ready to Merge() other reports into. func MakeReport() Report { return Report{