From 7a3dad2cebc0a1e58fbb5ad00d26d88927557723 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Tue, 4 Oct 2016 17:35:28 +0000 Subject: [PATCH 1/7] Add filters to details panel request --- client/app/scripts/actions/app-actions.js | 5 +++++ client/app/scripts/utils/web-api-utils.js | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 03646ed96..2ad28d933 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -170,6 +170,7 @@ export function changeTopologyOption(option, value, topologyId) { ); getNodeDetails( state.get('topologyUrlsById'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -260,6 +261,7 @@ export function clickNode(nodeId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -285,6 +287,7 @@ export function clickRelative(nodeId, topologyId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -543,6 +546,7 @@ export function receiveTopologies(topologies) { ); getNodeDetails( state.get('topologyUrlsById'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -645,6 +649,7 @@ export function route(urlState) { ); getNodeDetails( state.get('topologyUrlsById'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); diff --git a/client/app/scripts/utils/web-api-utils.js b/client/app/scripts/utils/web-api-utils.js index 9d95b9b25..d334cd830 100644 --- a/client/app/scripts/utils/web-api-utils.js +++ b/client/app/scripts/utils/web-api-utils.js @@ -163,12 +163,13 @@ export function getNodesDelta(topologyUrl, options, dispatch) { } } -export function getNodeDetails(topologyUrlsById, nodeMap, dispatch) { +export function getNodeDetails(topologyUrlsById, options, nodeMap, dispatch) { // get details for all opened nodes const obj = nodeMap.last(); if (obj && topologyUrlsById.has(obj.topologyId)) { const topologyUrl = topologyUrlsById.get(obj.topologyId); - const url = [topologyUrl, '/', encodeURIComponent(obj.id)] + const optionsQuery = buildOptionsQuery(options); + const url = [topologyUrl, '/', encodeURIComponent(obj.id), '?', optionsQuery] .join('').substr(1); reqwest({ url, From bbb2c10975b200bfa17a85e9917d4fcdec61e109 Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 5 Oct 2016 08:51:26 +0000 Subject: [PATCH 2/7] Apply filters to details panel --- app/api_topology.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/api_topology.go b/app/api_topology.go index f61e341a1..fedf7bddd 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -36,12 +36,12 @@ func handleTopology(ctx context.Context, renderer render.Renderer, decorator ren } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, _ render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] - rendered = renderer.Render(report, nil) + rendered = renderer.Render(report, decorator) node, ok = rendered[nodeID] ) if !ok { From 699fe45e650bed337c60b4258a7f6e4770348ad7 Mon Sep 17 00:00:00 2001 From: Simon Howe Date: Wed, 5 Oct 2016 12:23:31 +0200 Subject: [PATCH 3/7] Details panel: send topology options of node type being loaded --- client/app/scripts/actions/app-actions.js | 10 +++++----- client/app/scripts/utils/web-api-utils.js | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 2ad28d933..17a27c7e1 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -170,7 +170,7 @@ export function changeTopologyOption(option, value, topologyId) { ); getNodeDetails( state.get('topologyUrlsById'), - getActiveTopologyOptions(state), + state.get('topologyOptions'), state.get('nodeDetails'), dispatch ); @@ -261,7 +261,7 @@ export function clickNode(nodeId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), - getActiveTopologyOptions(state), + state.get('topologyOptions'), state.get('nodeDetails'), dispatch ); @@ -287,7 +287,7 @@ export function clickRelative(nodeId, topologyId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), - getActiveTopologyOptions(state), + state.get('topologyOptions'), state.get('nodeDetails'), dispatch ); @@ -546,7 +546,7 @@ export function receiveTopologies(topologies) { ); getNodeDetails( state.get('topologyUrlsById'), - getActiveTopologyOptions(state), + state.get('topologyOptions'), state.get('nodeDetails'), dispatch ); @@ -649,7 +649,7 @@ export function route(urlState) { ); getNodeDetails( state.get('topologyUrlsById'), - getActiveTopologyOptions(state), + state.get('topologyOptions'), state.get('nodeDetails'), dispatch ); diff --git a/client/app/scripts/utils/web-api-utils.js b/client/app/scripts/utils/web-api-utils.js index d334cd830..cee8f27ac 100644 --- a/client/app/scripts/utils/web-api-utils.js +++ b/client/app/scripts/utils/web-api-utils.js @@ -163,10 +163,11 @@ export function getNodesDelta(topologyUrl, options, dispatch) { } } -export function getNodeDetails(topologyUrlsById, options, nodeMap, dispatch) { +export function getNodeDetails(topologyUrlsById, topologyOptions, nodeMap, dispatch) { // get details for all opened nodes const obj = nodeMap.last(); if (obj && topologyUrlsById.has(obj.topologyId)) { + const options = topologyOptions.get(obj.topologyId); const topologyUrl = topologyUrlsById.get(obj.topologyId); const optionsQuery = buildOptionsQuery(options); const url = [topologyUrl, '/', encodeURIComponent(obj.id), '?', optionsQuery] From 3f27d5f6ccf786076ae181037d6497412d22b9cd Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 5 Oct 2016 12:22:50 +0000 Subject: [PATCH 4/7] Do not filter out the target nodes when obtaining the details panel --- app/api_topology.go | 11 ++++++----- render/filters.go | 22 ++++++++++++++++++++++ render/render.go | 11 +++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/app/api_topology.go b/app/api_topology.go index fedf7bddd..43c0f3668 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -38,11 +38,12 @@ func handleTopology(ctx context.Context, renderer render.Renderer, decorator ren // Individual nodes. func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { var ( - vars = mux.Vars(r) - topologyID = vars["topology"] - nodeID = vars["id"] - rendered = renderer.Render(report, decorator) - node, ok = rendered[nodeID] + vars = mux.Vars(r) + topologyID = vars["topology"] + nodeID = vars["id"] + preciousRenderer = render.PreciousNodeRenderer{nodeID, renderer} + rendered = preciousRenderer.Render(report, decorator) + node, ok = rendered[nodeID] ) if !ok { http.NotFound(w, r) diff --git a/render/filters.go b/render/filters.go index 347ca36f3..c1d2ea9e4 100644 --- a/render/filters.go +++ b/render/filters.go @@ -10,6 +10,28 @@ import ( "github.com/weaveworks/scope/report" ) +// PreciousNodeRenderer ensures a node is never filtered out by decorators +type PreciousNodeRenderer struct { + PreciousNodeID string + Renderer +} + +func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { + undecoratedNodes := p.Renderer.Render(rpt, nil) + preciousNode, foundBeforeDecoration := undecoratedNodes[p.PreciousNodeID] + finalNodes := applyDecorator{ConstantRenderer(undecoratedNodes)}.Render(rpt, dct) + if _, ok := finalNodes[p.PreciousNodeID]; !ok && foundBeforeDecoration { + finalNodes[p.PreciousNodeID] = preciousNode + } + return finalNodes +} + +func (p PreciousNodeRenderer) Stats(rpt report.Report, dct Decorator) Stats { + // default to the underlying renderer + // TODO: we don't take into account the precious node, so we may be off by one + return p.Renderer.Stats(rpt, dct) +} + // CustomRenderer allow for mapping functions that received the entire topology // in one call - useful for functions that need to consider the entire graph. // We should minimise the use of this renderer type, as it is very inflexible. diff --git a/render/render.go b/render/render.go index 9a9078a3e..2a11bdeff 100644 --- a/render/render.go +++ b/render/render.go @@ -185,3 +185,14 @@ func (cr conditionalRenderer) Stats(rpt report.Report, dct Decorator) Stats { } return Stats{} } + +// ConstantRenderer renders a fixed set of nodes +type ConstantRenderer report.Nodes + +func (c ConstantRenderer) Render(_ report.Report, _ Decorator) report.Nodes { + return report.Nodes(c) +} + +func (c ConstantRenderer) Stats(_ report.Report, _ Decorator) Stats { + return Stats{} +} From 5eabf5436cf6bd8552668d47c4116a8982bb634c Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 5 Oct 2016 13:07:17 +0000 Subject: [PATCH 5/7] Make linter happy --- app/api_topology.go | 2 +- render/filters.go | 2 ++ render/render.go | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/api_topology.go b/app/api_topology.go index 43c0f3668..d9d78b356 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -41,7 +41,7 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] - preciousRenderer = render.PreciousNodeRenderer{nodeID, renderer} + preciousRenderer = render.PreciousNodeRenderer{PreciousNodeID: nodeID, Renderer: renderer} rendered = preciousRenderer.Render(report, decorator) node, ok = rendered[nodeID] ) diff --git a/render/filters.go b/render/filters.go index c1d2ea9e4..071d68818 100644 --- a/render/filters.go +++ b/render/filters.go @@ -16,6 +16,7 @@ type PreciousNodeRenderer struct { Renderer } +// Render implements Renderer func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) report.Nodes { undecoratedNodes := p.Renderer.Render(rpt, nil) preciousNode, foundBeforeDecoration := undecoratedNodes[p.PreciousNodeID] @@ -26,6 +27,7 @@ func (p PreciousNodeRenderer) Render(rpt report.Report, dct Decorator) report.No return finalNodes } +// Stats implements Renderer func (p PreciousNodeRenderer) Stats(rpt report.Report, dct Decorator) Stats { // default to the underlying renderer // TODO: we don't take into account the precious node, so we may be off by one diff --git a/render/render.go b/render/render.go index 2a11bdeff..56928304f 100644 --- a/render/render.go +++ b/render/render.go @@ -189,10 +189,12 @@ func (cr conditionalRenderer) Stats(rpt report.Report, dct Decorator) Stats { // ConstantRenderer renders a fixed set of nodes type ConstantRenderer report.Nodes +// Render implements Renderer func (c ConstantRenderer) Render(_ report.Report, _ Decorator) report.Nodes { return report.Nodes(c) } +// Stats implements Renderer func (c ConstantRenderer) Stats(_ report.Report, _ Decorator) Stats { return Stats{} } From 3250b7289e80a207b25886b8e51b867f6574324f Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 5 Oct 2016 14:04:53 +0000 Subject: [PATCH 6/7] Do not apply filters if no filtering options are provided --- app/api_topologies.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/api_topologies.go b/app/api_topologies.go index a20a13605..deefe81f1 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -350,6 +350,11 @@ func (r *registry) rendererForTopology(topologyID string, values url.Values, rpt } topology = updateFilters(rpt, []APITopologyDesc{topology})[0] + if len(values) == 0 { + // Do not apply filtering if no options where provided + return topology.renderer, nil, nil + } + var decorators []render.Decorator for _, group := range topology.Options { value := values.Get(group.ID) From ffdcc2367e303c5054aa871c05f31678847734dd Mon Sep 17 00:00:00 2001 From: Alfonso Acosta Date: Wed, 5 Oct 2016 15:44:13 +0000 Subject: [PATCH 7/7] Only send filters to nodes from the current topology This reverts commit 699fe45e650bed337c60b4258a7f6e4770348ad7. --- client/app/scripts/actions/app-actions.js | 15 ++++++++++----- client/app/scripts/utils/web-api-utils.js | 14 +++++++++----- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 17a27c7e1..847c5c338 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -170,7 +170,8 @@ export function changeTopologyOption(option, value, topologyId) { ); getNodeDetails( state.get('topologyUrlsById'), - state.get('topologyOptions'), + state.get('currentTopologyId'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -261,7 +262,8 @@ export function clickNode(nodeId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), - state.get('topologyOptions'), + state.get('currentTopologyId'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -287,7 +289,8 @@ export function clickRelative(nodeId, topologyId, label, origin) { const state = getState(); getNodeDetails( state.get('topologyUrlsById'), - state.get('topologyOptions'), + state.get('currentTopologyId'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -546,7 +549,8 @@ export function receiveTopologies(topologies) { ); getNodeDetails( state.get('topologyUrlsById'), - state.get('topologyOptions'), + state.get('currentTopologyId'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); @@ -649,7 +653,8 @@ export function route(urlState) { ); getNodeDetails( state.get('topologyUrlsById'), - state.get('topologyOptions'), + state.get('currentTopologyId'), + getActiveTopologyOptions(state), state.get('nodeDetails'), dispatch ); diff --git a/client/app/scripts/utils/web-api-utils.js b/client/app/scripts/utils/web-api-utils.js index cee8f27ac..f5f0824f1 100644 --- a/client/app/scripts/utils/web-api-utils.js +++ b/client/app/scripts/utils/web-api-utils.js @@ -163,15 +163,19 @@ export function getNodesDelta(topologyUrl, options, dispatch) { } } -export function getNodeDetails(topologyUrlsById, topologyOptions, nodeMap, dispatch) { +export function getNodeDetails(topologyUrlsById, currentTopologyId, options, nodeMap, dispatch) { // get details for all opened nodes const obj = nodeMap.last(); if (obj && topologyUrlsById.has(obj.topologyId)) { - const options = topologyOptions.get(obj.topologyId); const topologyUrl = topologyUrlsById.get(obj.topologyId); - const optionsQuery = buildOptionsQuery(options); - const url = [topologyUrl, '/', encodeURIComponent(obj.id), '?', optionsQuery] - .join('').substr(1); + let urlComponents = [topologyUrl, '/', encodeURIComponent(obj.id)]; + if (currentTopologyId === obj.topologyId) { + // Only forward filters for nodes in the current topology + const optionsQuery = buildOptionsQuery(options); + urlComponents = urlComponents.concat(['?', optionsQuery]); + } + const url = urlComponents.join('').substr(1); + reqwest({ url, success: (res) => {