From fe98802e1aa3461bc1a1bd0ee2db7a60e13c8c77 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 11:11:33 +0200 Subject: [PATCH 01/31] Link scope-ui graphs clickable to prometheus queries scope-app: - Adds `-app.metrics-graph` cli flag for configuring the base url to use for graph links; supports `:orgID` and `:query` placeholders - Renders `metric_links` in node detail API response scope-ui: - Extends `` with option `alwaysShow` and adds boolean `isCloud` property - Links metric graphs in the ui's node details view for all k8s toplogies; or displays placeholder graph if no metrics available --- .../app/scripts/components/cloud-feature.js | 8 +- client/app/scripts/components/node-details.js | 16 +- .../node-details/node-details-health-item.js | 10 +- .../node-details-health-link-item.js | 47 +++++ .../node-details/node-details-health.js | 23 ++- client/app/styles/_base.scss | 27 ++- prog/app.go | 8 +- prog/main.go | 2 + prog/probe.go | 4 +- render/detailed/links.go | 167 ++++++++++++++++++ render/detailed/links_test.go | 76 ++++++++ render/detailed/summary.go | 47 ++--- render/expected/expected.go | 1 - 13 files changed, 396 insertions(+), 40 deletions(-) create mode 100644 client/app/scripts/components/node-details/node-details-health-link-item.js create mode 100644 render/detailed/links.go create mode 100644 render/detailed/links_test.go diff --git a/client/app/scripts/components/cloud-feature.js b/client/app/scripts/components/cloud-feature.js index b9c8f3524f..c34277b8ff 100644 --- a/client/app/scripts/components/cloud-feature.js +++ b/client/app/scripts/components/cloud-feature.js @@ -13,10 +13,16 @@ class CloudFeature extends React.Component { if (process.env.WEAVE_CLOUD) { return React.cloneElement(React.Children.only(this.props.children), { params: this.context.router.params, - router: this.context.router + router: this.context.router, + isCloud: true }); } + // also show if not in weave cloud? + if (this.props.alwaysShow) { + return React.cloneElement(React.Children.only(this.props.children), {isCloud: false}); + } + return null; } } diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index e3d76d7e0c..8ffc7aeddb 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -172,6 +172,14 @@ class NodeDetails extends React.Component { } }; + // collect by metric id (id => link) + const metricLinks = (details.metric_links || []) + .reduce((agg, link) => Object.assign(agg, {[link.id]: link}), {}); + + // collect links with no corresponding metric + const unattachedLinks = Object.assign({}, metricLinks); + (details.metrics || []).forEach(metric => delete unattachedLinks[metric.id]); + return (
{tools} @@ -197,9 +205,13 @@ class NodeDetails extends React.Component {
}
- {details.metrics &&
+ {Object.keys(metricLinks).length > 0 &&
Status
- +
} {details.metadata &&
Info
diff --git a/client/app/scripts/components/node-details/node-details-health-item.js b/client/app/scripts/components/node-details/node-details-health-item.js index 04767a00cd..fe572591cb 100644 --- a/client/app/scripts/components/node-details/node-details-health-item.js +++ b/client/app/scripts/components/node-details/node-details-health-item.js @@ -6,13 +6,17 @@ import { formatMetric } from '../../utils/string-utils'; function NodeDetailsHealthItem(props) { return (
-
{formatMetric(props.value, props)}
-
+ {props.value !== undefined &&
{formatMetric(props.value, props)}
} + {props.samples &&
+
} + {!props.samples &&
} +
+ {props.label} + {props.icon && }
-
{props.label}
); } diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js new file mode 100644 index 0000000000..a70ef35bf6 --- /dev/null +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -0,0 +1,47 @@ +import React from 'react'; + +import NodeDetailsHealthItem from './node-details-health-item'; + +export default class NodeDetailsHealthLinkItem extends React.Component { + + constructor(props, context) { + super(props, context); + + this.handleClick = this.handleClick.bind(this); + this.buildHref = this.buildHref.bind(this); + } + + handleClick(ev, href) { + ev.preventDefault(); + if (!href) return; + + const {router} = this.props; + if (router && href[0] === '/') { + router.push(href); + } else { + location.href = href; + } + } + + buildHref(url) { + if (!url || !this.props.isCloud) return url; + return url.replace(/:orgid/gi, encodeURIComponent(this.props.params.orgId)); + } + + render() { + const {links, withoutGraph, ...props} = this.props; + const href = this.buildHref(links[props.id] && links[props.id].url); + + if (!href) return ; + + return ( + this.handleClick(e, href)}> + {!withoutGraph && } + {withoutGraph && } + + ); + } +} diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index c515562643..193e1dc6d1 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -2,7 +2,8 @@ import React from 'react'; import ShowMore from '../show-more'; import NodeDetailsHealthOverflow from './node-details-health-overflow'; -import NodeDetailsHealthItem from './node-details-health-item'; +import NodeDetailsHealthLinkItem from './node-details-health-link-item'; +import CloudFeature from '../cloud-feature'; export default class NodeDetailsHealth extends React.Component { @@ -21,6 +22,9 @@ export default class NodeDetailsHealth extends React.Component { render() { const metrics = this.props.metrics || []; + const metricLinks = this.props.metricLinks || {}; + const unattachedLinks = this.props.unattachedLinks || {}; + const hasUnattached = Object.keys(unattachedLinks).length > 0; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; const primeMetrics = metrics.slice(0, primeCutoff); const overflowMetrics = metrics.slice(primeCutoff); @@ -32,7 +36,12 @@ export default class NodeDetailsHealth extends React.Component { return (
- {primeMetrics.map(item => )} + {primeMetrics.map(item => + + )} {showOverflow && + + {hasUnattached &&
+ {Object.keys(unattachedLinks).map(id => + + )} +
}
); } diff --git a/client/app/styles/_base.scss b/client/app/styles/_base.scss index 362097b08c..3a2e0cf13a 100644 --- a/client/app/styles/_base.scss +++ b/client/app/styles/_base.scss @@ -935,12 +935,31 @@ color: $text-secondary-color; text-transform: uppercase; font-size: 80%; + + .fa { + margin-left: 0.5em; + } } - &-value { - color: $text-secondary-color; - font-size: 150%; - padding-bottom: 0.5em; + &-placeholder { + font-size: 200%; + opacity: 0.2; + margin-bottom: 0.2em; + } + } + + &-link-item { + @extend .btn-opacity; + cursor: pointer; + opacity: $link-opacity-default; + width: 33%; + + .label { + text-transform: uppercase; + } + + .node-details-health-item { + width: auto; } } } diff --git a/prog/app.go b/prog/app.go index 5b06948d4f..2647ddb41a 100644 --- a/prog/app.go +++ b/prog/app.go @@ -18,17 +18,18 @@ import ( "github.com/gorilla/mux" "github.com/prometheus/client_golang/prometheus" "github.com/tylerb/graceful" - "github.com/weaveworks/go-checkpoint" - "github.com/weaveworks/scope/common/xfer" - "github.com/weaveworks/weave/common" billing "github.com/weaveworks/billing-client" "github.com/weaveworks/common/middleware" "github.com/weaveworks/common/network" + "github.com/weaveworks/go-checkpoint" "github.com/weaveworks/scope/app" "github.com/weaveworks/scope/app/multitenant" "github.com/weaveworks/scope/common/weave" + "github.com/weaveworks/scope/common/xfer" "github.com/weaveworks/scope/probe/docker" + "github.com/weaveworks/scope/render/detailed" + "github.com/weaveworks/weave/common" ) const ( @@ -216,6 +217,7 @@ func appMain(flags appFlags) { setLogLevel(flags.logLevel) setLogFormatter(flags.logPrefix) runtime.SetBlockProfileRate(flags.blockProfileRate) + detailed.SetMetricsGraphURL(flags.metricsGraphURL) defer log.Info("app exiting") rand.Seed(time.Now().UnixNano()) diff --git a/prog/main.go b/prog/main.go index 1c14773fad..b75e53b79e 100644 --- a/prog/main.go +++ b/prog/main.go @@ -160,6 +160,7 @@ type appFlags struct { memcachedCompressionLevel int userIDHeader string externalUI bool + metricsGraphURL string blockProfileRate int @@ -355,6 +356,7 @@ func setupFlags(flags *flags) { flag.IntVar(&flags.app.memcachedCompressionLevel, "app.memcached.compression", gzip.DefaultCompression, "How much to compress reports stored in memcached.") flag.StringVar(&flags.app.userIDHeader, "app.userid.header", "", "HTTP header to use as userid") flag.BoolVar(&flags.app.externalUI, "app.externalUI", false, "Point to externally hosted static UI assets") + flag.StringVar(&flags.app.metricsGraphURL, "app.metrics-graph", "", "Enable extended metrics graph by providing a templated URL (supports :orgID and :query)") flag.IntVar(&flags.app.blockProfileRate, "app.block.profile.rate", 0, "If more than 0, enable block profiling. The profiler aims to sample an average of one blocking event per rate nanoseconds spent blocked.") diff --git a/prog/probe.go b/prog/probe.go index a3f9806d55..3eed53cff1 100644 --- a/prog/probe.go +++ b/prog/probe.go @@ -13,11 +13,10 @@ import ( log "github.com/Sirupsen/logrus" "github.com/armon/go-metrics" "github.com/prometheus/client_golang/prometheus" - "github.com/weaveworks/go-checkpoint" - "github.com/weaveworks/weave/common" "github.com/weaveworks/common/network" "github.com/weaveworks/common/sanitize" + "github.com/weaveworks/go-checkpoint" "github.com/weaveworks/scope/common/hostname" "github.com/weaveworks/scope/common/weave" "github.com/weaveworks/scope/common/xfer" @@ -33,6 +32,7 @@ import ( "github.com/weaveworks/scope/probe/plugins" "github.com/weaveworks/scope/probe/process" "github.com/weaveworks/scope/report" + "github.com/weaveworks/weave/common" ) const ( diff --git a/render/detailed/links.go b/render/detailed/links.go new file mode 100644 index 0000000000..f811fc429a --- /dev/null +++ b/render/detailed/links.go @@ -0,0 +1,167 @@ +package detailed + +import ( + "bytes" + "encoding/json" + "net/url" + "strings" + "text/template" + + "github.com/weaveworks/scope/probe/docker" + "github.com/weaveworks/scope/report" +) + +// MetricLink describes a link referencing a metric. +type MetricLink struct { + // References the metric id + ID string `json:"id,omitempty"` + Label string `json:"label"` + URL string `json:"url"` + Priority int `json:"priority"` +} + +// Variable name for the query within the metrics graph url +const urlQueryVarName = ":query" + +var ( + // As configured by the user + metricsGraphURL = "" + + // Available metric links + linkTemplates = []MetricLink{ + {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1}, + {ID: docker.MemoryUsage, Label: "Memory", Priority: 2}, + } + + // Prometheus queries for topologies + topologyQueries = map[string]map[string]*template.Template{ + report.Pod: { + docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), + docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), + }, + report.ReplicaSet: { + docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-.+"})`), + docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-.+"}[1m]))`), + }, + report.Deployment: { + docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[0-9]+-[^-]+"})`), + docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[0-9]+-[^-]+"}[1m]))`), + }, + report.DaemonSet: { + docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), + docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + }, + report.Service: { + docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), + docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + }, + } +) + +// SetMetricsGraphURL sets the URL we deduce our eventual metric link from. +// Supports placeholders such as `:orgID` and `:query`. An empty url disables +// this feature. If the `:query` part is missing, a JSON version will be +// appended, see `queryParamsAsJSON()` for more info. +func SetMetricsGraphURL(url string) { + metricsGraphURL = url +} + +// NodeLinks returns the links of a node. The links are collected +// by a predefined set but filtered depending on whether a query +// is configured or not for the particular topology. +func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { + if metricsGraphURL == "" { + return nil + } + + queries := topologyQueries[n.Topology] + if len(queries) == 0 { + return nil + } + + links := []MetricLink{} + for _, link := range linkTemplates { + if _, ok := queries[link.ID]; ok { + links = append(links, link) + } + } + + return links +} + +// RenderLinks executes the templated links by supplying the node summary as data. +// It returns the modified summary. +func RenderMetricLinks(summary NodeSummary, n report.Node) NodeSummary { + queries := topologyQueries[n.Topology] + if len(queries) == 0 || len(summary.MetricLinks) == 0 { + return summary + } + + links := []MetricLink{} + var bs bytes.Buffer + for _, link := range summary.MetricLinks { + tpl := queries[link.ID] + if tpl == nil { + continue + } + + bs.Reset() + if err := tpl.Execute(&bs, summary); err != nil { + continue + } + + link.URL = buildURL(bs.String()) + links = append(links, link) + } + summary.MetricLinks = links + + return summary +} + +// buildURL puts together the URL by looking at the configured +// `metricsGraphURL`. +func buildURL(query string) string { + if strings.Contains(metricsGraphURL, urlQueryVarName) { + return strings.Replace(metricsGraphURL, urlQueryVarName, url.PathEscape(query), -1) + } + + params, err := queryParamsAsJSON(query) + if err != nil { + return "" + } + + if metricsGraphURL[len(metricsGraphURL)-1] != '/' { + metricsGraphURL += "/" + } + + return metricsGraphURL + url.PathEscape(params) +} + +// queryParamsAsJSON packs the query into a JSON of the +// format `{"cells":[{"queries":[$query]}]}`. +func queryParamsAsJSON(query string) (string, error) { + type cell struct { + Queries []string `json:"queries"` + } + type queryParams struct { + Cells []cell `json:"cells"` + } + params := &queryParams{[]cell{{[]string{query}}}} + + bs, err := json.Marshal(params) + if err != nil { + return "", err + } + + return string(bs), nil +} + +// prepareTemplate initializes unnamed text templates. +func prepareTemplate(query string) *template.Template { + tpl, err := template.New("").Parse(query) + if err != nil { + panic(err) + } + + return tpl +} diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go new file mode 100644 index 0000000000..f17f4d9a9d --- /dev/null +++ b/render/detailed/links_test.go @@ -0,0 +1,76 @@ +package detailed_test + +import ( + "testing" + + "github.com/weaveworks/scope/probe/docker" + "github.com/weaveworks/scope/render/detailed" + "github.com/weaveworks/scope/report" + "github.com/weaveworks/scope/test/fixture" + + "github.com/stretchr/testify/assert" +) + +func TestNodeMetricLinks_DefaultDisabled(t *testing.T) { + links := detailed.NodeMetricLinks(fixture.Report, fixture.Report.Pod.Nodes[fixture.ClientPodNodeID]) + assert.Nil(t, links) +} + +func TestNodeMetricLinks_UnknownTopology(t *testing.T) { + detailed.SetMetricsGraphURL("/foo") + node := report.MakeNode("foo").WithTopology("bar") + + links := detailed.NodeMetricLinks(report.Report{}, node) + assert.Nil(t, links) +} + +func TestNodeMetricLinks(t *testing.T) { + detailed.SetMetricsGraphURL("/foo") + defer detailed.SetMetricsGraphURL("") + node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] + expected := []detailed.MetricLink{ + {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1, URL: ""}, + {ID: docker.MemoryUsage, Label: "Memory", Priority: 2, URL: ""}, + } + + links := detailed.NodeMetricLinks(fixture.Report, node) + assert.Equal(t, expected, links) +} + +func TestRenderMetricLinks_UnknownTopology(t *testing.T) { + summary := detailed.NodeSummary{} + node := report.MakeNode("foo").WithTopology("bar") + + result := detailed.RenderMetricLinks(summary, node) + assert.Equal(t, summary, result) +} + +func TestRenderMetricLinks_Pod(t *testing.T) { + detailed.SetMetricsGraphURL("/prom/:orgID/notebook/new") + defer detailed.SetMetricsGraphURL("") + node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] + summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(fixture.Report, node)} + + result := detailed.RenderMetricLinks(summary, node) + assert.Equal(t, + "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%5C%22woo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D", + result.MetricLinks[0].URL) + assert.Equal(t, + "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28container_memory_usage_bytes%7Bpod_name=%5C%22woo%5C%22%7D%29%22%5D%7D%5D%7D", + result.MetricLinks[1].URL) +} + +func TestRenderMetricLinks_QueryReplacement(t *testing.T) { + detailed.SetMetricsGraphURL("/foo/:orgID/bar?q=:query") + defer detailed.SetMetricsGraphURL("") + node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] + summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(fixture.Report, node)} + + result := detailed.RenderMetricLinks(summary, node) + assert.Equal(t, + "/foo/:orgID/bar?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%22boo%22%7D%5B1m%5D%29%29", + result.MetricLinks[0].URL) + assert.Equal(t, + "/foo/:orgID/bar?q=sum%28container_memory_usage_bytes%7Bpod_name=%22boo%22%7D%29", + result.MetricLinks[1].URL) +} diff --git a/render/detailed/summary.go b/render/detailed/summary.go index de46963ccd..5a0a8d115e 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -44,19 +44,20 @@ type Column struct { // NodeSummary is summary information about a child for a Node. type NodeSummary struct { - ID string `json:"id"` - Label string `json:"label"` - LabelMinor string `json:"labelMinor"` - Rank string `json:"rank"` - Shape string `json:"shape,omitempty"` - Stack bool `json:"stack,omitempty"` - Linkable bool `json:"linkable,omitempty"` // Whether this node can be linked-to - Pseudo bool `json:"pseudo,omitempty"` - Metadata []report.MetadataRow `json:"metadata,omitempty"` - Parents []Parent `json:"parents,omitempty"` - Metrics []report.MetricRow `json:"metrics,omitempty"` - Tables []report.Table `json:"tables,omitempty"` - Adjacency report.IDList `json:"adjacency,omitempty"` + ID string `json:"id"` + Label string `json:"label"` + LabelMinor string `json:"labelMinor"` + Rank string `json:"rank"` + Shape string `json:"shape,omitempty"` + Stack bool `json:"stack,omitempty"` + Linkable bool `json:"linkable,omitempty"` // Whether this node can be linked-to + Pseudo bool `json:"pseudo,omitempty"` + Metadata []report.MetadataRow `json:"metadata,omitempty"` + Parents []Parent `json:"parents,omitempty"` + Metrics []report.MetricRow `json:"metrics,omitempty"` + Tables []report.Table `json:"tables,omitempty"` + Adjacency report.IDList `json:"adjacency,omitempty"` + MetricLinks []MetricLink `json:"metric_links,omitempty"` } var renderers = map[string]func(NodeSummary, report.Node) (NodeSummary, bool){ @@ -106,7 +107,8 @@ func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) { if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { - return renderer(baseNodeSummary(r, n), n) + summary, b := renderer(baseNodeSummary(r, n), n) + return RenderMetricLinks(summary, n), b } } else if _, ok := r.Topology(n.Topology); ok { summary := baseNodeSummary(r, n) @@ -133,14 +135,15 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { func baseNodeSummary(r report.Report, n report.Node) NodeSummary { t, _ := r.Topology(n.Topology) return NodeSummary{ - ID: n.ID, - Shape: t.GetShape(), - Linkable: true, - Metadata: NodeMetadata(r, n), - Metrics: NodeMetrics(r, n), - Parents: Parents(r, n), - Tables: NodeTables(r, n), - Adjacency: n.Adjacency, + ID: n.ID, + Shape: t.GetShape(), + Linkable: true, + Metadata: NodeMetadata(r, n), + Metrics: NodeMetrics(r, n), + MetricLinks: NodeMetricLinks(r, n), + Parents: Parents(r, n), + Tables: NodeTables(r, n), + Adjacency: n.Adjacency, } } diff --git a/render/expected/expected.go b/render/expected/expected.go index f687f501db..3024f31275 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -9,7 +9,6 @@ import ( "github.com/weaveworks/scope/test/fixture" ) -// Exported for testing. var ( circle = "circle" square = "square" From 14bf0c6efa16c0a92849f77df050d6afc6b4ec5b Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 14:15:49 +0200 Subject: [PATCH 02/31] Fix linting, cleanup, and add example to flag --- .../components/node-details/node-details-health-link-item.js | 4 ++-- prog/main.go | 2 +- render/detailed/links.go | 4 ++-- render/expected/expected.go | 1 + 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index a70ef35bf6..a536b51d88 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -29,8 +29,8 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } render() { - const {links, withoutGraph, ...props} = this.props; - const href = this.buildHref(links[props.id] && links[props.id].url); + const {links, withoutGraph, id, ...props} = this.props; + const href = this.buildHref(links[id] && links[id].url); if (!href) return ; diff --git a/prog/main.go b/prog/main.go index b75e53b79e..a5e52de7fe 100644 --- a/prog/main.go +++ b/prog/main.go @@ -356,7 +356,7 @@ func setupFlags(flags *flags) { flag.IntVar(&flags.app.memcachedCompressionLevel, "app.memcached.compression", gzip.DefaultCompression, "How much to compress reports stored in memcached.") flag.StringVar(&flags.app.userIDHeader, "app.userid.header", "", "HTTP header to use as userid") flag.BoolVar(&flags.app.externalUI, "app.externalUI", false, "Point to externally hosted static UI assets") - flag.StringVar(&flags.app.metricsGraphURL, "app.metrics-graph", "", "Enable extended metrics graph by providing a templated URL (supports :orgID and :query)") + flag.StringVar(&flags.app.metricsGraphURL, "app.metrics-graph", "", "Enable extended metrics graph by providing a templated URL (supports :orgID and :query). Example: --app.metric-graph=/prom/:orgID/notebook/new") flag.IntVar(&flags.app.blockProfileRate, "app.block.profile.rate", 0, "If more than 0, enable block profiling. The profiler aims to sample an average of one blocking event per rate nanoseconds spent blocked.") diff --git a/render/detailed/links.go b/render/detailed/links.go index f811fc429a..13f8c52e01 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -66,7 +66,7 @@ func SetMetricsGraphURL(url string) { metricsGraphURL = url } -// NodeLinks returns the links of a node. The links are collected +// NodeMetricLinks returns the links of a node. The links are collected // by a predefined set but filtered depending on whether a query // is configured or not for the particular topology. func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { @@ -89,7 +89,7 @@ func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { return links } -// RenderLinks executes the templated links by supplying the node summary as data. +// RenderMetricLinks executes the templated links by supplying the node summary as data. // It returns the modified summary. func RenderMetricLinks(summary NodeSummary, n report.Node) NodeSummary { queries := topologyQueries[n.Topology] diff --git a/render/expected/expected.go b/render/expected/expected.go index 3024f31275..f687f501db 100644 --- a/render/expected/expected.go +++ b/render/expected/expected.go @@ -9,6 +9,7 @@ import ( "github.com/weaveworks/scope/test/fixture" ) +// Exported for testing. var ( circle = "circle" square = "square" From aff3295279a64a78c40c94be6abbef02e75ecd1a Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 14:16:04 +0200 Subject: [PATCH 03/31] Remove unnecessary fixture dependency of tests --- render/detailed/links_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index f17f4d9a9d..5bb4e37045 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -6,52 +6,53 @@ import ( "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/render/detailed" "github.com/weaveworks/scope/report" - "github.com/weaveworks/scope/test/fixture" "github.com/stretchr/testify/assert" ) +var ( + sampleReport = report.Report{} + samplePodNode = report.MakeNode("noo").WithTopology(report.Pod) + sampleUnknownNode = report.MakeNode("???").WithTopology("foo") +) + func TestNodeMetricLinks_DefaultDisabled(t *testing.T) { - links := detailed.NodeMetricLinks(fixture.Report, fixture.Report.Pod.Nodes[fixture.ClientPodNodeID]) + links := detailed.NodeMetricLinks(sampleReport, samplePodNode) assert.Nil(t, links) } func TestNodeMetricLinks_UnknownTopology(t *testing.T) { detailed.SetMetricsGraphURL("/foo") - node := report.MakeNode("foo").WithTopology("bar") - links := detailed.NodeMetricLinks(report.Report{}, node) + links := detailed.NodeMetricLinks(sampleReport, sampleUnknownNode) assert.Nil(t, links) } func TestNodeMetricLinks(t *testing.T) { detailed.SetMetricsGraphURL("/foo") defer detailed.SetMetricsGraphURL("") - node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] expected := []detailed.MetricLink{ {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1, URL: ""}, {ID: docker.MemoryUsage, Label: "Memory", Priority: 2, URL: ""}, } - links := detailed.NodeMetricLinks(fixture.Report, node) + links := detailed.NodeMetricLinks(sampleReport, samplePodNode) assert.Equal(t, expected, links) } func TestRenderMetricLinks_UnknownTopology(t *testing.T) { summary := detailed.NodeSummary{} - node := report.MakeNode("foo").WithTopology("bar") - result := detailed.RenderMetricLinks(summary, node) + result := detailed.RenderMetricLinks(summary, sampleUnknownNode) assert.Equal(t, summary, result) } func TestRenderMetricLinks_Pod(t *testing.T) { detailed.SetMetricsGraphURL("/prom/:orgID/notebook/new") defer detailed.SetMetricsGraphURL("") - node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] - summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(fixture.Report, node)} + summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} - result := detailed.RenderMetricLinks(summary, node) + result := detailed.RenderMetricLinks(summary, samplePodNode) assert.Equal(t, "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%5C%22woo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D", result.MetricLinks[0].URL) @@ -63,10 +64,9 @@ func TestRenderMetricLinks_Pod(t *testing.T) { func TestRenderMetricLinks_QueryReplacement(t *testing.T) { detailed.SetMetricsGraphURL("/foo/:orgID/bar?q=:query") defer detailed.SetMetricsGraphURL("") - node := fixture.Report.Pod.Nodes[fixture.ClientPodNodeID] - summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(fixture.Report, node)} + summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} - result := detailed.RenderMetricLinks(summary, node) + result := detailed.RenderMetricLinks(summary, samplePodNode) assert.Equal(t, "/foo/:orgID/bar?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%22boo%22%7D%5B1m%5D%29%29", result.MetricLinks[0].URL) From d20e7be848812e1b0f136355108615f8db11d570 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 14:23:25 +0200 Subject: [PATCH 04/31] Use ugorji over core json --- render/detailed/links.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 13f8c52e01..5fb4e181c9 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -2,13 +2,14 @@ package detailed import ( "bytes" - "encoding/json" "net/url" "strings" "text/template" "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/report" + + "github.com/ugorji/go/codec" ) // MetricLink describes a link referencing a metric. @@ -148,12 +149,13 @@ func queryParamsAsJSON(query string) (string, error) { } params := &queryParams{[]cell{{[]string{query}}}} - bs, err := json.Marshal(params) - if err != nil { + buf := &bytes.Buffer{} + encoder := codec.NewEncoder(buf, &codec.JsonHandle{}) + if err := encoder.Encode(params); err != nil { return "", err } - return string(bs), nil + return buf.String(), nil } // prepareTemplate initializes unnamed text templates. From bcc06178ea160631930943fc6846db887a978d58 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 16:36:41 +0200 Subject: [PATCH 05/31] Track health graph click in mixpanel (&& cleanup) --- client/app/scripts/components/node-details.js | 3 ++- .../node-details/node-details-health-link-item.js | 7 +++++-- .../scripts/components/node-details/node-details-health.js | 5 +++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 8ffc7aeddb..27e7da1a33 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -158,7 +158,7 @@ class NodeDetails extends React.Component { } renderDetails() { - const { details, nodeControlStatus, nodeMatches = makeMap() } = this.props; + const { details, nodeControlStatus, nodeMatches = makeMap(), topologyId } = this.props; const showControls = details.controls && details.controls.length > 0; const nodeColor = getNodeColorDark(details.rank, details.label, details.pseudo); const {error, pending} = nodeControlStatus ? nodeControlStatus.toJS() : {}; @@ -211,6 +211,7 @@ class NodeDetails extends React.Component { metrics={details.metrics} metricLinks={metricLinks} unattachedLinks={unattachedLinks} + topologyId={topologyId} />
} {details.metadata &&
diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index a536b51d88..be020e3938 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -1,5 +1,6 @@ import React from 'react'; +import { trackMixpanelEvent } from '../../utils/tracking-utils'; import NodeDetailsHealthItem from './node-details-health-item'; export default class NodeDetailsHealthLinkItem extends React.Component { @@ -15,7 +16,9 @@ export default class NodeDetailsHealthLinkItem extends React.Component { ev.preventDefault(); if (!href) return; - const {router} = this.props; + const { router, topologyId } = this.props; + trackMixpanelEvent('scope.node.health.click', { topologyId }); + if (router && href[0] === '/') { router.push(href); } else { @@ -29,7 +32,7 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } render() { - const {links, withoutGraph, id, ...props} = this.props; + const { links, withoutGraph, id, ...props } = this.props; const href = this.buildHref(links[id] && links[id].url); if (!href) return ; diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index 193e1dc6d1..8ec61817b2 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -21,8 +21,7 @@ export default class NodeDetailsHealth extends React.Component { } render() { - const metrics = this.props.metrics || []; - const metricLinks = this.props.metricLinks || {}; + const { metrics = [], metricLinks = {}, topologyId } = this.props; const unattachedLinks = this.props.unattachedLinks || {}; const hasUnattached = Object.keys(unattachedLinks).length > 0; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; @@ -40,6 +39,7 @@ export default class NodeDetailsHealth extends React.Component { )} {showOverflow && )}
} From c483ef17f38076c072f9e29bf7ed21bb244fab71 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 29 Jun 2017 17:25:01 +0200 Subject: [PATCH 06/31] Initialize props with immutable map/list --- .../components/node-details/node-details-health.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index 8ec61817b2..6f45d5389e 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -1,4 +1,5 @@ import React from 'react'; +import { Map as makeMap, List as makeList } from 'immutable'; import ShowMore from '../show-more'; import NodeDetailsHealthOverflow from './node-details-health-overflow'; @@ -21,8 +22,13 @@ export default class NodeDetailsHealth extends React.Component { } render() { - const { metrics = [], metricLinks = {}, topologyId } = this.props; - const unattachedLinks = this.props.unattachedLinks || {}; + const { + metrics = makeList(), + metricLinks = makeMap(), + unattachedLinks = makeMap(), + topologyId, + } = this.props; + const hasUnattached = Object.keys(unattachedLinks).length > 0; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; const primeMetrics = metrics.slice(0, primeCutoff); From 2f5fe24de192118367c0eab629cbe41c236ae2db Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 4 Jul 2017 12:02:44 +0200 Subject: [PATCH 07/31] Improve comments; fix missing links w/o metrics --- client/app/scripts/components/node-details.js | 2 +- .../node-details/node-details-health.js | 5 ++--- render/detailed/links.go | 15 ++++++++++----- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 27e7da1a33..0239394850 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -205,7 +205,7 @@ class NodeDetails extends React.Component {
}
- {Object.keys(metricLinks).length > 0 &&
+ {((details.metrics || []).length + Object.keys(unattachedLinks).length > 0) &&
Status
0; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; const primeMetrics = metrics.slice(0, primeCutoff); const overflowMetrics = metrics.slice(primeCutoff); @@ -57,7 +56,7 @@ export default class NodeDetailsHealth extends React.Component { handleClick={this.handleClickMore} collection={this.props.metrics} expanded={this.state.expanded} notShown={notShown} hideNumber /> - {hasUnattached &&
+
{Object.keys(unattachedLinks).map(id => )} -
} +
); } diff --git a/render/detailed/links.go b/render/detailed/links.go index 5fb4e181c9..ba2df99c3b 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -12,7 +12,7 @@ import ( "github.com/ugorji/go/codec" ) -// MetricLink describes a link referencing a metric. +// MetricLink describes a URL referencing a metric. type MetricLink struct { // References the metric id ID string `json:"id,omitempty"` @@ -37,22 +37,27 @@ var ( // Prometheus queries for topologies topologyQueries = map[string]map[string]*template.Template{ report.Pod: { + // Metric names provided by cAdvisor in kubelet docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), }, report.ReplicaSet: { - docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-.+"})`), - docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-.+"}[1m]))`), + // Metric names provided by cAdvisor in kubelet + docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]"})`), + docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]"}[1m]))`), }, report.Deployment: { - docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[0-9]+-[^-]+"})`), - docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[0-9]+-[^-]+"}[1m]))`), + // Metric names provided by cAdvisor in kubelet + docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), + docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]+"}[1m]))`), }, report.DaemonSet: { + // Metric names defined as recording rule. Filters by `monitor=""` for cortex-only data. docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, report.Service: { + // Metric names defined as recording rule. Filters by `monitor=""` for cortex-only data. docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, From 4985b454afa994078a1f3d2d37d6a0850ec59754 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 11 Jul 2017 14:05:12 +0200 Subject: [PATCH 08/31] New design for hover states and overflow handling --- client/app/scripts/components/node-details.js | 36 +++++++++++++------ .../node-details/node-details-health-item.js | 7 ++-- .../node-details-health-link-item.js | 35 +++++++++++++++--- .../node-details-health-overflow-item.js | 2 +- .../node-details-health-overflow.js | 2 +- .../node-details/node-details-health.js | 14 ++------ client/app/scripts/utils/metric-utils.js | 4 ++- client/app/styles/_base.scss | 3 ++ 8 files changed, 69 insertions(+), 34 deletions(-) diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 0239394850..04f06ec6e5 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -56,6 +56,28 @@ class NodeDetails extends React.Component { resetDocumentTitle(); } + static collectMetrics(details) { + const metrics = details.metrics || []; + + // collect by metric id (id => link) + const metricLinks = (details.metric_links || []) + .reduce((agg, link) => Object.assign(agg, {[link.id]: link}), {}); + + const availableMetrics = metrics.reduce( + (agg, m) => Object.assign(agg, {[m.id]: true}), + {} + ); + + // append links with no metrics as fake metrics + (details.metric_links || []).forEach((link) => { + if (availableMetrics[link.id] === undefined) { + metrics.push({id: link.id, label: link.label}); + } + }); + + return { metrics, metricLinks }; + } + renderTools() { const showSwitchTopology = this.props.nodeId !== this.props.selectedNodeId; const topologyTitle = `View ${this.props.label} in ${this.props.topologyId}`; @@ -172,13 +194,7 @@ class NodeDetails extends React.Component { } }; - // collect by metric id (id => link) - const metricLinks = (details.metric_links || []) - .reduce((agg, link) => Object.assign(agg, {[link.id]: link}), {}); - - // collect links with no corresponding metric - const unattachedLinks = Object.assign({}, metricLinks); - (details.metrics || []).forEach(metric => delete unattachedLinks[metric.id]); + const { metrics, metricLinks } = NodeDetails.collectMetrics(details); return (
@@ -205,13 +221,13 @@ class NodeDetails extends React.Component {
}
- {((details.metrics || []).length + Object.keys(unattachedLinks).length > 0) &&
+ {metrics.length > 0 &&
Status
} {details.metadata &&
diff --git a/client/app/scripts/components/node-details/node-details-health-item.js b/client/app/scripts/components/node-details/node-details-health-item.js index fe572591cb..89cb1131a3 100644 --- a/client/app/scripts/components/node-details/node-details-health-item.js +++ b/client/app/scripts/components/node-details/node-details-health-item.js @@ -10,12 +10,11 @@ function NodeDetailsHealthItem(props) { {props.samples &&
+ first={props.first} last={props.last} strokeWidth={props.strokeWidth} + strokeColor={props.strokeColor} />
} - {!props.samples &&
} -
+
{props.label} - {props.icon && }
); diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index be020e3938..690ba5854f 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -1,15 +1,29 @@ import React from 'react'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; +import { getMetricColor } from '../../utils/metric-utils'; import NodeDetailsHealthItem from './node-details-health-item'; export default class NodeDetailsHealthLinkItem extends React.Component { constructor(props, context) { super(props, context); + this.state = { + hovered: false + }; this.handleClick = this.handleClick.bind(this); this.buildHref = this.buildHref.bind(this); + this.onMouseOver = this.onMouseOver.bind(this); + this.onMouseOut = this.onMouseOut.bind(this); + } + + onMouseOver() { + this.setState({hovered: true}); + } + + onMouseOut() { + this.setState({hovered: false}); } handleClick(ev, href) { @@ -32,18 +46,29 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } render() { - const { links, withoutGraph, id, ...props } = this.props; + const { links, id, nodeColor, ...props } = this.props; const href = this.buildHref(links[id] && links[id].url); - if (!href) return ; + const hasData = (props.samples && props.samples.length > 0) || props.value !== undefined; + const labelColor = this.state.hovered && !hasData ? nodeColor : undefined; + const sparkline = {}; + if (this.state.hovered) { + sparkline.strokeColor = getMetricColor(id); + sparkline.strokeWidth = '2px'; + } + return ( this.handleClick(e, href)}> - {!withoutGraph && } - {withoutGraph && } + onClick={e => this.handleClick(e, href)} + onMouseOver={this.onMouseOver} + onMouseOut={this.onMouseOut}> + ); } diff --git a/client/app/scripts/components/node-details/node-details-health-overflow-item.js b/client/app/scripts/components/node-details/node-details-health-overflow-item.js index 6f69bd2fab..36c6ff6de3 100644 --- a/client/app/scripts/components/node-details/node-details-health-overflow-item.js +++ b/client/app/scripts/components/node-details/node-details-health-overflow-item.js @@ -6,7 +6,7 @@ function NodeDetailsHealthOverflowItem(props) { return (
- {formatMetric(props.value, props)} + {props.value !== undefined && formatMetric(props.value, props)}
{props.label}
diff --git a/client/app/scripts/components/node-details/node-details-health-overflow.js b/client/app/scripts/components/node-details/node-details-health-overflow.js index 76fcea7f8c..537c3a6950 100644 --- a/client/app/scripts/components/node-details/node-details-health-overflow.js +++ b/client/app/scripts/components/node-details/node-details-health-overflow.js @@ -18,7 +18,7 @@ export default class NodeDetailsHealthOverflow extends React.Component { const items = this.props.items.slice(0, 4); return ( -
+
{items.map(item => )}
); diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index b2648fa253..e323d75bbb 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -25,8 +25,8 @@ export default class NodeDetailsHealth extends React.Component { const { metrics = makeList(), metricLinks = makeMap(), - unattachedLinks = makeMap(), topologyId, + nodeColor, } = this.props; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; @@ -45,6 +45,7 @@ export default class NodeDetailsHealth extends React.Component { {...item} links={metricLinks} topologyId={topologyId} + nodeColor={nodeColor} /> )} {showOverflow && - -
- {Object.keys(unattachedLinks).map(id => - - )} -
); } diff --git a/client/app/scripts/utils/metric-utils.js b/client/app/scripts/utils/metric-utils.js index bf14432e83..4350bddb6e 100644 --- a/client/app/scripts/utils/metric-utils.js +++ b/client/app/scripts/utils/metric-utils.js @@ -52,7 +52,9 @@ export function getMetricValue(metric) { export function getMetricColor(metric) { - const metricId = metric && metric.get('id'); + const metricId = typeof metric === 'string' + ? metric + : metric && metric.get('id'); if (/mem/.test(metricId)) { return 'steelBlue'; } else if (/cpu/.test(metricId)) { diff --git a/client/app/styles/_base.scss b/client/app/styles/_base.scss index 3a2e0cf13a..7f8d52f934 100644 --- a/client/app/styles/_base.scss +++ b/client/app/styles/_base.scss @@ -930,11 +930,14 @@ &-item { padding: 8px 16px; width: 33%; + display: flex; + flex-direction: column; &-label { color: $text-secondary-color; text-transform: uppercase; font-size: 80%; + margin-top: auto; .fa { margin-left: 0.5em; From c895b78ad1ded0f5be8bc2462ed94c5509d44d35 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 11 Jul 2017 14:13:51 +0200 Subject: [PATCH 09/31] Expand on the prom query source in comments --- render/detailed/links.go | 79 ++++++++++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index ba2df99c3b..51403cdcaf 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -37,29 +37,70 @@ var ( // Prometheus queries for topologies topologyQueries = map[string]map[string]*template.Template{ report.Pod: { - // Metric names provided by cAdvisor in kubelet - docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), - docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), - }, - report.ReplicaSet: { - // Metric names provided by cAdvisor in kubelet - docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]"})`), - docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]"}[1m]))`), + // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), + // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), }, report.Deployment: { - // Metric names provided by cAdvisor in kubelet - docker.MemoryUsage: prepareTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), - docker.CPUTotalUsage: prepareTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]+"}[1m]))`), + // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), + // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]+"}[1m]))`), }, report.DaemonSet: { - // Metric names defined as recording rule. Filters by `monitor=""` for cortex-only data. - docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), - docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + /* + A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: + + namespace_name:container_memory_usage_bytes:sum = + sum by (namespace, name) ( + sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + + Additionally, we filter by `monitor=""` for cortex-only data. + */ + + docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), + /* + A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: + + namespace_name:container_cpu_usage_seconds_total:sum_rate = + sum by (namespace, name) ( + sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + */ + docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, report.Service: { - // Metric names defined as recording rule. Filters by `monitor=""` for cortex-only data. - docker.MemoryUsage: prepareTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), - docker.CPUTotalUsage: prepareTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + /* + A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: + + namespace_name:container_memory_usage_bytes:sum = + sum by (namespace, name) ( + sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + + Additionally, we filter by `monitor=""` for cortex-only data. + */ + docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), + + /* + A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: + + namespace_name:container_cpu_usage_seconds_total:sum_rate = + sum by (namespace, name) ( + sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + */ + docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, } ) @@ -163,8 +204,8 @@ func queryParamsAsJSON(query string) (string, error) { return buf.String(), nil } -// prepareTemplate initializes unnamed text templates. -func prepareTemplate(query string) *template.Template { +// parsedTemplate initializes unnamed text templates. +func parsedTemplate(query string) *template.Template { tpl, err := template.New("").Parse(query) if err != nil { panic(err) From 6ac24799c38f44ae8d29459f6f9ed9001833b3d7 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 11 Jul 2017 14:50:59 +0200 Subject: [PATCH 10/31] Fix linting errors --- render/detailed/links.go | 80 ++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 51403cdcaf..3de365e23d 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -38,68 +38,68 @@ var ( topologyQueries = map[string]map[string]*template.Template{ report.Pod: { // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), }, report.Deployment: { // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]+"}[1m]))`), }, report.DaemonSet: { /* - A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: + A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: - namespace_name:container_memory_usage_bytes:sum = - sum by (namespace, name) ( - sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) + namespace_name:container_memory_usage_bytes:sum = + sum by (namespace, name) ( + sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) - Additionally, we filter by `monitor=""` for cortex-only data. - */ + Additionally, we filter by `monitor=""` for cortex-only data. + */ + docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), - docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), /* - A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: - - namespace_name:container_cpu_usage_seconds_total:sum_rate = - sum by (namespace, name) ( - sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - */ + A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: + + namespace_name:container_cpu_usage_seconds_total:sum_rate = + sum by (namespace, name) ( + sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + */ docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, report.Service: { /* - A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: + A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: - namespace_name:container_memory_usage_bytes:sum = - sum by (namespace, name) ( - sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) + namespace_name:container_memory_usage_bytes:sum = + sum by (namespace, name) ( + sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) - Additionally, we filter by `monitor=""` for cortex-only data. - */ - docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), + Additionally, we filter by `monitor=""` for cortex-only data. + */ + docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), /* - A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: - - namespace_name:container_cpu_usage_seconds_total:sum_rate = - sum by (namespace, name) ( - sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - */ + A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: + + namespace_name:container_cpu_usage_seconds_total:sum_rate = + sum by (namespace, name) ( + sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) + * on (pod_name) group_left(name) + k8s_pod_labels{job="monitoring/kube-api-exporter"} + ) + */ docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), }, } From b52701b45b3469972c0d9487ff991df2848257ca Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Wed, 12 Jul 2017 12:30:17 +0200 Subject: [PATCH 11/31] Avoid global by passing metricsGraphURL down --- app/api_report_test.go | 2 +- app/api_topologies.go | 12 +++++----- app/api_topologies_test.go | 4 ++-- app/api_topology.go | 11 ++++----- app/router.go | 8 +++---- prog/app.go | 8 +++---- render/detailed/connections.go | 12 +++++----- render/detailed/links.go | 25 +++++---------------- render/detailed/links_test.go | 19 +++------------- render/detailed/node.go | 14 ++++++------ render/detailed/node_test.go | 8 +++---- render/detailed/summary.go | 40 ++++++++++++++++++--------------- render/detailed/summary_test.go | 6 ++--- 13 files changed, 73 insertions(+), 96 deletions(-) diff --git a/app/api_report_test.go b/app/api_report_test.go index 1d7bd2b69f..c1da6f8002 100644 --- a/app/api_report_test.go +++ b/app/api_report_test.go @@ -14,7 +14,7 @@ import ( func topologyServer() *httptest.Server { router := mux.NewRouter().SkipClean(true) - app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}) + app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}, "") return httptest.NewServer(router) } diff --git a/app/api_topologies.go b/app/api_topologies.go index 42a62f8e12..a56a243856 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -548,17 +548,17 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt return topology.renderer, nil, nil } -type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) +type reporterHandler func(context.Context, Reporter, string, http.ResponseWriter, *http.Request) -func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { +func captureReporter(rep Reporter, metricsGraphURL string, f reporterHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - f(ctx, rep, w, r) + f(ctx, rep, metricsGraphURL, w, r) } } -type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, string, http.ResponseWriter, *http.Request) -func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { +func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( topologyID = mux.Vars(req)["topology"] @@ -579,6 +579,6 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, rpt, w, req) + f(ctx, renderer, decorator, rpt, metricGraphsURL, w, req) } } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index a817eb796c..71c6906399 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -183,14 +183,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator)), nil + return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator), ""), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) app.RegisterReportPostHandler(c, router) - app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}) + app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}, "") ts := httptest.NewServer(router) defer ts.Close() diff --git a/app/api_topology.go b/app/api_topology.go index 173e275b99..a29f052d13 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,14 +29,14 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(report, renderer.Render(report, decorator)), + Nodes: detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, decorator 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, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] @@ -49,13 +49,14 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node, metricsGraphURL)}) } // Websocket for the full topology. func handleWebsocket( ctx context.Context, rep Reporter, + metricsGraphURL string, w http.ResponseWriter, r *http.Request, ) { @@ -123,7 +124,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(report, renderer.Render(report, decorator)) + newTopo := detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/router.go b/app/router.go index 414f45f99f..05999d293e 100644 --- a/app/router.go +++ b/app/router.go @@ -91,7 +91,7 @@ func gzipHandler(h http.HandlerFunc) http.HandlerFunc { } // RegisterTopologyRoutes registers the various topology routes with a http mux. -func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool) { +func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool, metricsGraphURL string) { get := router.Methods("GET").Subrouter() get.HandleFunc("/api", gzipHandler(requestContextDecorator(apiHandler(r, capabilities)))) @@ -99,15 +99,15 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[str gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get. HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleTopology)))). Name("api_topology_topology") get. HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(captureReporter(r, handleWebsocket))). // NB not gzip! + requestContextDecorator(captureReporter(r, metricsGraphURL, handleWebsocket))). // NB not gzip! Name("api_topology_topology_ws") get. MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleNode)))). Name("api_topology_topology_id") get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) diff --git a/prog/app.go b/prog/app.go index 2647ddb41a..f78054c9b8 100644 --- a/prog/app.go +++ b/prog/app.go @@ -28,7 +28,6 @@ import ( "github.com/weaveworks/scope/common/weave" "github.com/weaveworks/scope/common/xfer" "github.com/weaveworks/scope/probe/docker" - "github.com/weaveworks/scope/render/detailed" "github.com/weaveworks/weave/common" ) @@ -52,7 +51,7 @@ func init() { } // Router creates the mux for all the various app components. -func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool) http.Handler { +func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool, metricsGraphURL string) http.Handler { router := mux.NewRouter().SkipClean(true) // We pull in the http.DefaultServeMux to get the pprof routes @@ -62,7 +61,7 @@ func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.RegisterReportPostHandler(collector, router) app.RegisterControlRoutes(router, controlRouter) app.RegisterPipeRoutes(router, pipeRouter) - app.RegisterTopologyRoutes(router, collector, capabilities) + app.RegisterTopologyRoutes(router, collector, capabilities, metricsGraphURL) uiHandler := http.FileServer(GetFS(externalUI)) router.PathPrefix("/ui").Name("static").Handler( @@ -217,7 +216,6 @@ func appMain(flags appFlags) { setLogLevel(flags.logLevel) setLogFormatter(flags.logPrefix) runtime.SetBlockProfileRate(flags.blockProfileRate) - detailed.SetMetricsGraphURL(flags.metricsGraphURL) defer log.Info("app exiting") rand.Seed(time.Now().UnixNano()) @@ -299,7 +297,7 @@ func appMain(flags appFlags) { capabilities := map[string]bool{ xfer.HistoricReportsCapability: collector.HasHistoricReports(), } - handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities) + handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities, flags.metricsGraphURL) if flags.logHTTP { handler = middleware.Log{ LogRequestHeaders: flags.logHTTPHeaders, diff --git a/render/detailed/connections.go b/render/detailed/connections.go index b89ebb723a..337e565acd 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -121,13 +121,13 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) { return addr, true } -func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { +func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool, metricsGraphURL string) []Connection { output := []Connection{} for row, count := range c.counts { // Use MakeNodeSummary to render the id and label of this node // TODO(paulbellamy): Would be cleaner if we hade just a // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. - summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID]) + summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID], metricsGraphURL) connection := Connection{ ID: fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port), NodeID: summary.ID, @@ -162,7 +162,7 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal return output } -func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { +func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(n) counts := newConnectionCounters() @@ -188,11 +188,11 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Inbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n)), + Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), } } -func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { +func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { localEndpoints := endpointChildrenOf(n) counts := newConnectionCounters() @@ -220,7 +220,7 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Outbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n)), + Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), } } diff --git a/render/detailed/links.go b/render/detailed/links.go index 3de365e23d..fa9084d4a6 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -15,7 +15,7 @@ import ( // MetricLink describes a URL referencing a metric. type MetricLink struct { // References the metric id - ID string `json:"id,omitempty"` + ID string `json:"id"` Label string `json:"label"` URL string `json:"url"` Priority int `json:"priority"` @@ -25,9 +25,6 @@ type MetricLink struct { const urlQueryVarName = ":query" var ( - // As configured by the user - metricsGraphURL = "" - // Available metric links linkTemplates = []MetricLink{ {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1}, @@ -105,22 +102,10 @@ var ( } ) -// SetMetricsGraphURL sets the URL we deduce our eventual metric link from. -// Supports placeholders such as `:orgID` and `:query`. An empty url disables -// this feature. If the `:query` part is missing, a JSON version will be -// appended, see `queryParamsAsJSON()` for more info. -func SetMetricsGraphURL(url string) { - metricsGraphURL = url -} - // NodeMetricLinks returns the links of a node. The links are collected // by a predefined set but filtered depending on whether a query // is configured or not for the particular topology. func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { - if metricsGraphURL == "" { - return nil - } - queries := topologyQueries[n.Topology] if len(queries) == 0 { return nil @@ -137,8 +122,10 @@ func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { } // RenderMetricLinks executes the templated links by supplying the node summary as data. +// `metricsGraphURL` supports placeholders such as `:orgID` and `:query`. If the `:query` +// part is missing, a JSON version will be appended, see `queryParamsAsJSON()` for more info. // It returns the modified summary. -func RenderMetricLinks(summary NodeSummary, n report.Node) NodeSummary { +func RenderMetricLinks(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { queries := topologyQueries[n.Topology] if len(queries) == 0 || len(summary.MetricLinks) == 0 { return summary @@ -157,7 +144,7 @@ func RenderMetricLinks(summary NodeSummary, n report.Node) NodeSummary { continue } - link.URL = buildURL(bs.String()) + link.URL = buildURL(bs.String(), metricsGraphURL) links = append(links, link) } summary.MetricLinks = links @@ -167,7 +154,7 @@ func RenderMetricLinks(summary NodeSummary, n report.Node) NodeSummary { // buildURL puts together the URL by looking at the configured // `metricsGraphURL`. -func buildURL(query string) string { +func buildURL(query, metricsGraphURL string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { return strings.Replace(metricsGraphURL, urlQueryVarName, url.PathEscape(query), -1) } diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index 5bb4e37045..29328d7d26 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -16,21 +16,12 @@ var ( sampleUnknownNode = report.MakeNode("???").WithTopology("foo") ) -func TestNodeMetricLinks_DefaultDisabled(t *testing.T) { - links := detailed.NodeMetricLinks(sampleReport, samplePodNode) - assert.Nil(t, links) -} - func TestNodeMetricLinks_UnknownTopology(t *testing.T) { - detailed.SetMetricsGraphURL("/foo") - links := detailed.NodeMetricLinks(sampleReport, sampleUnknownNode) assert.Nil(t, links) } func TestNodeMetricLinks(t *testing.T) { - detailed.SetMetricsGraphURL("/foo") - defer detailed.SetMetricsGraphURL("") expected := []detailed.MetricLink{ {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1, URL: ""}, {ID: docker.MemoryUsage, Label: "Memory", Priority: 2, URL: ""}, @@ -43,16 +34,14 @@ func TestNodeMetricLinks(t *testing.T) { func TestRenderMetricLinks_UnknownTopology(t *testing.T) { summary := detailed.NodeSummary{} - result := detailed.RenderMetricLinks(summary, sampleUnknownNode) + result := detailed.RenderMetricLinks(summary, sampleUnknownNode, "") assert.Equal(t, summary, result) } func TestRenderMetricLinks_Pod(t *testing.T) { - detailed.SetMetricsGraphURL("/prom/:orgID/notebook/new") - defer detailed.SetMetricsGraphURL("") summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} - result := detailed.RenderMetricLinks(summary, samplePodNode) + result := detailed.RenderMetricLinks(summary, samplePodNode, "/prom/:orgID/notebook/new") assert.Equal(t, "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%5C%22woo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D", result.MetricLinks[0].URL) @@ -62,11 +51,9 @@ func TestRenderMetricLinks_Pod(t *testing.T) { } func TestRenderMetricLinks_QueryReplacement(t *testing.T) { - detailed.SetMetricsGraphURL("/foo/:orgID/bar?q=:query") - defer detailed.SetMetricsGraphURL("") summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} - result := detailed.RenderMetricLinks(summary, samplePodNode) + result := detailed.RenderMetricLinks(summary, samplePodNode, "/foo/:orgID/bar?q=:query") assert.Equal(t, "/foo/:orgID/bar?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%22boo%22%7D%5B1m%5D%29%29", result.MetricLinks[0].URL) diff --git a/render/detailed/node.go b/render/detailed/node.go index 8e211b8138..1481d99d8c 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -80,15 +80,15 @@ func (c *ControlInstance) CodecDecodeSelf(decoder *codec.Decoder) { // 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, r report.Report, ns report.Nodes, n report.Node) Node { - summary, _ := MakeNodeSummary(r, n) +func MakeNode(topologyID string, r report.Report, ns report.Nodes, n report.Node, metricsGraphURL string) Node { + summary, _ := MakeNodeSummary(r, n, metricsGraphURL) return Node{ NodeSummary: summary, Controls: controls(r, n), - Children: children(r, n), + Children: children(r, n, metricsGraphURL), Connections: []ConnectionsSummary{ - incomingConnectionsSummary(topologyID, r, n, ns), - outgoingConnectionsSummary(topologyID, r, n, ns), + incomingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), + outgoingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), }, } } @@ -181,13 +181,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(r report.Report, n report.Node) []NodeSummaryGroup { +func children(r report.Report, n report.Node, metricsGraphURL string) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(r, child) + summary, ok := MakeNodeSummary(r, child, metricsGraphURL) if !ok { return } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index d7c213ae45..b612297599 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(fixture.Report, r.Render(fixture.Report, nil)[id]) + s, ok := detailed.MakeNodeSummary(fixture.Report, r.Render(fixture.Report, nil)[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, nil) renderableNode := renderableNodes[fixture.ClientHostNodeID] - have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode, "") containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) @@ -183,7 +183,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode, "") serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) serverProcessNodeSummary.Linkable = true @@ -313,7 +313,7 @@ func TestMakeDetailedPodNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("pods", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("pods", 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 5a0a8d115e..697e7ecdd6 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -103,20 +103,21 @@ var primaryAPITopology = map[string]string{ } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) { +func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (NodeSummary, bool) { + metricLinks := metricsGraphURL != "" if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { - summary, b := renderer(baseNodeSummary(r, n), n) - return RenderMetricLinks(summary, n), b + summary, b := renderer(baseNodeSummary(r, n, metricLinks), n) + return RenderMetricLinks(summary, n, metricsGraphURL), b } } else if _, ok := r.Topology(n.Topology); ok { - summary := baseNodeSummary(r, n) + summary := baseNodeSummary(r, n, metricLinks) summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback return summary, true } if strings.HasPrefix(n.Topology, "group:") { - return groupNodeSummary(baseNodeSummary(r, n), r, n) + return groupNodeSummary(baseNodeSummary(r, n, metricLinks), r, n) } return NodeSummary{}, false } @@ -132,19 +133,22 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { return n } -func baseNodeSummary(r report.Report, n report.Node) NodeSummary { +func baseNodeSummary(r report.Report, n report.Node, metricLinks bool) NodeSummary { t, _ := r.Topology(n.Topology) - return NodeSummary{ - ID: n.ID, - Shape: t.GetShape(), - Linkable: true, - Metadata: NodeMetadata(r, n), - Metrics: NodeMetrics(r, n), - MetricLinks: NodeMetricLinks(r, n), - Parents: Parents(r, n), - Tables: NodeTables(r, n), - Adjacency: n.Adjacency, + ns := NodeSummary{ + ID: n.ID, + Shape: t.GetShape(), + Linkable: true, + Metadata: NodeMetadata(r, n), + Metrics: NodeMetrics(r, n), + Parents: Parents(r, n), + Tables: NodeTables(r, n), + Adjacency: n.Adjacency, } + if metricLinks { + ns.MetricLinks = NodeMetricLinks(r, n) + } + return ns } func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) { @@ -373,11 +377,11 @@ 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(r report.Report, rns report.Nodes) NodeSummaries { +func Summaries(r report.Report, rns report.Nodes, metricsGraphURL string) NodeSummaries { result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(r, node); ok { + if summary, ok := MakeNodeSummary(r, node, metricsGraphURL); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index e4a5ad72d7..4bf843bc46 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(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil)) + have := detailed.Summaries(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil), "") // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -51,7 +51,7 @@ func TestSummaries(t *testing.T) { input := fixture.Report.Copy() input.Process.Nodes[fixture.ClientProcess1NodeID].Metrics[process.CPUUsage] = metric - have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil)) + have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil), "") node, ok := have[fixture.ClientProcess1NodeID] if !ok { @@ -184,7 +184,7 @@ func TestMakeNodeSummary(t *testing.T) { }, } for _, testcase := range testcases { - have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input) + have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input, "") if ok != testcase.ok { t.Errorf("%s: MakeNodeSummary failed: expected ok value to be: %v", testcase.name, testcase.ok) continue From 42f2054e12c9bf11b4b4f312789307bcb8d57ce0 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 13 Jul 2017 19:23:32 +0200 Subject: [PATCH 12/31] Update Deployment and DaemonSet queries --- render/detailed/links.go | 46 ++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index fa9084d4a6..0835186db1 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -41,37 +41,25 @@ var ( }, report.Deployment: { // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"{{.Label}}-[^-]+-[^-]+"})`), + // Pod names are automatically generated by k8s using the deployment name: + // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"})`), // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"{{.Label}}-[^-]+-[^-]+"}[1m]))`), + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"}[1m]))`), }, report.DaemonSet: { - /* - A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: - - namespace_name:container_memory_usage_bytes:sum = - sum by (namespace, name) ( - sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - - Additionally, we filter by `monitor=""` for cortex-only data. - */ - docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), - - /* - A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: - - namespace_name:container_cpu_usage_seconds_total:sum_rate = - sum by (namespace, name) ( - sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - */ - docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. + // Pod names are automatically generated by k8s using the DaemonSet name. + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+$"})`), + // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+$"}[1m]))`), }, + + /* + NB: requirements for the Service yaml are + - The `spec.selector` needs to select on `name` + - The `metadata.name` needs to be the same value as `spec.selector.name` + */ report.Service: { /* A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: @@ -96,8 +84,10 @@ var ( * on (pod_name) group_left(name) k8s_pod_labels{job="monitoring/kube-api-exporter"} ) + + Additionally, we filter by `monitor=""` for cortex-only data. */ - docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}}"}`), + docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}},monitor=""}`), }, } ) From 3722d557ce7eaf5980482edd555afb2fffff8652 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 17 Jul 2017 14:02:51 +0200 Subject: [PATCH 13/31] Switch to migrated rules in kube-state-metrics --- render/detailed/links.go | 39 ++++++--------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 0835186db1..5a078a545d 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -54,40 +54,13 @@ var ( // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+$"}[1m]))`), }, - - /* - NB: requirements for the Service yaml are - - The `spec.selector` needs to select on `name` - - The `metadata.name` needs to be the same value as `spec.selector.name` - */ report.Service: { - /* - A recording rule defines `namespace_name:container_memory_usage_bytes:sum`: - - namespace_name:container_memory_usage_bytes:sum = - sum by (namespace, name) ( - sum(container_memory_usage_bytes{image!=""}) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - - Additionally, we filter by `monitor=""` for cortex-only data. - */ - docker.MemoryUsage: parsedTemplate(`namespace_name:container_memory_usage_bytes:sum{name="{{.Label}}",monitor=""}`), - - /* - A recording rule defines `namespace_name:container_cpu_usage_seconds_total:sum_rate`: - - namespace_name:container_cpu_usage_seconds_total:sum_rate = - sum by (namespace, name) ( - sum(rate(container_cpu_usage_seconds_total{image!=""}[5m])) by (pod_name, namespace) - * on (pod_name) group_left(name) - k8s_pod_labels{job="monitoring/kube-api-exporter"} - ) - - Additionally, we filter by `monitor=""` for cortex-only data. - */ - docker.CPUTotalUsage: parsedTemplate(`namespace_name:container_cpu_usage_seconds_total:sum_rate{name="{{.Label}},monitor=""}`), + // These recording rules must be defined in the prometheus config. + // NB: Pods need to be labeled and selected by their respective Service name, meaning: + // - The Service's `spec.selector` needs to select on `name` + // - The Service's `metadata.name` needs to be the same value as `spec.selector.name` + docker.MemoryUsage: parsedTemplate(`namespace_label_name:container_memory_usage_bytes:sum{label_name="{{.Label}}"}`), + docker.CPUTotalUsage: parsedTemplate(`namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{.Label}}}`), }, } ) From 5334099f92ecf3748ce99dabe235a79247314783 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Wed, 19 Jul 2017 11:01:47 +0200 Subject: [PATCH 14/31] Append links directly to metrics The initial idea was to keep it separate since the unattached links were also to be displayed distinctively from the metrics. With the new design, unattached links are rendered in the same list as metrics with attached links. Therefore, we treat unattached metric links as an empty metric. --- client/app/scripts/charts/nodes-grid.js | 1 + .../app/scripts/components/cloud-feature.js | 5 +- client/app/scripts/components/cloud-link.js | 72 ++++++++ client/app/scripts/components/node-details.js | 29 +-- .../node-details/node-details-health-item.js | 11 +- .../node-details-health-link-item.js | 59 ++---- .../node-details-health-overflow-item.js | 2 +- .../node-details/node-details-health.js | 18 +- .../node-details-table-node-metric-link.js | 46 +++++ .../node-details-table-node-metric.js | 13 -- .../node-details/node-details-table-row.js | 10 +- .../node-details/node-details-table.js | 2 +- client/app/scripts/components/sparkline.js | 84 ++++++--- client/app/scripts/selectors/node-metric.js | 1 + client/app/scripts/utils/color-utils.js | 10 + client/app/scripts/utils/metric-utils.js | 4 +- client/app/styles/_base.scss | 22 ++- client/package.json | 1 + client/yarn.lock | 10 + render/detailed/links.go | 174 ++++++++++++------ render/detailed/links_test.go | 96 ++++++---- render/detailed/summary.go | 43 ++--- report/metric_row.go | 79 ++++---- 23 files changed, 510 insertions(+), 282 deletions(-) create mode 100644 client/app/scripts/components/cloud-link.js create mode 100644 client/app/scripts/components/node-details/node-details-table-node-metric-link.js delete mode 100644 client/app/scripts/components/node-details/node-details-table-node-metric.js diff --git a/client/app/scripts/charts/nodes-grid.js b/client/app/scripts/charts/nodes-grid.js index 7826c54ad2..9c6793951c 100644 --- a/client/app/scripts/charts/nodes-grid.js +++ b/client/app/scripts/charts/nodes-grid.js @@ -23,6 +23,7 @@ function getColumns(nodes) { .toList() .flatMap((n) => { const metrics = (n.get('metrics') || makeList()) + .filter(m => !m.get('valueEmpty')) .map(m => makeMap({ id: m.get('id'), label: m.get('label'), dataType: 'number' })); return metrics; }) diff --git a/client/app/scripts/components/cloud-feature.js b/client/app/scripts/components/cloud-feature.js index c34277b8ff..61f4ada49a 100644 --- a/client/app/scripts/components/cloud-feature.js +++ b/client/app/scripts/components/cloud-feature.js @@ -13,14 +13,13 @@ class CloudFeature extends React.Component { if (process.env.WEAVE_CLOUD) { return React.cloneElement(React.Children.only(this.props.children), { params: this.context.router.params, - router: this.context.router, - isCloud: true + router: this.context.router }); } // also show if not in weave cloud? if (this.props.alwaysShow) { - return React.cloneElement(React.Children.only(this.props.children), {isCloud: false}); + return React.cloneElement(React.Children.only(this.props.children)); } return null; diff --git a/client/app/scripts/components/cloud-link.js b/client/app/scripts/components/cloud-link.js new file mode 100644 index 0000000000..315919e7b2 --- /dev/null +++ b/client/app/scripts/components/cloud-link.js @@ -0,0 +1,72 @@ +import React from 'react'; +import { connect } from 'react-redux'; +import filterInvalidDOMProps from 'filter-invalid-dom-props'; + +import CloudFeature from './cloud-feature'; + +/** + * CloudLink provides an anchor that allows to set a target + * that is comprised of Weave Cloud related pieces. + * + * We support here relative links with a leading `/` that rewrite + * the browser url as well as cloud-related placeholders (:orgId). + * + * If no `url` is given, only the children is rendered (no anchor). + * + * If you want to render the content even if not on the cloud, set + * the `alwaysShow` property. A location redirect will be made for + * clicks instead. + */ +const CloudLink = ({ alwaysShow, ...props }) => ( + + + + ); + +class LinkWrapper extends React.Component { + constructor(props, context) { + super(props, context); + + this.handleClick = this.handleClick.bind(this); + this.buildHref = this.buildHref.bind(this); + } + + handleClick(ev, href) { + ev.preventDefault(); + if (!href) return; + + const { router, onClick } = this.props; + + if (onClick) { + onClick(); + } + + if (router && href[0] === '/') { + router.push(href); + } else { + location.href = href; + } + } + + buildHref(url) { + const { params } = this.props; + if (!url || !params || !params.orgId) return url; + return url.replace(/:orgid/gi, encodeURIComponent(this.props.params.orgId)); + } + + render() { + const { url, children, ...props } = this.props; + if (!url) { + return children; + } + + const href = this.buildHref(url); + return ( + this.handleClick(e, href)}> + {children} + + ); + } +} + +export default connect()(CloudLink); diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 04f06ec6e5..9d8702cb57 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -56,28 +56,6 @@ class NodeDetails extends React.Component { resetDocumentTitle(); } - static collectMetrics(details) { - const metrics = details.metrics || []; - - // collect by metric id (id => link) - const metricLinks = (details.metric_links || []) - .reduce((agg, link) => Object.assign(agg, {[link.id]: link}), {}); - - const availableMetrics = metrics.reduce( - (agg, m) => Object.assign(agg, {[m.id]: true}), - {} - ); - - // append links with no metrics as fake metrics - (details.metric_links || []).forEach((link) => { - if (availableMetrics[link.id] === undefined) { - metrics.push({id: link.id, label: link.label}); - } - }); - - return { metrics, metricLinks }; - } - renderTools() { const showSwitchTopology = this.props.nodeId !== this.props.selectedNodeId; const topologyTitle = `View ${this.props.label} in ${this.props.topologyId}`; @@ -194,8 +172,6 @@ class NodeDetails extends React.Component { } }; - const { metrics, metricLinks } = NodeDetails.collectMetrics(details); - return (
{tools} @@ -221,11 +197,10 @@ class NodeDetails extends React.Component {
}
- {metrics.length > 0 &&
+ {details.metrics &&
Status
diff --git a/client/app/scripts/components/node-details/node-details-health-item.js b/client/app/scripts/components/node-details/node-details-health-item.js index 89cb1131a3..2004016ec8 100644 --- a/client/app/scripts/components/node-details/node-details-health-item.js +++ b/client/app/scripts/components/node-details/node-details-health-item.js @@ -6,13 +6,14 @@ import { formatMetric } from '../../utils/string-utils'; function NodeDetailsHealthItem(props) { return (
- {props.value !== undefined &&
{formatMetric(props.value, props)}
} - {props.samples &&
+ {!props.valueEmpty &&
{formatMetric(props.value, props)}
} +
-
} + first={props.first} last={props.last} hoverColor={props.metricColor} + hovered={props.samples && props.hovered} + /> +
{props.label}
diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index 690ba5854f..b2417249cb 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -1,21 +1,21 @@ import React from 'react'; -import { trackMixpanelEvent } from '../../utils/tracking-utils'; -import { getMetricColor } from '../../utils/metric-utils'; import NodeDetailsHealthItem from './node-details-health-item'; +import CloudLink from '../cloud-link'; +import { getMetricColor } from '../../utils/metric-utils'; +import { trackMixpanelEvent } from '../../utils/tracking-utils'; export default class NodeDetailsHealthLinkItem extends React.Component { - constructor(props, context) { - super(props, context); + constructor(props) { + super(props); this.state = { hovered: false }; - this.handleClick = this.handleClick.bind(this); - this.buildHref = this.buildHref.bind(this); this.onMouseOver = this.onMouseOver.bind(this); this.onMouseOut = this.onMouseOut.bind(this); + this.onClick = this.onClick.bind(this); } onMouseOver() { @@ -26,50 +26,31 @@ export default class NodeDetailsHealthLinkItem extends React.Component { this.setState({hovered: false}); } - handleClick(ev, href) { - ev.preventDefault(); - if (!href) return; - - const { router, topologyId } = this.props; - trackMixpanelEvent('scope.node.health.click', { topologyId }); - - if (router && href[0] === '/') { - router.push(href); - } else { - location.href = href; - } - } - - buildHref(url) { - if (!url || !this.props.isCloud) return url; - return url.replace(/:orgid/gi, encodeURIComponent(this.props.params.orgId)); + onClick() { + trackMixpanelEvent('scope.node.metric.click', { topologyId: this.props.topologyId }); } render() { - const { links, id, nodeColor, ...props } = this.props; - const href = this.buildHref(links[id] && links[id].url); - if (!href) return ; + const { id, nodeColor, url, ...props } = this.props; - const hasData = (props.samples && props.samples.length > 0) || props.value !== undefined; - const labelColor = this.state.hovered && !hasData ? nodeColor : undefined; - const sparkline = {}; - if (this.state.hovered) { - sparkline.strokeColor = getMetricColor(id); - sparkline.strokeWidth = '2px'; - } + const labelColor = this.state.hovered && !props.samples ? nodeColor : undefined; + const metricColor = getMetricColor(id); return ( - this.handleClick(e, href)} onMouseOver={this.onMouseOver} - onMouseOut={this.onMouseOut}> + onMouseOut={this.onMouseOut} + onClick={this.onClick} + url={url} + > - + ); } } diff --git a/client/app/scripts/components/node-details/node-details-health-overflow-item.js b/client/app/scripts/components/node-details/node-details-health-overflow-item.js index 36c6ff6de3..4372d87dec 100644 --- a/client/app/scripts/components/node-details/node-details-health-overflow-item.js +++ b/client/app/scripts/components/node-details/node-details-health-overflow-item.js @@ -6,7 +6,7 @@ function NodeDetailsHealthOverflowItem(props) { return (
- {props.value !== undefined && formatMetric(props.value, props)} + {!props.valueEmpty && formatMetric(props.value, props)}
{props.label}
diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index e323d75bbb..5ceddca6b5 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -1,10 +1,9 @@ import React from 'react'; -import { Map as makeMap, List as makeList } from 'immutable'; +import { List as makeList } from 'immutable'; import ShowMore from '../show-more'; import NodeDetailsHealthOverflow from './node-details-health-overflow'; import NodeDetailsHealthLinkItem from './node-details-health-link-item'; -import CloudFeature from '../cloud-feature'; export default class NodeDetailsHealth extends React.Component { @@ -24,7 +23,6 @@ export default class NodeDetailsHealth extends React.Component { render() { const { metrics = makeList(), - metricLinks = makeMap(), topologyId, nodeColor, } = this.props; @@ -40,14 +38,12 @@ export default class NodeDetailsHealth extends React.Component { return (
- {primeMetrics.map(item => - - )} + {primeMetrics.map(item => )} {showOverflow && + + {!valueEmpty && formatMetric(value, this.props)} + + + ); + } +} + +export default NodeDetailsTableNodeMetricLink; diff --git a/client/app/scripts/components/node-details/node-details-table-node-metric.js b/client/app/scripts/components/node-details/node-details-table-node-metric.js deleted file mode 100644 index b0dc2b79d2..0000000000 --- a/client/app/scripts/components/node-details/node-details-table-node-metric.js +++ /dev/null @@ -1,13 +0,0 @@ -import React from 'react'; - -import { formatMetric } from '../../utils/string-utils'; - -function NodeDetailsTableNodeMetric(props) { - return ( - - {formatMetric(props.value, props)} - - ); -} - -export default NodeDetailsTableNodeMetric; diff --git a/client/app/scripts/components/node-details/node-details-table-row.js b/client/app/scripts/components/node-details/node-details-table-row.js index 2f17cd33c2..4b910d943a 100644 --- a/client/app/scripts/components/node-details/node-details-table-row.js +++ b/client/app/scripts/components/node-details/node-details-table-row.js @@ -5,7 +5,7 @@ import { intersperse } from '../../utils/array-utils'; import NodeDetailsTableNodeLink from './node-details-table-node-link'; -import NodeDetailsTableNodeMetric from './node-details-table-node-metric'; +import NodeDetailsTableNodeMetricLink from './node-details-table-node-metric-link'; import { formatDataType } from '../../utils/string-utils'; function getValuesForNode(node) { @@ -40,7 +40,7 @@ function getValuesForNode(node) { } -function renderValues(node, columns = [], columnStyles = [], timestamp = null) { +function renderValues(node, columns = [], columnStyles = [], timestamp = null, topologyId = null) { const fields = getValuesForNode(node); return columns.map(({ id }, i) => { const field = fields[id]; @@ -76,8 +76,10 @@ function renderValues(node, columns = [], columnStyles = [], timestamp = null) { ); } + // valueType === 'metrics' return ( - + ); } // empty cell to complete the row for proper hover @@ -142,7 +144,7 @@ export default class NodeDetailsTableRow extends React.Component { render() { const { node, nodeIdKey, topologyId, columns, onClick, colStyles, timestamp } = this.props; const [firstColumnStyle, ...columnStyles] = colStyles; - const values = renderValues(node, columns, columnStyles, timestamp); + const values = renderValues(node, columns, columnStyles, timestamp, topologyId); const nodeId = node[nodeIdKey]; const className = classNames('node-details-table-node', { diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index 6fff5f0495..d5f41f2703 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -109,7 +109,7 @@ function getSortedNodes(nodes, sortedByHeader, sortedDesc) { const getValue = getValueForSortedBy(sortedByHeader); const withAndWithoutValues = groupBy(nodes, (n) => { const v = getValue(n); - return v !== null && v !== undefined ? 'withValues' : 'withoutValues'; + return !n.valueEmpty && v !== null && v !== undefined ? 'withValues' : 'withoutValues'; }); const withValues = sortNodes(withAndWithoutValues.withValues, getValue, sortedDesc); const withoutValues = sortNodes(withAndWithoutValues.withoutValues, getValue, sortedDesc); diff --git a/client/app/scripts/components/sparkline.js b/client/app/scripts/components/sparkline.js index f802d329c7..b11e122624 100644 --- a/client/app/scripts/components/sparkline.js +++ b/client/app/scripts/components/sparkline.js @@ -7,8 +7,12 @@ import { line, curveLinear } from 'd3-shape'; import { scaleLinear } from 'd3-scale'; import { formatMetricSvg } from '../utils/string-utils'; +import { brightenColor, darkenColor } from '../utils/color-utils'; +const HOVER_RADIUS_MULTIPLY = 1.5; +const HOVER_STROKE_MULTIPLY = 5; + export default class Sparkline extends React.Component { constructor(props, context) { super(props, context); @@ -20,19 +24,19 @@ export default class Sparkline extends React.Component { .y(d => this.y(d.value)); } + initRanges() { + // adjust scales and leave some room for the circle on the right, upper, and lower edge + const padding = 2 + Math.ceil(this.props.circleRadius * HOVER_RADIUS_MULTIPLY); + this.x.range([2, this.props.width - padding]); + this.y.range([this.props.height - padding, padding]); + this.line.curve(this.props.curve); + } + getGraphData() { // data is of shape [{date, value}, ...] and is sorted by date (ASC) let data = this.props.data; - // Do nothing if no data or data w/o date are passed in. - if (data === undefined || data.length === 0 || data[0].date === undefined) { - return
; - } - - // adjust scales - this.x.range([2, this.props.width - 2]); - this.y.range([this.props.height - 2, 2]); - this.line.curve(this.props.curve); + this.initRanges(); // Convert dates into D3 dates data = data.map(d => ({ @@ -70,30 +74,66 @@ export default class Sparkline extends React.Component { return {title, lastX, lastY, data}; } + getEmptyGraphData() { + this.initRanges(); + const first = new Date(0); + const last = new Date(15); + this.x.domain([first, last]); + this.y.domain([0, 1]); + + return { + title: '', + lastX: this.x(last), + lastY: this.y(0), + data: [ + {date: first, value: 0}, + {date: last, value: 0}, + ], + }; + } + render() { - // Do nothing if no data or data w/o date are passed in. + let strokeColor = this.props.strokeColor; + let strokeWidth = this.props.strokeWidth; + let radius = this.props.circleRadius; + let fillOpacity = 0.6; + let circleColor; + let graph = {}; + if (!this.props.data || this.props.data.length === 0 || this.props.data[0].date === undefined) { - return
; + // no data means just a dead line w/o circle + graph = this.getEmptyGraphData(); + strokeColor = brightenColor(strokeColor); + radius = 0; + } else { + graph = this.getGraphData(); + + if (this.props.hovered) { + strokeColor = this.props.hoverColor; + circleColor = strokeColor; + strokeWidth *= HOVER_STROKE_MULTIPLY; + radius *= HOVER_RADIUS_MULTIPLY; + fillOpacity = 1; + } else { + circleColor = darkenColor(strokeColor); + } } - const {lastX, lastY, title, data} = this.getGraphData(); - return ( -
+
); } - } Sparkline.propTypes = { @@ -104,8 +144,10 @@ Sparkline.defaultProps = { width: 80, height: 24, strokeColor: '#7d7da8', - strokeWidth: '0.5px', + strokeWidth: 0.5, + hoverColor: '#7d7da8', curve: curveLinear, - circleDiameter: 1.75, + circleRadius: 1.75, + hovered: false, data: [], }; diff --git a/client/app/scripts/selectors/node-metric.js b/client/app/scripts/selectors/node-metric.js index 6ac3f5d448..4058534fd1 100644 --- a/client/app/scripts/selectors/node-metric.js +++ b/client/app/scripts/selectors/node-metric.js @@ -23,6 +23,7 @@ export const availableMetricsSelector = createSelector( return nodes .valueSeq() .flatMap(n => n.get('metrics', makeList())) + .filter(m => !m.get('valueEmpty')) .map(m => makeMap({ id: m.get('id'), label: m.get('label') })) .toSet() .toList() diff --git a/client/app/scripts/utils/color-utils.js b/client/app/scripts/utils/color-utils.js index 676d7af04d..2f9b082745 100644 --- a/client/app/scripts/utils/color-utils.js +++ b/client/app/scripts/utils/color-utils.js @@ -82,3 +82,13 @@ export function brightenColor(c) { } return color.toString(); } + +export function darkenColor(c) { + let color = hsl(c); + if (hsl.l < 0.5) { + color = color.darker(0.5); + } else { + color = color.darker(0.8); + } + return color.toString(); +} diff --git a/client/app/scripts/utils/metric-utils.js b/client/app/scripts/utils/metric-utils.js index 4350bddb6e..e14547d628 100644 --- a/client/app/scripts/utils/metric-utils.js +++ b/client/app/scripts/utils/metric-utils.js @@ -58,12 +58,12 @@ export function getMetricColor(metric) { if (/mem/.test(metricId)) { return 'steelBlue'; } else if (/cpu/.test(metricId)) { - return colors('cpu'); + return colors('cpu').toString(); } else if (/files/.test(metricId)) { // purple return '#9467bd'; } else if (/load/.test(metricId)) { - return colors('load'); + return colors('load').toString(); } return 'steelBlue'; } diff --git a/client/app/styles/_base.scss b/client/app/styles/_base.scss index 7f8d52f934..0f5f28d04b 100644 --- a/client/app/styles/_base.scss +++ b/client/app/styles/_base.scss @@ -12,6 +12,10 @@ // TODO: Remove this line once Service UI CONFIGURE button stops being added to Scope. .scope-wrapper .setup-nav-button { display: none; } +a { + text-decoration: none; +} + .browsehappy { margin: 0.2em 0; background: #ccc; @@ -932,18 +936,22 @@ width: 33%; display: flex; flex-direction: column; + flex-grow: 1; &-label { color: $text-secondary-color; text-transform: uppercase; font-size: 80%; - margin-top: auto; .fa { margin-left: 0.5em; } } + &-sparkline { + margin-top: auto; + } + &-placeholder { font-size: 200%; opacity: 0.2; @@ -952,10 +960,12 @@ } &-link-item { - @extend .btn-opacity; + @extend .palable; cursor: pointer; opacity: $link-opacity-default; width: 33%; + display: flex; + color: inherit; .label { text-transform: uppercase; @@ -1109,6 +1119,14 @@ text-align: right; } + &-metric-link { + @extend .btn-opacity; + text-decoration: underline; + cursor: pointer; + opacity: $link-opacity-default; + color: $text-color; + } + &-value-scalar { // width: 2em; text-align: right; diff --git a/client/package.json b/client/package.json index 5d971a1c91..3245191ef9 100644 --- a/client/package.json +++ b/client/package.json @@ -23,6 +23,7 @@ "dagre": "0.7.4", "debug": "2.6.6", "filesize": "3.5.9", + "filter-invalid-dom-props": "^2.0.0", "font-awesome": "4.7.0", "immutable": "3.8.1", "lcp": "1.1.0", diff --git a/client/yarn.lock b/client/yarn.lock index 8518dc6ffd..a4cc86908f 100644 --- a/client/yarn.lock +++ b/client/yarn.lock @@ -2499,6 +2499,12 @@ fill-range@^2.1.0: repeat-element "^1.1.2" repeat-string "^1.5.2" +filter-invalid-dom-props@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/filter-invalid-dom-props/-/filter-invalid-dom-props-2.0.0.tgz#527f1494cb3c4f282a73c43804153eb80c42dc2c" + dependencies: + html-attributes "1.1.0" + finalhandler@~1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.0.1.tgz#bcd15d1689c0e5ed729b6f7f541a6df984117db8" @@ -2889,6 +2895,10 @@ hosted-git-info@^2.1.4: version "2.3.1" resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.3.1.tgz#ac439421605f0beb0ea1349de7d8bb28e50be1dd" +html-attributes@1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/html-attributes/-/html-attributes-1.1.0.tgz#82027a4fac7a6070ea6c18cc3886aea18d6dea09" + html-comment-regex@^1.1.0: version "1.1.1" resolved "https://registry.yarnpkg.com/html-comment-regex/-/html-comment-regex-1.1.1.tgz#668b93776eaae55ebde8f3ad464b307a4963625e" diff --git a/render/detailed/links.go b/render/detailed/links.go index 5a078a545d..acb2210f9e 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -12,46 +12,70 @@ import ( "github.com/ugorji/go/codec" ) -// MetricLink describes a URL referencing a metric. -type MetricLink struct { - // References the metric id - ID string `json:"id"` - Label string `json:"label"` - URL string `json:"url"` - Priority int `json:"priority"` -} - -// Variable name for the query within the metrics graph url +// Replacement variable name for the query in the metrics graph url const urlQueryVarName = ":query" var ( - // Available metric links - linkTemplates = []MetricLink{ - {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1}, - {ID: docker.MemoryUsage, Label: "Memory", Priority: 2}, + // Metadata for shown queries + shownQueries = []struct { + ID string + Label string + }{ + { + ID: docker.CPUTotalUsage, + Label: "CPU", + }, + { + ID: docker.MemoryUsage, + Label: "Memory", + }, + { + ID: "receive_bytes", + Label: "Rx/s", + }, + { + ID: "transmit_bytes", + Label: "Tx/s", + }, } // Prometheus queries for topologies + // + // Metrics + // - `container_cpu_usage_seconds_total` --> cAdvisor in Kubelets. + // - `container_memory_usage_bytes` --> cAdvisor in Kubelets. topologyQueries = map[string]map[string]*template.Template{ + // Containers + + report.Container: { + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{container_name="{{.Label}}"})`), + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{container_name="{{.Label}}"}[1m]))`), + }, + report.ContainerImage: { + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{image="{{.Label}}"})`), + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{image="{{.Label}}"}[1m]))`), + }, + "group:container:docker_container_hostname": { + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), + docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), + }, + + // Kubernetes topologies + report.Pod: { - // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), - // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), + "receive_bytes": parsedTemplate(`sum(rate(container_network_receive_bytes_total{pod_name="{{.Label}}"}[5m]))`), + "transmit_bytes": parsedTemplate(`sum(rate(container_network_transmit_bytes_total{pod_name="{{.Label}}"}[5m]))`), }, - report.Deployment: { - // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - // Pod names are automatically generated by k8s using the deployment name: - // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"})`), - // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + // Pod naming: + // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label + "__k8s_controllers": { + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"})`), docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"}[1m]))`), }, report.DaemonSet: { - // `container_memory_usage_bytes` is provided by cAdvisor in Kubelets. - // Pod names are automatically generated by k8s using the DaemonSet name. - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+$"})`), - // `container_cpu_usage_seconds_total` is provided by cAdvisor in Kubelets. + docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+$"})`), docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+$"}[1m]))`), }, report.Service: { @@ -59,45 +83,61 @@ var ( // NB: Pods need to be labeled and selected by their respective Service name, meaning: // - The Service's `spec.selector` needs to select on `name` // - The Service's `metadata.name` needs to be the same value as `spec.selector.name` - docker.MemoryUsage: parsedTemplate(`namespace_label_name:container_memory_usage_bytes:sum{label_name="{{.Label}}"}`), + docker.MemoryUsage: parsedTemplate(`namespace_label_name:container_memory_usage_bytes:sum{label_name="{{.Label}}"}`), docker.CPUTotalUsage: parsedTemplate(`namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{.Label}}}`), }, } + k8sControllers = map[string]struct{}{ + report.Deployment: {}, + "stateful_set": {}, + "cron_job": {}, + } ) -// NodeMetricLinks returns the links of a node. The links are collected -// by a predefined set but filtered depending on whether a query -// is configured or not for the particular topology. -func NodeMetricLinks(_ report.Report, n report.Node) []MetricLink { - queries := topologyQueries[n.Topology] +// RenderMetricURLs sets respective URLs for metrics in a node summary. Missing metrics +// where we have a query for will be appended as an empty metric (no values or samples). +func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { + if metricsGraphURL == "" { + return summary + } + + queries := getTopologyQueries(n.Topology) if len(queries) == 0 { - return nil + return summary } - links := []MetricLink{} - for _, link := range linkTemplates { - if _, ok := queries[link.ID]; ok { - links = append(links, link) + var maxprio float64 + var bs bytes.Buffer + var ms []report.MetricRow + found := make(map[string]struct{}) + + // Set URL on existing metrics + for _, metric := range summary.Metrics { + if metric.Priority > maxprio { + maxprio = metric.Priority + } + tpl := queries[metric.ID] + if tpl == nil { + continue } - } - return links -} + bs.Reset() + if err := tpl.Execute(&bs, summary); err != nil { + continue + } -// RenderMetricLinks executes the templated links by supplying the node summary as data. -// `metricsGraphURL` supports placeholders such as `:orgID` and `:query`. If the `:query` -// part is missing, a JSON version will be appended, see `queryParamsAsJSON()` for more info. -// It returns the modified summary. -func RenderMetricLinks(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { - queries := topologyQueries[n.Topology] - if len(queries) == 0 || len(summary.MetricLinks) == 0 { - return summary + ms = append(ms, metric) + ms[len(ms)-1].URL = buildURL(bs.String(), metricsGraphURL) + found[metric.ID] = struct{}{} } - links := []MetricLink{} - var bs bytes.Buffer - for _, link := range summary.MetricLinks { - tpl := queries[link.ID] + // Append empty metrics for unattached queries + for _, metadata := range shownQueries { + if _, ok := found[metadata.ID]; ok { + continue + } + + tpl := queries[metadata.ID] if tpl == nil { continue } @@ -107,19 +147,26 @@ func RenderMetricLinks(summary NodeSummary, n report.Node, metricsGraphURL strin continue } - link.URL = buildURL(bs.String(), metricsGraphURL) - links = append(links, link) + maxprio++ + ms = append(ms, report.MetricRow{ + ID: metadata.ID, + Label: metadata.Label, + URL: buildURL(bs.String(), metricsGraphURL), + Metric: &report.Metric{}, + Priority: maxprio, + ValueEmpty: true, + }) } - summary.MetricLinks = links + + summary.Metrics = ms return summary } -// buildURL puts together the URL by looking at the configured -// `metricsGraphURL`. +// buildURL puts together the URL by looking at the configured `metricsGraphURL`. func buildURL(query, metricsGraphURL string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { - return strings.Replace(metricsGraphURL, urlQueryVarName, url.PathEscape(query), -1) + return strings.Replace(metricsGraphURL, urlQueryVarName, url.QueryEscape(query), -1) } params, err := queryParamsAsJSON(query) @@ -131,7 +178,7 @@ func buildURL(query, metricsGraphURL string) string { metricsGraphURL += "/" } - return metricsGraphURL + url.PathEscape(params) + return metricsGraphURL + url.QueryEscape(params) } // queryParamsAsJSON packs the query into a JSON of the @@ -163,3 +210,10 @@ func parsedTemplate(query string) *template.Template { return tpl } + +func getTopologyQueries(t string) map[string]*template.Template { + if _, ok := k8sControllers[t]; ok { + t = "__k8s_controllers" + } + return topologyQueries[t] +} diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index 29328d7d26..811f51fc57 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -10,54 +10,82 @@ import ( "github.com/stretchr/testify/assert" ) +const ( + sampleMetricsGraphURL = "/prom/:orgID/notebook/new" +) + var ( - sampleReport = report.Report{} - samplePodNode = report.MakeNode("noo").WithTopology(report.Pod) sampleUnknownNode = report.MakeNode("???").WithTopology("foo") + samplePodNode = report.MakeNode("noo").WithTopology(report.Pod) + sampleMetrics = []report.MetricRow{ + {ID: docker.MemoryUsage}, + {ID: docker.CPUTotalUsage}, + } ) -func TestNodeMetricLinks_UnknownTopology(t *testing.T) { - links := detailed.NodeMetricLinks(sampleReport, sampleUnknownNode) - assert.Nil(t, links) +func TestRenderMetricURLs_Disabled(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, samplePodNode, "") + + assert.Empty(t, result.Metrics[0].URL) + assert.Empty(t, result.Metrics[1].URL) } -func TestNodeMetricLinks(t *testing.T) { - expected := []detailed.MetricLink{ - {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1, URL: ""}, - {ID: docker.MemoryUsage, Label: "Memory", Priority: 2, URL: ""}, - } +func TestRenderMetricURLs_UnknownTopology(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, sampleUnknownNode, sampleMetricsGraphURL) - links := detailed.NodeMetricLinks(sampleReport, samplePodNode) - assert.Equal(t, expected, links) + assert.Empty(t, result.Metrics[0].URL) + assert.Empty(t, result.Metrics[1].URL) } -func TestRenderMetricLinks_UnknownTopology(t *testing.T) { - summary := detailed.NodeSummary{} +func TestRenderMetricURLs(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) - result := detailed.RenderMetricLinks(summary, sampleUnknownNode, "") - assert.Equal(t, summary, result) + u := "/prom/:orgID/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22sum%28container_memory_usage_bytes%7Bpod_name%3D%5C%22foo%5C%22%7D%29%22%5D%7D%5D%7D" + assert.Equal(t, u, result.Metrics[0].URL) + u = "/prom/:orgID/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name%3D%5C%22foo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D" + assert.Equal(t, u, result.Metrics[1].URL) } -func TestRenderMetricLinks_Pod(t *testing.T) { - summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} +func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { + result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode, sampleMetricsGraphURL) + + m := result.Metrics[0] + assert.Equal(t, docker.CPUTotalUsage, m.ID) + assert.Equal(t, "CPU", m.Label) + assert.NotEmpty(t, m.URL) + assert.True(t, m.ValueEmpty) + assert.Equal(t, float64(1), m.Priority) + + m = result.Metrics[1] + assert.NotEmpty(t, m.URL) + assert.True(t, m.ValueEmpty) + assert.Equal(t, float64(2), m.Priority) +} + +func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) { + s := detailed.NodeSummary{ + Label: "foo", + Metrics: []report.MetricRow{{ID: docker.MemoryUsage, Priority: 1}}, + } + result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) + + assert.NotEmpty(t, result.Metrics[0].URL) + assert.False(t, result.Metrics[0].ValueEmpty) - result := detailed.RenderMetricLinks(summary, samplePodNode, "/prom/:orgID/notebook/new") - assert.Equal(t, - "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%5C%22woo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D", - result.MetricLinks[0].URL) - assert.Equal(t, - "/prom/:orgID/notebook/new/%7B%22cells%22:%5B%7B%22queries%22:%5B%22sum%28container_memory_usage_bytes%7Bpod_name=%5C%22woo%5C%22%7D%29%22%5D%7D%5D%7D", - result.MetricLinks[1].URL) + assert.NotEmpty(t, result.Metrics[1].URL) + assert.True(t, result.Metrics[1].ValueEmpty) + assert.Equal(t, float64(2), result.Metrics[1].Priority) // first empty metric starts at non-empty prio + 1 } -func TestRenderMetricLinks_QueryReplacement(t *testing.T) { - summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} +func TestRenderMetricURLs_QueryReplacement(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, samplePodNode, "http://example.test/?q=:query") - result := detailed.RenderMetricLinks(summary, samplePodNode, "/foo/:orgID/bar?q=:query") - assert.Equal(t, - "/foo/:orgID/bar?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name=%22boo%22%7D%5B1m%5D%29%29", - result.MetricLinks[0].URL) - assert.Equal(t, - "/foo/:orgID/bar?q=sum%28container_memory_usage_bytes%7Bpod_name=%22boo%22%7D%29", - result.MetricLinks[1].URL) + u := "http://example.test/?q=sum%28container_memory_usage_bytes%7Bpod_name%3D%22foo%22%7D%29" + assert.Equal(t, u, result.Metrics[0].URL) + u = "http://example.test/?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name%3D%22foo%22%7D%5B1m%5D%29%29" + assert.Equal(t, u, result.Metrics[1].URL) } diff --git a/render/detailed/summary.go b/render/detailed/summary.go index 697e7ecdd6..d302f6b38b 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -44,20 +44,19 @@ type Column struct { // NodeSummary is summary information about a child for a Node. type NodeSummary struct { - ID string `json:"id"` - Label string `json:"label"` - LabelMinor string `json:"labelMinor"` - Rank string `json:"rank"` - Shape string `json:"shape,omitempty"` - Stack bool `json:"stack,omitempty"` - Linkable bool `json:"linkable,omitempty"` // Whether this node can be linked-to - Pseudo bool `json:"pseudo,omitempty"` - Metadata []report.MetadataRow `json:"metadata,omitempty"` - Parents []Parent `json:"parents,omitempty"` - Metrics []report.MetricRow `json:"metrics,omitempty"` - Tables []report.Table `json:"tables,omitempty"` - Adjacency report.IDList `json:"adjacency,omitempty"` - MetricLinks []MetricLink `json:"metric_links,omitempty"` + ID string `json:"id"` + Label string `json:"label"` + LabelMinor string `json:"labelMinor"` + Rank string `json:"rank"` + Shape string `json:"shape,omitempty"` + Stack bool `json:"stack,omitempty"` + Linkable bool `json:"linkable,omitempty"` // Whether this node can be linked-to + Pseudo bool `json:"pseudo,omitempty"` + Metadata []report.MetadataRow `json:"metadata,omitempty"` + Parents []Parent `json:"parents,omitempty"` + Metrics []report.MetricRow `json:"metrics,omitempty"` + Tables []report.Table `json:"tables,omitempty"` + Adjacency report.IDList `json:"adjacency,omitempty"` } var renderers = map[string]func(NodeSummary, report.Node) (NodeSummary, bool){ @@ -104,20 +103,20 @@ var primaryAPITopology = map[string]string{ // MakeNodeSummary summarizes a node, if possible. func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (NodeSummary, bool) { - metricLinks := metricsGraphURL != "" if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { - summary, b := renderer(baseNodeSummary(r, n, metricLinks), n) - return RenderMetricLinks(summary, n, metricsGraphURL), b + summary, b := renderer(baseNodeSummary(r, n), n) + return RenderMetricURLs(summary, n, metricsGraphURL), b } } else if _, ok := r.Topology(n.Topology); ok { - summary := baseNodeSummary(r, n, metricLinks) + summary := baseNodeSummary(r, n) summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback return summary, true } if strings.HasPrefix(n.Topology, "group:") { - return groupNodeSummary(baseNodeSummary(r, n, metricLinks), r, n) + summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n) + return RenderMetricURLs(summary, n, metricsGraphURL), b } return NodeSummary{}, false } @@ -133,7 +132,7 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { return n } -func baseNodeSummary(r report.Report, n report.Node, metricLinks bool) NodeSummary { +func baseNodeSummary(r report.Report, n report.Node) NodeSummary { t, _ := r.Topology(n.Topology) ns := NodeSummary{ ID: n.ID, @@ -145,9 +144,7 @@ func baseNodeSummary(r report.Report, n report.Node, metricLinks bool) NodeSumma Tables: NodeTables(r, n), Adjacency: n.Adjacency, } - if metricLinks { - ns.MetricLinks = NodeMetricLinks(r, n) - } + return ns } diff --git a/report/metric_row.go b/report/metric_row.go index b33b378510..2d61e02189 100644 --- a/report/metric_row.go +++ b/report/metric_row.go @@ -16,13 +16,15 @@ const ( // MetricRow is a tuple of data used to render a metric as a sparkline and // accoutrements. type MetricRow struct { - ID string - Label string - Format string - Group string - Value float64 - Priority float64 - Metric *Metric + ID string + Label string + Format string + Group string + Value float64 + ValueEmpty bool + Priority float64 + URL string + Metric *Metric } // Summary returns a copy of the MetricRow, without the samples, just the value if there is one. @@ -49,17 +51,19 @@ func (*MetricRow) UnmarshalJSON(b []byte) error { // Needed to flatten the fields for backwards compatibility with probes // (time.Time is encoded in binary in MsgPack) type wiredMetricRow struct { - ID string `json:"id"` - Label string `json:"label"` - Format string `json:"format,omitempty"` - Group string `json:"group,omitempty"` - Value float64 `json:"value"` - Priority float64 `json:"priority,omitempty"` - Samples []Sample `json:"samples"` - Min float64 `json:"min"` - Max float64 `json:"max"` - First string `json:"first,omitempty"` - Last string `json:"last,omitempty"` + ID string `json:"id"` + Label string `json:"label"` + Format string `json:"format,omitempty"` + Group string `json:"group,omitempty"` + Value float64 `json:"value"` + ValueEmpty bool `json:"valueEmpty,omitempty"` + Priority float64 `json:"priority,omitempty"` + Samples []Sample `json:"samples"` + Min float64 `json:"min"` + Max float64 `json:"max"` + First string `json:"first,omitempty"` + Last string `json:"last,omitempty"` + URL string `json:"url"` } // CodecEncodeSelf marshals this MetricRow. It takes the basic Metric @@ -67,17 +71,19 @@ type wiredMetricRow struct { func (m *MetricRow) CodecEncodeSelf(encoder *codec.Encoder) { in := m.Metric.ToIntermediate() encoder.Encode(wiredMetricRow{ - ID: m.ID, - Label: m.Label, - Format: m.Format, - Group: m.Group, - Value: m.Value, - Priority: m.Priority, - Samples: in.Samples, - Min: in.Min, - Max: in.Max, - First: in.First, - Last: in.Last, + ID: m.ID, + Label: m.Label, + Format: m.Format, + Group: m.Group, + Value: m.Value, + ValueEmpty: m.ValueEmpty, + Priority: m.Priority, + URL: m.URL, + Samples: in.Samples, + Min: in.Min, + Max: in.Max, + First: in.First, + Last: in.Last, }) } @@ -94,13 +100,14 @@ func (m *MetricRow) CodecDecodeSelf(decoder *codec.Decoder) { } metric := w.FromIntermediate() *m = MetricRow{ - ID: in.ID, - Label: in.Label, - Format: in.Format, - Group: in.Group, - Value: in.Value, - Priority: in.Priority, - Metric: &metric, + ID: in.ID, + Label: in.Label, + Format: in.Format, + Group: in.Group, + Value: in.Value, + ValueEmpty: in.ValueEmpty, + Priority: in.Priority, + Metric: &metric, } } From 9372498003ff63ee1c4657668c705d759d02d276 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 24 Jul 2017 15:26:01 +0200 Subject: [PATCH 15/31] Pass `layout` when tracking mixpanel event --- .../node-details/node-details-health-link-item.js | 6 +++++- .../node-details/node-details-table-node-metric-link.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index b2417249cb..e146ed92b5 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -4,6 +4,7 @@ import NodeDetailsHealthItem from './node-details-health-item'; import CloudLink from '../cloud-link'; import { getMetricColor } from '../../utils/metric-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; +import { GRAPH_VIEW_MODE } from '../../constants/naming'; export default class NodeDetailsHealthLinkItem extends React.Component { @@ -27,7 +28,10 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } onClick() { - trackMixpanelEvent('scope.node.metric.click', { topologyId: this.props.topologyId }); + trackMixpanelEvent('scope.node.metric.click', { + layout: GRAPH_VIEW_MODE, + topologyId: this.props.topologyId, + }); } render() { diff --git a/client/app/scripts/components/node-details/node-details-table-node-metric-link.js b/client/app/scripts/components/node-details/node-details-table-node-metric-link.js index 5196914a46..51d9ee4559 100644 --- a/client/app/scripts/components/node-details/node-details-table-node-metric-link.js +++ b/client/app/scripts/components/node-details/node-details-table-node-metric-link.js @@ -3,6 +3,7 @@ import React from 'react'; import CloudLink from '../cloud-link'; import { formatMetric } from '../../utils/string-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; +import { TABLE_VIEW_MODE } from '../../constants/naming'; class NodeDetailsTableNodeMetricLink extends React.Component { constructor(props) { @@ -12,7 +13,10 @@ class NodeDetailsTableNodeMetricLink extends React.Component { } onClick() { - trackMixpanelEvent('scope.node.metric.click', { topologyId: this.props.topologyId }); + trackMixpanelEvent('scope.node.metric.click', { + layout: TABLE_VIEW_MODE, + topologyId: this.props.topologyId, + }); } static dismissEvent(ev) { From b34e11193346af2ab38bc5962109ae1bed464159 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 24 Jul 2017 17:54:21 +0200 Subject: [PATCH 16/31] Revert "Avoid global by passing metricsGraphURL down" This reverts commit 1a002880ed38cbb8da73e41954513d7144b362c4. --- app/api_report_test.go | 2 +- app/api_topologies.go | 12 ++++++------ app/api_topologies_test.go | 4 ++-- app/api_topology.go | 11 +++++------ app/router.go | 8 ++++---- prog/app.go | 8 +++++--- render/detailed/connections.go | 12 ++++++------ render/detailed/links.go | 19 +++++++++++++++---- render/detailed/links_test.go | 22 ++++++++++++++++------ render/detailed/node.go | 14 +++++++------- render/detailed/node_test.go | 8 ++++---- render/detailed/summary.go | 14 ++++++-------- render/detailed/summary_test.go | 6 +++--- 13 files changed, 80 insertions(+), 60 deletions(-) diff --git a/app/api_report_test.go b/app/api_report_test.go index c1da6f8002..1d7bd2b69f 100644 --- a/app/api_report_test.go +++ b/app/api_report_test.go @@ -14,7 +14,7 @@ import ( func topologyServer() *httptest.Server { router := mux.NewRouter().SkipClean(true) - app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}, "") + app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}) return httptest.NewServer(router) } diff --git a/app/api_topologies.go b/app/api_topologies.go index a56a243856..42a62f8e12 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -548,17 +548,17 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt return topology.renderer, nil, nil } -type reporterHandler func(context.Context, Reporter, string, http.ResponseWriter, *http.Request) +type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) -func captureReporter(rep Reporter, metricsGraphURL string, f reporterHandler) CtxHandlerFunc { +func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - f(ctx, rep, metricsGraphURL, w, r) + f(ctx, rep, w, r) } } -type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, string, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, http.ResponseWriter, *http.Request) -func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rendererHandler) CtxHandlerFunc { +func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( topologyID = mux.Vars(req)["topology"] @@ -579,6 +579,6 @@ func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rende respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, rpt, metricGraphsURL, w, req) + f(ctx, renderer, decorator, rpt, w, req) } } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 71c6906399..a817eb796c 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -183,14 +183,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator), ""), nil + return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator)), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) app.RegisterReportPostHandler(c, router) - app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}, "") + app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}) ts := httptest.NewServer(router) defer ts.Close() diff --git a/app/api_topology.go b/app/api_topology.go index a29f052d13..173e275b99 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,14 +29,14 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL), + Nodes: detailed.Summaries(report, renderer.Render(report, decorator)), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, 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"] @@ -49,14 +49,13 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node, metricsGraphURL)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node)}) } // Websocket for the full topology. func handleWebsocket( ctx context.Context, rep Reporter, - metricsGraphURL string, w http.ResponseWriter, r *http.Request, ) { @@ -124,7 +123,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL) + newTopo := detailed.Summaries(report, renderer.Render(report, decorator)) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/router.go b/app/router.go index 05999d293e..414f45f99f 100644 --- a/app/router.go +++ b/app/router.go @@ -91,7 +91,7 @@ func gzipHandler(h http.HandlerFunc) http.HandlerFunc { } // RegisterTopologyRoutes registers the various topology routes with a http mux. -func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool, metricsGraphURL string) { +func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool) { get := router.Methods("GET").Subrouter() get.HandleFunc("/api", gzipHandler(requestContextDecorator(apiHandler(r, capabilities)))) @@ -99,15 +99,15 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[str gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get. HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleTopology)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))). Name("api_topology_topology") get. HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(captureReporter(r, metricsGraphURL, handleWebsocket))). // NB not gzip! + requestContextDecorator(captureReporter(r, handleWebsocket))). // NB not gzip! Name("api_topology_topology_ws") get. MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleNode)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))). Name("api_topology_topology_id") get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) diff --git a/prog/app.go b/prog/app.go index f78054c9b8..2647ddb41a 100644 --- a/prog/app.go +++ b/prog/app.go @@ -28,6 +28,7 @@ import ( "github.com/weaveworks/scope/common/weave" "github.com/weaveworks/scope/common/xfer" "github.com/weaveworks/scope/probe/docker" + "github.com/weaveworks/scope/render/detailed" "github.com/weaveworks/weave/common" ) @@ -51,7 +52,7 @@ func init() { } // Router creates the mux for all the various app components. -func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool, metricsGraphURL string) http.Handler { +func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool) http.Handler { router := mux.NewRouter().SkipClean(true) // We pull in the http.DefaultServeMux to get the pprof routes @@ -61,7 +62,7 @@ func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.RegisterReportPostHandler(collector, router) app.RegisterControlRoutes(router, controlRouter) app.RegisterPipeRoutes(router, pipeRouter) - app.RegisterTopologyRoutes(router, collector, capabilities, metricsGraphURL) + app.RegisterTopologyRoutes(router, collector, capabilities) uiHandler := http.FileServer(GetFS(externalUI)) router.PathPrefix("/ui").Name("static").Handler( @@ -216,6 +217,7 @@ func appMain(flags appFlags) { setLogLevel(flags.logLevel) setLogFormatter(flags.logPrefix) runtime.SetBlockProfileRate(flags.blockProfileRate) + detailed.SetMetricsGraphURL(flags.metricsGraphURL) defer log.Info("app exiting") rand.Seed(time.Now().UnixNano()) @@ -297,7 +299,7 @@ func appMain(flags appFlags) { capabilities := map[string]bool{ xfer.HistoricReportsCapability: collector.HasHistoricReports(), } - handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities, flags.metricsGraphURL) + handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities) if flags.logHTTP { handler = middleware.Log{ LogRequestHeaders: flags.logHTTPHeaders, diff --git a/render/detailed/connections.go b/render/detailed/connections.go index 337e565acd..b89ebb723a 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -121,13 +121,13 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) { return addr, true } -func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool, metricsGraphURL string) []Connection { +func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { output := []Connection{} for row, count := range c.counts { // Use MakeNodeSummary to render the id and label of this node // TODO(paulbellamy): Would be cleaner if we hade just a // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. - summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID], metricsGraphURL) + summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID]) connection := Connection{ ID: fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port), NodeID: summary.ID, @@ -162,7 +162,7 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal return output } -func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { +func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(n) counts := newConnectionCounters() @@ -188,11 +188,11 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Inbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), + Connections: counts.rows(r, ns, isInternetNode(n)), } } -func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { +func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { localEndpoints := endpointChildrenOf(n) counts := newConnectionCounters() @@ -220,7 +220,7 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Outbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), + Connections: counts.rows(r, ns, isInternetNode(n)), } } diff --git a/render/detailed/links.go b/render/detailed/links.go index acb2210f9e..ffbf9683ed 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -16,6 +16,9 @@ import ( const urlQueryVarName = ":query" var ( + // As configured by the user + metricsGraphURL = "" + // Metadata for shown queries shownQueries = []struct { ID string @@ -94,9 +97,17 @@ var ( } ) +// SetMetricsGraphURL sets the URL we deduce our eventual metric link from. +// Supports placeholders such as `:orgID` and `:query`. An empty url disables +// this feature. If the `:query` part is missing, a JSON version will be +// appended, see `queryParamsAsJSON()` for more info. +func SetMetricsGraphURL(url string) { + metricsGraphURL = url +} + // RenderMetricURLs sets respective URLs for metrics in a node summary. Missing metrics // where we have a query for will be appended as an empty metric (no values or samples). -func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { +func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { if metricsGraphURL == "" { return summary } @@ -127,7 +138,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string } ms = append(ms, metric) - ms[len(ms)-1].URL = buildURL(bs.String(), metricsGraphURL) + ms[len(ms)-1].URL = buildURL(bs.String()) found[metric.ID] = struct{}{} } @@ -151,7 +162,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string ms = append(ms, report.MetricRow{ ID: metadata.ID, Label: metadata.Label, - URL: buildURL(bs.String(), metricsGraphURL), + URL: buildURL(bs.String()), Metric: &report.Metric{}, Priority: maxprio, ValueEmpty: true, @@ -164,7 +175,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string } // buildURL puts together the URL by looking at the configured `metricsGraphURL`. -func buildURL(query, metricsGraphURL string) string { +func buildURL(query string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { return strings.Replace(metricsGraphURL, urlQueryVarName, url.QueryEscape(query), -1) } diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index 811f51fc57..0a107d6c3a 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -25,23 +25,27 @@ var ( func TestRenderMetricURLs_Disabled(t *testing.T) { s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode, "") + result := detailed.RenderMetricURLs(s, samplePodNode) assert.Empty(t, result.Metrics[0].URL) assert.Empty(t, result.Metrics[1].URL) } func TestRenderMetricURLs_UnknownTopology(t *testing.T) { + detailed.SetMetricsGraphURL(sampleMetricsGraphURL) + defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, sampleUnknownNode, sampleMetricsGraphURL) + result := detailed.RenderMetricURLs(s, sampleUnknownNode) assert.Empty(t, result.Metrics[0].URL) assert.Empty(t, result.Metrics[1].URL) } func TestRenderMetricURLs(t *testing.T) { + detailed.SetMetricsGraphURL(sampleMetricsGraphURL) + defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) + result := detailed.RenderMetricURLs(s, samplePodNode) u := "/prom/:orgID/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22sum%28container_memory_usage_bytes%7Bpod_name%3D%5C%22foo%5C%22%7D%29%22%5D%7D%5D%7D" assert.Equal(t, u, result.Metrics[0].URL) @@ -50,7 +54,9 @@ func TestRenderMetricURLs(t *testing.T) { } func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { - result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode, sampleMetricsGraphURL) + detailed.SetMetricsGraphURL(sampleMetricsGraphURL) + defer detailed.SetMetricsGraphURL("") + result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode) m := result.Metrics[0] assert.Equal(t, docker.CPUTotalUsage, m.ID) @@ -66,11 +72,13 @@ func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { } func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) { + detailed.SetMetricsGraphURL(sampleMetricsGraphURL) + defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{ Label: "foo", Metrics: []report.MetricRow{{ID: docker.MemoryUsage, Priority: 1}}, } - result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) + result := detailed.RenderMetricURLs(s, samplePodNode) assert.NotEmpty(t, result.Metrics[0].URL) assert.False(t, result.Metrics[0].ValueEmpty) @@ -81,8 +89,10 @@ func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) { } func TestRenderMetricURLs_QueryReplacement(t *testing.T) { + detailed.SetMetricsGraphURL("http://example.test/?q=:query") + defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode, "http://example.test/?q=:query") + result := detailed.RenderMetricURLs(s, samplePodNode) u := "http://example.test/?q=sum%28container_memory_usage_bytes%7Bpod_name%3D%22foo%22%7D%29" assert.Equal(t, u, result.Metrics[0].URL) diff --git a/render/detailed/node.go b/render/detailed/node.go index 1481d99d8c..8e211b8138 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -80,15 +80,15 @@ func (c *ControlInstance) CodecDecodeSelf(decoder *codec.Decoder) { // 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, r report.Report, ns report.Nodes, n report.Node, metricsGraphURL string) Node { - summary, _ := MakeNodeSummary(r, n, metricsGraphURL) +func MakeNode(topologyID string, r report.Report, ns report.Nodes, n report.Node) Node { + summary, _ := MakeNodeSummary(r, n) return Node{ NodeSummary: summary, Controls: controls(r, n), - Children: children(r, n, metricsGraphURL), + Children: children(r, n), Connections: []ConnectionsSummary{ - incomingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), - outgoingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), + incomingConnectionsSummary(topologyID, r, n, ns), + outgoingConnectionsSummary(topologyID, r, n, ns), }, } } @@ -181,13 +181,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(r report.Report, n report.Node, metricsGraphURL string) []NodeSummaryGroup { +func children(r report.Report, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(r, child, metricsGraphURL) + summary, ok := MakeNodeSummary(r, child) if !ok { return } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index b612297599..d7c213ae45 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(fixture.Report, r.Render(fixture.Report, nil)[id], "") + s, ok := detailed.MakeNodeSummary(fixture.Report, r.Render(fixture.Report, nil)[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, nil) renderableNode := renderableNodes[fixture.ClientHostNodeID] - have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode) containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) @@ -183,7 +183,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) serverProcessNodeSummary.Linkable = true @@ -313,7 +313,7 @@ func TestMakeDetailedPodNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("pods", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("pods", 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 d302f6b38b..825819bec0 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -102,12 +102,12 @@ var primaryAPITopology = map[string]string{ } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (NodeSummary, bool) { +func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) { if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { summary, b := renderer(baseNodeSummary(r, n), n) - return RenderMetricURLs(summary, n, metricsGraphURL), b + return RenderMetricURLs(summary, n), b } } else if _, ok := r.Topology(n.Topology); ok { summary := baseNodeSummary(r, n) @@ -116,7 +116,7 @@ func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (No } if strings.HasPrefix(n.Topology, "group:") { summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n) - return RenderMetricURLs(summary, n, metricsGraphURL), b + return RenderMetricURLs(summary, n), b } return NodeSummary{}, false } @@ -134,7 +134,7 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { func baseNodeSummary(r report.Report, n report.Node) NodeSummary { t, _ := r.Topology(n.Topology) - ns := NodeSummary{ + return NodeSummary{ ID: n.ID, Shape: t.GetShape(), Linkable: true, @@ -144,8 +144,6 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary { Tables: NodeTables(r, n), Adjacency: n.Adjacency, } - - return ns } func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) { @@ -374,11 +372,11 @@ 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(r report.Report, rns report.Nodes, metricsGraphURL string) NodeSummaries { +func Summaries(r report.Report, rns report.Nodes) NodeSummaries { result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(r, node, metricsGraphURL); ok { + if summary, ok := MakeNodeSummary(r, node); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index 4bf843bc46..e4a5ad72d7 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(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil), "") + have := detailed.Summaries(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil)) // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -51,7 +51,7 @@ func TestSummaries(t *testing.T) { input := fixture.Report.Copy() input.Process.Nodes[fixture.ClientProcess1NodeID].Metrics[process.CPUUsage] = metric - have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil), "") + have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil)) node, ok := have[fixture.ClientProcess1NodeID] if !ok { @@ -184,7 +184,7 @@ func TestMakeNodeSummary(t *testing.T) { }, } for _, testcase := range testcases { - have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input, "") + have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input) if ok != testcase.ok { t.Errorf("%s: MakeNodeSummary failed: expected ok value to be: %v", testcase.name, testcase.ok) continue From 34dd1181b4babfe7c57c59ec2e3942b6f72b29c4 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 25 Jul 2017 15:09:57 +0200 Subject: [PATCH 17/31] Fix query typo --- render/detailed/links.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index ffbf9683ed..90764ada0b 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -87,7 +87,7 @@ var ( // - The Service's `spec.selector` needs to select on `name` // - The Service's `metadata.name` needs to be the same value as `spec.selector.name` docker.MemoryUsage: parsedTemplate(`namespace_label_name:container_memory_usage_bytes:sum{label_name="{{.Label}}"}`), - docker.CPUTotalUsage: parsedTemplate(`namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{.Label}}}`), + docker.CPUTotalUsage: parsedTemplate(`namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{.Label}}"}`), }, } k8sControllers = map[string]struct{}{ From 484b18eed924744b55f7baac4bba6729d3705943 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 27 Jul 2017 09:57:00 +0200 Subject: [PATCH 18/31] Color metric label on hover --- .../components/node-details/node-details-health-link-item.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index e146ed92b5..2484175646 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -37,7 +37,6 @@ export default class NodeDetailsHealthLinkItem extends React.Component { render() { const { id, nodeColor, url, ...props } = this.props; - const labelColor = this.state.hovered && !props.samples ? nodeColor : undefined; const metricColor = getMetricColor(id); return ( @@ -53,7 +52,7 @@ export default class NodeDetailsHealthLinkItem extends React.Component { {...props} hovered={this.state.hovered} metricColor={metricColor} - labelColor={labelColor} /> + labelColor={this.state.hovered && nodeColor} /> ); } From 92d434157eb527c98efe6c3984e34b3c08a51dad Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Fri, 28 Jul 2017 11:52:30 +0200 Subject: [PATCH 19/31] JS feedback from David --- client/app/scripts/charts/nodes-grid.js | 4 ---- client/app/scripts/components/node-details.js | 1 - .../node-details/node-details-health-link-item.js | 11 +++-------- .../node-details/node-details-health.js | 2 -- .../node-details/node-details-table-node-link.js | 5 ++++- .../node-details-table-node-metric-link.js | 15 +++------------ .../node-details/node-details-table-row.js | 13 +++++++++++++ .../components/node-details/node-details-table.js | 5 ++++- client/app/styles/_base.scss | 2 +- client/package.json | 2 +- client/yarn.lock | 2 +- 11 files changed, 30 insertions(+), 32 deletions(-) diff --git a/client/app/scripts/charts/nodes-grid.js b/client/app/scripts/charts/nodes-grid.js index 9c6793951c..dee2b08bd0 100644 --- a/client/app/scripts/charts/nodes-grid.js +++ b/client/app/scripts/charts/nodes-grid.js @@ -88,10 +88,6 @@ class NodesGrid extends React.Component { } onClickRow(ev, node) { - // TODO: do this better - if (ev.target.className === 'node-details-table-node-link') { - return; - } trackMixpanelEvent('scope.node.click', { layout: TABLE_VIEW_MODE, topologyId: this.props.currentTopology.get('id'), diff --git a/client/app/scripts/components/node-details.js b/client/app/scripts/components/node-details.js index 9d8702cb57..e951a7bfa2 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -202,7 +202,6 @@ class NodeDetails extends React.Component {
} {details.metadata &&
diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index 2484175646..d6a43f6620 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -4,7 +4,6 @@ import NodeDetailsHealthItem from './node-details-health-item'; import CloudLink from '../cloud-link'; import { getMetricColor } from '../../utils/metric-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; -import { GRAPH_VIEW_MODE } from '../../constants/naming'; export default class NodeDetailsHealthLinkItem extends React.Component { @@ -28,15 +27,11 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } onClick() { - trackMixpanelEvent('scope.node.metric.click', { - layout: GRAPH_VIEW_MODE, - topologyId: this.props.topologyId, - }); + trackMixpanelEvent('scope.node.metric.click', { topologyId: this.props.topologyId }); } render() { - const { id, nodeColor, url, ...props } = this.props; - + const { id, url, ...props } = this.props; const metricColor = getMetricColor(id); return ( @@ -52,7 +47,7 @@ export default class NodeDetailsHealthLinkItem extends React.Component { {...props} hovered={this.state.hovered} metricColor={metricColor} - labelColor={this.state.hovered && nodeColor} /> + /> ); } diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index 5ceddca6b5..102c92d622 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -24,7 +24,6 @@ export default class NodeDetailsHealth extends React.Component { const { metrics = makeList(), topologyId, - nodeColor, } = this.props; const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; @@ -42,7 +41,6 @@ export default class NodeDetailsHealth extends React.Component { {...item} key={item.id} topologyId={topologyId} - nodeColor={nodeColor} />)} {showOverflow && + ref={this.saveNodeRef} onClick={this.handleClick} + {...dismissRowClickProps} + > {label} ); diff --git a/client/app/scripts/components/node-details/node-details-table-node-metric-link.js b/client/app/scripts/components/node-details/node-details-table-node-metric-link.js index 51d9ee4559..db5090604e 100644 --- a/client/app/scripts/components/node-details/node-details-table-node-metric-link.js +++ b/client/app/scripts/components/node-details/node-details-table-node-metric-link.js @@ -3,7 +3,7 @@ import React from 'react'; import CloudLink from '../cloud-link'; import { formatMetric } from '../../utils/string-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; -import { TABLE_VIEW_MODE } from '../../constants/naming'; +import { dismissRowClickProps } from './node-details-table-row'; class NodeDetailsTableNodeMetricLink extends React.Component { constructor(props) { @@ -13,15 +13,7 @@ class NodeDetailsTableNodeMetricLink extends React.Component { } onClick() { - trackMixpanelEvent('scope.node.metric.click', { - layout: TABLE_VIEW_MODE, - topologyId: this.props.topologyId, - }); - } - - static dismissEvent(ev) { - ev.preventDefault(); - ev.stopPropagation(); + trackMixpanelEvent('scope.node.metric.click', { topologyId: this.props.topologyId }); } render() { @@ -31,8 +23,7 @@ class NodeDetailsTableNodeMetricLink extends React.Component { { + ev.preventDefault(); + ev.stopPropagation(); + } +}; + export default class NodeDetailsTableRow extends React.Component { constructor(props, context) { super(props, context); diff --git a/client/app/scripts/components/node-details/node-details-table.js b/client/app/scripts/components/node-details/node-details-table.js index d5f41f2703..928718385a 100644 --- a/client/app/scripts/components/node-details/node-details-table.js +++ b/client/app/scripts/components/node-details/node-details-table.js @@ -108,8 +108,11 @@ function sortNodes(nodes, getValue, sortedDesc) { function getSortedNodes(nodes, sortedByHeader, sortedDesc) { const getValue = getValueForSortedBy(sortedByHeader); const withAndWithoutValues = groupBy(nodes, (n) => { + if (!n || n.valueEmpty) { + return 'withoutValues'; + } const v = getValue(n); - return !n.valueEmpty && v !== null && v !== undefined ? 'withValues' : 'withoutValues'; + return v !== null && v !== undefined ? 'withValues' : 'withoutValues'; }); const withValues = sortNodes(withAndWithoutValues.withValues, getValue, sortedDesc); const withoutValues = sortNodes(withAndWithoutValues.withoutValues, getValue, sortedDesc); diff --git a/client/app/styles/_base.scss b/client/app/styles/_base.scss index 0f5f28d04b..a859eb6558 100644 --- a/client/app/styles/_base.scss +++ b/client/app/styles/_base.scss @@ -960,7 +960,7 @@ a { } &-link-item { - @extend .palable; + @extend .btn-opacity; cursor: pointer; opacity: $link-opacity-default; width: 33%; diff --git a/client/package.json b/client/package.json index 3245191ef9..c1f96fa8b7 100644 --- a/client/package.json +++ b/client/package.json @@ -23,7 +23,7 @@ "dagre": "0.7.4", "debug": "2.6.6", "filesize": "3.5.9", - "filter-invalid-dom-props": "^2.0.0", + "filter-invalid-dom-props": "2.0.0", "font-awesome": "4.7.0", "immutable": "3.8.1", "lcp": "1.1.0", diff --git a/client/yarn.lock b/client/yarn.lock index a4cc86908f..d57cd128a1 100644 --- a/client/yarn.lock +++ b/client/yarn.lock @@ -2499,7 +2499,7 @@ fill-range@^2.1.0: repeat-element "^1.1.2" repeat-string "^1.5.2" -filter-invalid-dom-props@^2.0.0: +filter-invalid-dom-props@2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/filter-invalid-dom-props/-/filter-invalid-dom-props-2.0.0.tgz#527f1494cb3c4f282a73c43804153eb80c42dc2c" dependencies: From 8a07205a5fcaecf3319e7b151d1417aa67d534b9 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Fri, 28 Jul 2017 16:07:42 +0200 Subject: [PATCH 20/31] Replace templates with simple string replacement --- render/detailed/links.go | 104 ++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 62 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 90764ada0b..6277aa50c0 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -4,7 +4,6 @@ import ( "bytes" "net/url" "strings" - "text/template" "github.com/weaveworks/scope/probe/docker" "github.com/weaveworks/scope/report" @@ -47,47 +46,47 @@ var ( // Metrics // - `container_cpu_usage_seconds_total` --> cAdvisor in Kubelets. // - `container_memory_usage_bytes` --> cAdvisor in Kubelets. - topologyQueries = map[string]map[string]*template.Template{ + topologyQueries = map[string]map[string]string{ // Containers report.Container: { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{container_name="{{.Label}}"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{container_name="{{.Label}}"}[1m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{container_name="{{label}}"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{container_name="{{label}}"}[1m]))`, }, report.ContainerImage: { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{image="{{.Label}}"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{image="{{.Label}}"}[1m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{image="{{label}}"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{image="{{label}}"}[1m]))`, }, "group:container:docker_container_hostname": { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name="{{label}}"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name="{{label}}"}[1m]))`, }, // Kubernetes topologies report.Pod: { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name="{{.Label}}"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name="{{.Label}}"}[1m]))`), - "receive_bytes": parsedTemplate(`sum(rate(container_network_receive_bytes_total{pod_name="{{.Label}}"}[5m]))`), - "transmit_bytes": parsedTemplate(`sum(rate(container_network_transmit_bytes_total{pod_name="{{.Label}}"}[5m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name="{{label}}"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name="{{label}}"}[1m]))`, + "receive_bytes": `sum(rate(container_network_receive_bytes_total{pod_name="{{label}}"}[5m]))`, + "transmit_bytes": `sum(rate(container_network_transmit_bytes_total{pod_name="{{label}}"}[5m]))`, }, // Pod naming: // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label "__k8s_controllers": { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+-[^-]+$"}[1m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name=~"^{{label}}-[^-]+-[^-]+$"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{label}}-[^-]+-[^-]+$"}[1m]))`, }, report.DaemonSet: { - docker.MemoryUsage: parsedTemplate(`sum(container_memory_usage_bytes{pod_name=~"^{{.Label}}-[^-]+$"})`), - docker.CPUTotalUsage: parsedTemplate(`sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{.Label}}-[^-]+$"}[1m]))`), + docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name=~"^{{label}}-[^-]+$"})`, + docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{label}}-[^-]+$"}[1m]))`, }, report.Service: { // These recording rules must be defined in the prometheus config. // NB: Pods need to be labeled and selected by their respective Service name, meaning: // - The Service's `spec.selector` needs to select on `name` // - The Service's `metadata.name` needs to be the same value as `spec.selector.name` - docker.MemoryUsage: parsedTemplate(`namespace_label_name:container_memory_usage_bytes:sum{label_name="{{.Label}}"}`), - docker.CPUTotalUsage: parsedTemplate(`namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{.Label}}"}`), + docker.MemoryUsage: `namespace_label_name:container_memory_usage_bytes:sum{label_name="{{label}}"}`, + docker.CPUTotalUsage: `namespace_label_name:container_cpu_usage_seconds_total:sum_rate{label_name="{{label}}"}`, }, } k8sControllers = map[string]struct{}{ @@ -112,13 +111,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { return summary } - queries := getTopologyQueries(n.Topology) - if len(queries) == 0 { - return summary - } - var maxprio float64 - var bs bytes.Buffer var ms []report.MetricRow found := make(map[string]struct{}) @@ -127,18 +120,14 @@ func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { if metric.Priority > maxprio { maxprio = metric.Priority } - tpl := queries[metric.ID] - if tpl == nil { - continue - } - bs.Reset() - if err := tpl.Execute(&bs, summary); err != nil { - continue - } + query := metricQuery(summary, n, metric.ID) ms = append(ms, metric) - ms[len(ms)-1].URL = buildURL(bs.String()) + if query != "" { + ms[len(ms)-1].URL = metricURL(query) + } + found[metric.ID] = struct{}{} } @@ -148,13 +137,8 @@ func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { continue } - tpl := queries[metadata.ID] - if tpl == nil { - continue - } - - bs.Reset() - if err := tpl.Execute(&bs, summary); err != nil { + query := metricQuery(summary, n, metadata.ID) + if query == "" { continue } @@ -162,7 +146,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { ms = append(ms, report.MetricRow{ ID: metadata.ID, Label: metadata.Label, - URL: buildURL(bs.String()), + URL: metricURL(query), Metric: &report.Metric{}, Priority: maxprio, ValueEmpty: true, @@ -174,8 +158,22 @@ func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { return summary } -// buildURL puts together the URL by looking at the configured `metricsGraphURL`. -func buildURL(query string) string { +// metricQuery returns the query for the given node and metric. +func metricQuery(summary NodeSummary, n report.Node, metricID string) string { + t := n.Topology + if _, ok := k8sControllers[n.Topology]; ok { + t = "__k8s_controllers" + } + queries := topologyQueries[t] + if len(queries) == 0 { + return "" + } + + return strings.Replace(queries[metricID], "{{label}}", summary.Label, -1) +} + +// metricURL builds the URL by embedding it into the configured `metricsGraphURL`. +func metricURL(query string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { return strings.Replace(metricsGraphURL, urlQueryVarName, url.QueryEscape(query), -1) } @@ -192,8 +190,7 @@ func buildURL(query string) string { return metricsGraphURL + url.QueryEscape(params) } -// queryParamsAsJSON packs the query into a JSON of the -// format `{"cells":[{"queries":[$query]}]}`. +// queryParamsAsJSON packs the query into a JSON of the format `{"cells":[{"queries":[$query]}]}`. func queryParamsAsJSON(query string) (string, error) { type cell struct { Queries []string `json:"queries"` @@ -211,20 +208,3 @@ func queryParamsAsJSON(query string) (string, error) { return buf.String(), nil } - -// parsedTemplate initializes unnamed text templates. -func parsedTemplate(query string) *template.Template { - tpl, err := template.New("").Parse(query) - if err != nil { - panic(err) - } - - return tpl -} - -func getTopologyQueries(t string) map[string]*template.Template { - if _, ok := k8sControllers[t]; ok { - t = "__k8s_controllers" - } - return topologyQueries[t] -} From 9c610bfaecb367ce9dcb12deeaae152e9190e618 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 1 Aug 2017 10:37:07 +0200 Subject: [PATCH 21/31] Do not mix immutablejs with plain array functions --- .../app/scripts/components/node-details/node-details-health.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index 102c92d622..58fad14a32 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -1,5 +1,4 @@ import React from 'react'; -import { List as makeList } from 'immutable'; import ShowMore from '../show-more'; import NodeDetailsHealthOverflow from './node-details-health-overflow'; @@ -22,7 +21,7 @@ export default class NodeDetailsHealth extends React.Component { render() { const { - metrics = makeList(), + metrics = [], topologyId, } = this.props; From 60a04db33ff39c23dbe2dc96823e0bf5780aa20b Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 7 Aug 2017 16:59:07 +0100 Subject: [PATCH 22/31] No preview for overflow health items --- .../node-details/node-details-health-item.js | 7 ++- .../node-details-health-link-item.js | 3 + .../node-details-health-overflow-item.js | 16 ----- .../node-details-health-overflow.js | 26 -------- .../node-details/node-details-health.js | 39 +++++++----- client/app/scripts/components/sparkline.js | 61 ++++++++----------- 6 files changed, 56 insertions(+), 96 deletions(-) delete mode 100644 client/app/scripts/components/node-details/node-details-health-overflow-item.js delete mode 100644 client/app/scripts/components/node-details/node-details-health-overflow.js diff --git a/client/app/scripts/components/node-details/node-details-health-item.js b/client/app/scripts/components/node-details/node-details-health-item.js index 2004016ec8..be6a205ef0 100644 --- a/client/app/scripts/components/node-details/node-details-health-item.js +++ b/client/app/scripts/components/node-details/node-details-health-item.js @@ -4,17 +4,18 @@ import Sparkline from '../sparkline'; import { formatMetric } from '../../utils/string-utils'; function NodeDetailsHealthItem(props) { + const labelStyle = { color: props.labelColor }; return (
- {!props.valueEmpty &&
{formatMetric(props.value, props)}
} + {!props.valueEmpty &&
{formatMetric(props.value, props)}
}
-
+
{props.label}
diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index d6a43f6620..72f1fc6ec3 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -3,6 +3,7 @@ import React from 'react'; import NodeDetailsHealthItem from './node-details-health-item'; import CloudLink from '../cloud-link'; import { getMetricColor } from '../../utils/metric-utils'; +import { darkenColor } from '../../utils/color-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; export default class NodeDetailsHealthLinkItem extends React.Component { @@ -33,6 +34,7 @@ export default class NodeDetailsHealthLinkItem extends React.Component { render() { const { id, url, ...props } = this.props; const metricColor = getMetricColor(id); + const labelColor = this.state.hovered && !props.valueEmpty && darkenColor(metricColor); return ( diff --git a/client/app/scripts/components/node-details/node-details-health-overflow-item.js b/client/app/scripts/components/node-details/node-details-health-overflow-item.js deleted file mode 100644 index 4372d87dec..0000000000 --- a/client/app/scripts/components/node-details/node-details-health-overflow-item.js +++ /dev/null @@ -1,16 +0,0 @@ -import React from 'react'; - -import { formatMetric } from '../../utils/string-utils'; - -function NodeDetailsHealthOverflowItem(props) { - return ( -
-
- {!props.valueEmpty && formatMetric(props.value, props)} -
-
{props.label}
-
- ); -} - -export default NodeDetailsHealthOverflowItem; diff --git a/client/app/scripts/components/node-details/node-details-health-overflow.js b/client/app/scripts/components/node-details/node-details-health-overflow.js deleted file mode 100644 index 537c3a6950..0000000000 --- a/client/app/scripts/components/node-details/node-details-health-overflow.js +++ /dev/null @@ -1,26 +0,0 @@ -import React from 'react'; - -import NodeDetailsHealthOverflowItem from './node-details-health-overflow-item'; - -export default class NodeDetailsHealthOverflow extends React.Component { - - constructor(props, context) { - super(props, context); - this.handleClick = this.handleClick.bind(this); - } - - handleClick(ev) { - ev.preventDefault(); - this.props.handleClick(); - } - - render() { - const items = this.props.items.slice(0, 4); - - return ( -
- {items.map(item => )} -
- ); - } -} diff --git a/client/app/scripts/components/node-details/node-details-health.js b/client/app/scripts/components/node-details/node-details-health.js index 58fad14a32..8d763656de 100644 --- a/client/app/scripts/components/node-details/node-details-health.js +++ b/client/app/scripts/components/node-details/node-details-health.js @@ -1,7 +1,6 @@ import React from 'react'; import ShowMore from '../show-more'; -import NodeDetailsHealthOverflow from './node-details-health-overflow'; import NodeDetailsHealthLinkItem from './node-details-health-link-item'; export default class NodeDetailsHealth extends React.Component { @@ -25,30 +24,38 @@ export default class NodeDetailsHealth extends React.Component { topologyId, } = this.props; - const primeCutoff = metrics.length > 3 && !this.state.expanded ? 2 : metrics.length; - const primeMetrics = metrics.slice(0, primeCutoff); - const overflowMetrics = metrics.slice(primeCutoff); - const showOverflow = overflowMetrics.length > 0 && !this.state.expanded; - const flexWrap = showOverflow || !this.state.expanded ? 'nowrap' : 'wrap'; - const justifyContent = showOverflow || !this.state.expanded ? 'space-around' : 'flex-start'; - const notShown = overflowMetrics.length; + let primeMetrics = metrics.filter(m => !m.valueEmpty); + let emptyMetrics = metrics.filter(m => m.valueEmpty); + + if (primeMetrics.length === 0 && emptyMetrics.length > 0) { + primeMetrics = emptyMetrics; + emptyMetrics = []; + } + + const shownWithData = this.state.expanded ? primeMetrics : primeMetrics.slice(0, 3); + const shownEmpty = this.state.expanded ? emptyMetrics : []; + const notShown = metrics.length - shownWithData.length - shownEmpty.length; return ( -
+
+
+ {shownWithData.map(item => )} +
- {primeMetrics.map(item => )} - {showOverflow && }
+ handleClick={this.handleClickMore} collection={metrics} + expanded={this.state.expanded} notShown={notShown} hideNumber={this.state.expanded} + />
); } diff --git a/client/app/scripts/components/sparkline.js b/client/app/scripts/components/sparkline.js index b11e122624..898bd5f41e 100644 --- a/client/app/scripts/components/sparkline.js +++ b/client/app/scripts/components/sparkline.js @@ -7,11 +7,11 @@ import { line, curveLinear } from 'd3-shape'; import { scaleLinear } from 'd3-scale'; import { formatMetricSvg } from '../utils/string-utils'; -import { brightenColor, darkenColor } from '../utils/color-utils'; const HOVER_RADIUS_MULTIPLY = 1.5; const HOVER_STROKE_MULTIPLY = 5; +const MARGIN = 2; export default class Sparkline extends React.Component { constructor(props, context) { @@ -24,11 +24,15 @@ export default class Sparkline extends React.Component { .y(d => this.y(d.value)); } - initRanges() { + initRanges(hasCircle) { // adjust scales and leave some room for the circle on the right, upper, and lower edge - const padding = 2 + Math.ceil(this.props.circleRadius * HOVER_RADIUS_MULTIPLY); - this.x.range([2, this.props.width - padding]); - this.y.range([this.props.height - padding, padding]); + let circleSpace = MARGIN; + if (hasCircle) { + circleSpace += Math.ceil(this.props.circleRadius * HOVER_RADIUS_MULTIPLY); + } + + this.x.range([MARGIN, this.props.width - circleSpace]); + this.y.range([this.props.height - circleSpace, circleSpace]); this.line.curve(this.props.curve); } @@ -36,7 +40,7 @@ export default class Sparkline extends React.Component { // data is of shape [{date, value}, ...] and is sorted by date (ASC) let data = this.props.data; - this.initRanges(); + this.initRanges(true); // Convert dates into D3 dates data = data.map(d => ({ @@ -75,7 +79,7 @@ export default class Sparkline extends React.Component { } getEmptyGraphData() { - this.initRanges(); + this.initRanges(false); const first = new Date(0); const last = new Date(15); this.x.domain([first, last]); @@ -93,43 +97,30 @@ export default class Sparkline extends React.Component { } render() { - let strokeColor = this.props.strokeColor; - let strokeWidth = this.props.strokeWidth; - let radius = this.props.circleRadius; - let fillOpacity = 0.6; - let circleColor; - let graph = {}; - - if (!this.props.data || this.props.data.length === 0 || this.props.data[0].date === undefined) { - // no data means just a dead line w/o circle - graph = this.getEmptyGraphData(); - strokeColor = brightenColor(strokeColor); - radius = 0; - } else { - graph = this.getGraphData(); - - if (this.props.hovered) { - strokeColor = this.props.hoverColor; - circleColor = strokeColor; - strokeWidth *= HOVER_STROKE_MULTIPLY; - radius *= HOVER_RADIUS_MULTIPLY; - fillOpacity = 1; - } else { - circleColor = darkenColor(strokeColor); - } - } + const dash = 5; + const hasData = this.props.data && this.props.data.length > 0; + const strokeColor = this.props.hovered && hasData + ? this.props.hoverColor + : this.props.strokeColor; + const strokeWidth = this.props.strokeWidth * (this.props.hovered ? HOVER_STROKE_MULTIPLY : 1); + const strokeDasharray = hasData || `${dash}, ${dash}`; + const radius = this.props.circleRadius * (this.props.hovered ? HOVER_RADIUS_MULTIPLY : 1); + const fillOpacity = this.props.hovered ? 1 : 0.6; + const circleColor = hasData && this.props.hovered ? strokeColor : strokeColor; + const graph = hasData ? this.getGraphData() : this.getEmptyGraphData(); return (
- + />}
); From ca45b90aa4eb9d34a870b64be8fee76991f77297 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Thu, 10 Aug 2017 18:46:07 +0100 Subject: [PATCH 23/31] Use percentage and MB for CPU/Memory urls And more DRY. --- render/detailed/links.go | 85 +++++++++++++++++++---------------- render/detailed/links_test.go | 17 +++---- 2 files changed, 55 insertions(+), 47 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 6277aa50c0..ca02ed9fd1 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -2,6 +2,7 @@ package detailed import ( "bytes" + "fmt" "net/url" "strings" @@ -11,8 +12,13 @@ import ( "github.com/ugorji/go/codec" ) -// Replacement variable name for the query in the metrics graph url -const urlQueryVarName = ":query" +const ( + // Replacement variable name for the query in the metrics graph url + urlQueryVarName = ":query" + + idReceiveBytes = "receive_bytes" + idTransmitBytes = "transmit_bytes" +) var ( // As configured by the user @@ -32,54 +38,32 @@ var ( Label: "Memory", }, { - ID: "receive_bytes", + ID: idReceiveBytes, Label: "Rx/s", }, { - ID: "transmit_bytes", + ID: idTransmitBytes, Label: "Tx/s", }, } // Prometheus queries for topologies - // - // Metrics - // - `container_cpu_usage_seconds_total` --> cAdvisor in Kubelets. - // - `container_memory_usage_bytes` --> cAdvisor in Kubelets. topologyQueries = map[string]map[string]string{ // Containers - report.Container: { - docker.MemoryUsage: `sum(container_memory_usage_bytes{container_name="{{label}}"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{container_name="{{label}}"}[1m]))`, - }, - report.ContainerImage: { - docker.MemoryUsage: `sum(container_memory_usage_bytes{image="{{label}}"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{image="{{label}}"}[1m]))`, - }, - "group:container:docker_container_hostname": { - docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name="{{label}}"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name="{{label}}"}[1m]))`, - }, + report.Container: formatMetricQueries(`container_name="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), + report.ContainerImage: formatMetricQueries(`image="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), + "group:container:docker_container_hostname": formatMetricQueries(`pod_name="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), // Kubernetes topologies - report.Pod: { - docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name="{{label}}"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name="{{label}}"}[1m]))`, - "receive_bytes": `sum(rate(container_network_receive_bytes_total{pod_name="{{label}}"}[5m]))`, - "transmit_bytes": `sum(rate(container_network_transmit_bytes_total{pod_name="{{label}}"}[5m]))`, - }, - // Pod naming: - // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label - "__k8s_controllers": { - docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name=~"^{{label}}-[^-]+-[^-]+$"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{label}}-[^-]+-[^-]+$"}[1m]))`, - }, - report.DaemonSet: { - docker.MemoryUsage: `sum(container_memory_usage_bytes{pod_name=~"^{{label}}-[^-]+$"})`, - docker.CPUTotalUsage: `sum(rate(container_cpu_usage_seconds_total{pod_name=~"^{{label}}-[^-]+$"}[1m]))`, - }, + report.Pod: formatMetricQueries( + `pod_name="{{label}}"`, + []string{docker.MemoryUsage, docker.CPUTotalUsage, idReceiveBytes, idTransmitBytes}, + ), + // Pod naming: // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label + "__k8s_controllers": formatMetricQueries(`pod_name=~"^{{label}}-[^-]+-[^-]+$"}`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), + report.DaemonSet: formatMetricQueries(`pod_name=~"^{{label}}-[^-]+$"}`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), report.Service: { // These recording rules must be defined in the prometheus config. // NB: Pods need to be labeled and selected by their respective Service name, meaning: @@ -90,12 +74,35 @@ var ( }, } k8sControllers = map[string]struct{}{ - report.Deployment: {}, - "stateful_set": {}, - "cron_job": {}, + report.Deployment: {}, + report.StatefulSet: {}, + report.CronJob: {}, } ) +func formatMetricQueries(filter string, ids []string) map[string]string { + queries := make(map[string]string) + for _, id := range ids { + // All `container_*`metrics are provided by cAdvisor in Kubelets + switch id { + case docker.MemoryUsage: + queries[id] = fmt.Sprintf("sum(container_memory_usage_bytes{%s})/1024/1024", filter) + case docker.CPUTotalUsage: + queries[id] = fmt.Sprintf( + "sum(rate(container_cpu_usage_seconds_total{%s}[1m]))/count(container_cpu_usage_seconds_total{%s})*100", + filter, + filter, + ) + case idReceiveBytes: + queries[id] = fmt.Sprintf(`sum(rate(container_network_receive_bytes_total{%s}[5m]))`, filter) + case idTransmitBytes: + queries[id] = fmt.Sprintf(`sum(rate(container_network_transmit_bytes_total{%s}[5m]))`, filter) + } + } + + return queries +} + // SetMetricsGraphURL sets the URL we deduce our eventual metric link from. // Supports placeholders such as `:orgID` and `:query`. An empty url disables // this feature. If the `:query` part is missing, a JSON version will be diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index 0a107d6c3a..b1b87889d5 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -1,6 +1,7 @@ package detailed_test import ( + "strings" "testing" "github.com/weaveworks/scope/probe/docker" @@ -47,10 +48,10 @@ func TestRenderMetricURLs(t *testing.T) { s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} result := detailed.RenderMetricURLs(s, samplePodNode) - u := "/prom/:orgID/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22sum%28container_memory_usage_bytes%7Bpod_name%3D%5C%22foo%5C%22%7D%29%22%5D%7D%5D%7D" - assert.Equal(t, u, result.Metrics[0].URL) - u = "/prom/:orgID/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name%3D%5C%22foo%5C%22%7D%5B1m%5D%29%29%22%5D%7D%5D%7D" - assert.Equal(t, u, result.Metrics[1].URL) + assert.Equal(t, 0, strings.Index(result.Metrics[0].URL, sampleMetricsGraphURL)) + assert.Contains(t, result.Metrics[0].URL, "container_memory_usage_bytes%7Bpod_name%3D%5C%22foo%5C%22%7D") + assert.Equal(t, 0, strings.Index(result.Metrics[1].URL, sampleMetricsGraphURL)) + assert.Contains(t, result.Metrics[1].URL, "container_cpu_usage_seconds_total%7Bpod_name%3D%5C%22foo%5C%22%7D") } func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { @@ -94,8 +95,8 @@ func TestRenderMetricURLs_QueryReplacement(t *testing.T) { s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} result := detailed.RenderMetricURLs(s, samplePodNode) - u := "http://example.test/?q=sum%28container_memory_usage_bytes%7Bpod_name%3D%22foo%22%7D%29" - assert.Equal(t, u, result.Metrics[0].URL) - u = "http://example.test/?q=sum%28rate%28container_cpu_usage_seconds_total%7Bpod_name%3D%22foo%22%7D%5B1m%5D%29%29" - assert.Equal(t, u, result.Metrics[1].URL) + assert.Contains(t, result.Metrics[0].URL, "http://example.test/?q=") + assert.Contains(t, result.Metrics[0].URL, "container_memory_usage_bytes%7Bpod_name%3D%22foo%22%7D") + assert.Contains(t, result.Metrics[1].URL, "http://example.test/?q=") + assert.Contains(t, result.Metrics[1].URL, "container_cpu_usage_seconds_total%7Bpod_name%3D%22foo%22%7D") } From 0c167e0b103e70c8db496667fa322401afe443aa Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Fri, 11 Aug 2017 18:12:16 +0100 Subject: [PATCH 24/31] Pass timetravel timestamp to cortex in deeplink Closes #2753 --- .../node-details-health-link-item-test.js | 29 ++++++++++++ .../node-details-health-link-item.js | 47 +++++++++++++++++-- 2 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 client/app/scripts/components/node-details/__tests__/node-details-health-link-item-test.js diff --git a/client/app/scripts/components/node-details/__tests__/node-details-health-link-item-test.js b/client/app/scripts/components/node-details/__tests__/node-details-health-link-item-test.js new file mode 100644 index 0000000000..f379b2f307 --- /dev/null +++ b/client/app/scripts/components/node-details/__tests__/node-details-health-link-item-test.js @@ -0,0 +1,29 @@ +import moment from 'moment'; + +import { appendTime } from '../node-details-health-link-item'; + + +describe('NodeDetailsHealthLinkItem', () => { + describe('appendTime', () => { + const time = moment.unix(1496275200); + + it('returns url for empty url or time', () => { + expect(appendTime('', time)).toEqual(''); + expect(appendTime('foo', null)).toEqual('foo'); + expect(appendTime('', null)).toEqual(''); + }); + + it('appends as json for cloud link', () => { + const url = appendTime('/prom/:orgid/notebook/new/%7B%22cells%22%3A%5B%7B%22queries%22%3A%5B%22go_goroutines%22%5D%7D%5D%7D', time); + expect(url).toContain(time.unix()); + + const payload = JSON.parse(decodeURIComponent(url.substr(url.indexOf('new/') + 4))); + expect(payload.time.queryEnd).toEqual(time.unix()); + }); + + it('appends as GET parameter', () => { + expect(appendTime('http://example.test?q=foo', time)).toEqual('http://example.test?q=foo&time=1496275200'); + expect(appendTime('http://example.test/q=foo/', time)).toEqual('http://example.test/q=foo/?time=1496275200'); + }); + }); +}); diff --git a/client/app/scripts/components/node-details/node-details-health-link-item.js b/client/app/scripts/components/node-details/node-details-health-link-item.js index 72f1fc6ec3..f9084390ce 100644 --- a/client/app/scripts/components/node-details/node-details-health-link-item.js +++ b/client/app/scripts/components/node-details/node-details-health-link-item.js @@ -1,4 +1,5 @@ import React from 'react'; +import { connect } from 'react-redux'; import NodeDetailsHealthItem from './node-details-health-item'; import CloudLink from '../cloud-link'; @@ -6,7 +7,37 @@ import { getMetricColor } from '../../utils/metric-utils'; import { darkenColor } from '../../utils/color-utils'; import { trackMixpanelEvent } from '../../utils/tracking-utils'; -export default class NodeDetailsHealthLinkItem extends React.Component { +/** + * @param {string} url + * @param {Moment} time + * @returns {string} + */ +export function appendTime(url, time) { + if (!url || !time) return url; + + // rudimentary check whether we have a cloud link + const cloudLinkPathEnd = 'notebook/new/'; + const pos = url.indexOf(cloudLinkPathEnd); + if (pos !== -1) { + let payload; + const json = decodeURIComponent(url.substr(pos + cloudLinkPathEnd.length)); + try { + payload = JSON.parse(json); + payload.time = { queryEnd: time.unix() }; + } catch (e) { + return url; + } + + return `${url.substr(0, pos + cloudLinkPathEnd.length)}${encodeURIComponent(JSON.stringify(payload) || '')}`; + } + + if (url.indexOf('?') !== -1) { + return `${url}&time=${time.unix()}`; + } + return `${url}?time=${time.unix()}`; +} + +class NodeDetailsHealthLinkItem extends React.Component { constructor(props) { super(props); @@ -32,10 +63,12 @@ export default class NodeDetailsHealthLinkItem extends React.Component { } render() { - const { id, url, ...props } = this.props; + const { id, url, pausedAt, ...props } = this.props; const metricColor = getMetricColor(id); const labelColor = this.state.hovered && !props.valueEmpty && darkenColor(metricColor); + const timedUrl = appendTime(url, pausedAt); + return ( Date: Mon, 14 Aug 2017 14:21:26 +0100 Subject: [PATCH 25/31] Remove grouped containers since queries are incorrect --- render/detailed/links.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index ca02ed9fd1..53ee9ad386 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -51,9 +51,8 @@ var ( topologyQueries = map[string]map[string]string{ // Containers - report.Container: formatMetricQueries(`container_name="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), - report.ContainerImage: formatMetricQueries(`image="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), - "group:container:docker_container_hostname": formatMetricQueries(`pod_name="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), + report.Container: formatMetricQueries(`container_name="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), + report.ContainerImage: formatMetricQueries(`image="{{label}}"`, []string{docker.MemoryUsage, docker.CPUTotalUsage}), // Kubernetes topologies From bcb06947ff2fb48ceb969e2f5c9a8cb3ed39a4ec Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Wed, 9 Aug 2017 11:44:23 +0100 Subject: [PATCH 26/31] Revert "Revert "Avoid global by passing metricsGraphURL down"" This reverts commit 4e738016969d66bc7b1951f1d63f865f76a20bb8. --- app/api_report_test.go | 2 +- app/api_topologies.go | 12 ++++++------ app/api_topologies_test.go | 4 ++-- app/api_topology.go | 11 ++++++----- app/router.go | 8 ++++---- prog/app.go | 8 +++----- render/detailed/connections.go | 12 ++++++------ render/detailed/links.go | 13 ++----------- render/detailed/links_test.go | 22 ++++++---------------- render/detailed/node.go | 14 +++++++------- render/detailed/node_test.go | 8 ++++---- render/detailed/summary.go | 14 ++++++++------ render/detailed/summary_test.go | 6 +++--- 13 files changed, 58 insertions(+), 76 deletions(-) diff --git a/app/api_report_test.go b/app/api_report_test.go index 1d7bd2b69f..c1da6f8002 100644 --- a/app/api_report_test.go +++ b/app/api_report_test.go @@ -14,7 +14,7 @@ import ( func topologyServer() *httptest.Server { router := mux.NewRouter().SkipClean(true) - app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}) + app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}, "") return httptest.NewServer(router) } diff --git a/app/api_topologies.go b/app/api_topologies.go index 42a62f8e12..a56a243856 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -548,17 +548,17 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt return topology.renderer, nil, nil } -type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) +type reporterHandler func(context.Context, Reporter, string, http.ResponseWriter, *http.Request) -func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { +func captureReporter(rep Reporter, metricsGraphURL string, f reporterHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - f(ctx, rep, w, r) + f(ctx, rep, metricsGraphURL, w, r) } } -type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, string, http.ResponseWriter, *http.Request) -func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { +func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( topologyID = mux.Vars(req)["topology"] @@ -579,6 +579,6 @@ func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFu respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, rpt, w, req) + f(ctx, renderer, decorator, rpt, metricGraphsURL, w, req) } } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index a817eb796c..71c6906399 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -183,14 +183,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator)), nil + return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator), ""), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) app.RegisterReportPostHandler(c, router) - app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}) + app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}, "") ts := httptest.NewServer(router) defer ts.Close() diff --git a/app/api_topology.go b/app/api_topology.go index 173e275b99..a29f052d13 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,14 +29,14 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(report, renderer.Render(report, decorator)), + Nodes: detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, decorator 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, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] @@ -49,13 +49,14 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node, metricsGraphURL)}) } // Websocket for the full topology. func handleWebsocket( ctx context.Context, rep Reporter, + metricsGraphURL string, w http.ResponseWriter, r *http.Request, ) { @@ -123,7 +124,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(report, renderer.Render(report, decorator)) + newTopo := detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/router.go b/app/router.go index 414f45f99f..05999d293e 100644 --- a/app/router.go +++ b/app/router.go @@ -91,7 +91,7 @@ func gzipHandler(h http.HandlerFunc) http.HandlerFunc { } // RegisterTopologyRoutes registers the various topology routes with a http mux. -func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool) { +func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool, metricsGraphURL string) { get := router.Methods("GET").Subrouter() get.HandleFunc("/api", gzipHandler(requestContextDecorator(apiHandler(r, capabilities)))) @@ -99,15 +99,15 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[str gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get. HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleTopology)))). Name("api_topology_topology") get. HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(captureReporter(r, handleWebsocket))). // NB not gzip! + requestContextDecorator(captureReporter(r, metricsGraphURL, handleWebsocket))). // NB not gzip! Name("api_topology_topology_ws") get. MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleNode)))). Name("api_topology_topology_id") get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) diff --git a/prog/app.go b/prog/app.go index 2647ddb41a..f78054c9b8 100644 --- a/prog/app.go +++ b/prog/app.go @@ -28,7 +28,6 @@ import ( "github.com/weaveworks/scope/common/weave" "github.com/weaveworks/scope/common/xfer" "github.com/weaveworks/scope/probe/docker" - "github.com/weaveworks/scope/render/detailed" "github.com/weaveworks/weave/common" ) @@ -52,7 +51,7 @@ func init() { } // Router creates the mux for all the various app components. -func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool) http.Handler { +func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.PipeRouter, externalUI bool, capabilities map[string]bool, metricsGraphURL string) http.Handler { router := mux.NewRouter().SkipClean(true) // We pull in the http.DefaultServeMux to get the pprof routes @@ -62,7 +61,7 @@ func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.RegisterReportPostHandler(collector, router) app.RegisterControlRoutes(router, controlRouter) app.RegisterPipeRoutes(router, pipeRouter) - app.RegisterTopologyRoutes(router, collector, capabilities) + app.RegisterTopologyRoutes(router, collector, capabilities, metricsGraphURL) uiHandler := http.FileServer(GetFS(externalUI)) router.PathPrefix("/ui").Name("static").Handler( @@ -217,7 +216,6 @@ func appMain(flags appFlags) { setLogLevel(flags.logLevel) setLogFormatter(flags.logPrefix) runtime.SetBlockProfileRate(flags.blockProfileRate) - detailed.SetMetricsGraphURL(flags.metricsGraphURL) defer log.Info("app exiting") rand.Seed(time.Now().UnixNano()) @@ -299,7 +297,7 @@ func appMain(flags appFlags) { capabilities := map[string]bool{ xfer.HistoricReportsCapability: collector.HasHistoricReports(), } - handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities) + handler := router(collector, controlRouter, pipeRouter, flags.externalUI, capabilities, flags.metricsGraphURL) if flags.logHTTP { handler = middleware.Log{ LogRequestHeaders: flags.logHTTPHeaders, diff --git a/render/detailed/connections.go b/render/detailed/connections.go index b89ebb723a..337e565acd 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -121,13 +121,13 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) { return addr, true } -func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { +func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool, metricsGraphURL string) []Connection { output := []Connection{} for row, count := range c.counts { // Use MakeNodeSummary to render the id and label of this node // TODO(paulbellamy): Would be cleaner if we hade just a // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. - summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID]) + summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID], metricsGraphURL) connection := Connection{ ID: fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port), NodeID: summary.ID, @@ -162,7 +162,7 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal return output } -func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { +func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(n) counts := newConnectionCounters() @@ -188,11 +188,11 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Inbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n)), + Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), } } -func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { +func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { localEndpoints := endpointChildrenOf(n) counts := newConnectionCounters() @@ -220,7 +220,7 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Outbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n)), + Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), } } diff --git a/render/detailed/links.go b/render/detailed/links.go index 53ee9ad386..957d80bc14 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -21,9 +21,6 @@ const ( ) var ( - // As configured by the user - metricsGraphURL = "" - // Metadata for shown queries shownQueries = []struct { ID string @@ -102,17 +99,11 @@ func formatMetricQueries(filter string, ids []string) map[string]string { return queries } -// SetMetricsGraphURL sets the URL we deduce our eventual metric link from. -// Supports placeholders such as `:orgID` and `:query`. An empty url disables -// this feature. If the `:query` part is missing, a JSON version will be -// appended, see `queryParamsAsJSON()` for more info. -func SetMetricsGraphURL(url string) { - metricsGraphURL = url -} +var metricsGraphURL = "" // RenderMetricURLs sets respective URLs for metrics in a node summary. Missing metrics // where we have a query for will be appended as an empty metric (no values or samples). -func RenderMetricURLs(summary NodeSummary, n report.Node) NodeSummary { +func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { if metricsGraphURL == "" { return summary } diff --git a/render/detailed/links_test.go b/render/detailed/links_test.go index b1b87889d5..91b3f32911 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -26,27 +26,23 @@ var ( func TestRenderMetricURLs_Disabled(t *testing.T) { s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode) + result := detailed.RenderMetricURLs(s, samplePodNode, "") assert.Empty(t, result.Metrics[0].URL) assert.Empty(t, result.Metrics[1].URL) } func TestRenderMetricURLs_UnknownTopology(t *testing.T) { - detailed.SetMetricsGraphURL(sampleMetricsGraphURL) - defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, sampleUnknownNode) + result := detailed.RenderMetricURLs(s, sampleUnknownNode, sampleMetricsGraphURL) assert.Empty(t, result.Metrics[0].URL) assert.Empty(t, result.Metrics[1].URL) } func TestRenderMetricURLs(t *testing.T) { - detailed.SetMetricsGraphURL(sampleMetricsGraphURL) - defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode) + result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) assert.Equal(t, 0, strings.Index(result.Metrics[0].URL, sampleMetricsGraphURL)) assert.Contains(t, result.Metrics[0].URL, "container_memory_usage_bytes%7Bpod_name%3D%5C%22foo%5C%22%7D") @@ -55,9 +51,7 @@ func TestRenderMetricURLs(t *testing.T) { } func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { - detailed.SetMetricsGraphURL(sampleMetricsGraphURL) - defer detailed.SetMetricsGraphURL("") - result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode) + result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode, sampleMetricsGraphURL) m := result.Metrics[0] assert.Equal(t, docker.CPUTotalUsage, m.ID) @@ -73,13 +67,11 @@ func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { } func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) { - detailed.SetMetricsGraphURL(sampleMetricsGraphURL) - defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{ Label: "foo", Metrics: []report.MetricRow{{ID: docker.MemoryUsage, Priority: 1}}, } - result := detailed.RenderMetricURLs(s, samplePodNode) + result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) assert.NotEmpty(t, result.Metrics[0].URL) assert.False(t, result.Metrics[0].ValueEmpty) @@ -90,10 +82,8 @@ func TestRenderMetricURLs_CombinedEmptyMetrics(t *testing.T) { } func TestRenderMetricURLs_QueryReplacement(t *testing.T) { - detailed.SetMetricsGraphURL("http://example.test/?q=:query") - defer detailed.SetMetricsGraphURL("") s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} - result := detailed.RenderMetricURLs(s, samplePodNode) + result := detailed.RenderMetricURLs(s, samplePodNode, "http://example.test/?q=:query") assert.Contains(t, result.Metrics[0].URL, "http://example.test/?q=") assert.Contains(t, result.Metrics[0].URL, "container_memory_usage_bytes%7Bpod_name%3D%22foo%22%7D") diff --git a/render/detailed/node.go b/render/detailed/node.go index 8e211b8138..1481d99d8c 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -80,15 +80,15 @@ func (c *ControlInstance) CodecDecodeSelf(decoder *codec.Decoder) { // 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, r report.Report, ns report.Nodes, n report.Node) Node { - summary, _ := MakeNodeSummary(r, n) +func MakeNode(topologyID string, r report.Report, ns report.Nodes, n report.Node, metricsGraphURL string) Node { + summary, _ := MakeNodeSummary(r, n, metricsGraphURL) return Node{ NodeSummary: summary, Controls: controls(r, n), - Children: children(r, n), + Children: children(r, n, metricsGraphURL), Connections: []ConnectionsSummary{ - incomingConnectionsSummary(topologyID, r, n, ns), - outgoingConnectionsSummary(topologyID, r, n, ns), + incomingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), + outgoingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), }, } } @@ -181,13 +181,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(r report.Report, n report.Node) []NodeSummaryGroup { +func children(r report.Report, n report.Node, metricsGraphURL string) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(r, child) + summary, ok := MakeNodeSummary(r, child, metricsGraphURL) if !ok { return } diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index d7c213ae45..b612297599 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(fixture.Report, r.Render(fixture.Report, nil)[id]) + s, ok := detailed.MakeNodeSummary(fixture.Report, r.Render(fixture.Report, nil)[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, nil) renderableNode := renderableNodes[fixture.ClientHostNodeID] - have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode, "") containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) @@ -183,7 +183,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode, "") serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) serverProcessNodeSummary.Linkable = true @@ -313,7 +313,7 @@ func TestMakeDetailedPodNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("pods", fixture.Report, renderableNodes, renderableNode) + have := detailed.MakeNode("pods", 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 825819bec0..d302f6b38b 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -102,12 +102,12 @@ var primaryAPITopology = map[string]string{ } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) { +func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (NodeSummary, bool) { if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { summary, b := renderer(baseNodeSummary(r, n), n) - return RenderMetricURLs(summary, n), b + return RenderMetricURLs(summary, n, metricsGraphURL), b } } else if _, ok := r.Topology(n.Topology); ok { summary := baseNodeSummary(r, n) @@ -116,7 +116,7 @@ func MakeNodeSummary(r report.Report, n report.Node) (NodeSummary, bool) { } if strings.HasPrefix(n.Topology, "group:") { summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n) - return RenderMetricURLs(summary, n), b + return RenderMetricURLs(summary, n, metricsGraphURL), b } return NodeSummary{}, false } @@ -134,7 +134,7 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { func baseNodeSummary(r report.Report, n report.Node) NodeSummary { t, _ := r.Topology(n.Topology) - return NodeSummary{ + ns := NodeSummary{ ID: n.ID, Shape: t.GetShape(), Linkable: true, @@ -144,6 +144,8 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary { Tables: NodeTables(r, n), Adjacency: n.Adjacency, } + + return ns } func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) { @@ -372,11 +374,11 @@ 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(r report.Report, rns report.Nodes) NodeSummaries { +func Summaries(r report.Report, rns report.Nodes, metricsGraphURL string) NodeSummaries { result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(r, node); ok { + if summary, ok := MakeNodeSummary(r, node, metricsGraphURL); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index e4a5ad72d7..4bf843bc46 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(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil)) + have := detailed.Summaries(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil), "") // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -51,7 +51,7 @@ func TestSummaries(t *testing.T) { input := fixture.Report.Copy() input.Process.Nodes[fixture.ClientProcess1NodeID].Metrics[process.CPUUsage] = metric - have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil)) + have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil), "") node, ok := have[fixture.ClientProcess1NodeID] if !ok { @@ -184,7 +184,7 @@ func TestMakeNodeSummary(t *testing.T) { }, } for _, testcase := range testcases { - have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input) + have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input, "") if ok != testcase.ok { t.Errorf("%s: MakeNodeSummary failed: expected ok value to be: %v", testcase.name, testcase.ok) continue From f6af1788a6f1633d136a73c740f83d5a936deb30 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Wed, 9 Aug 2017 17:30:36 +0100 Subject: [PATCH 27/31] Get rid of metricsGraphURL singleton Fixes #2740 --- app/api_topologies.go | 6 +++--- app/api_topology.go | 19 +++++++++++-------- render/detailed/connections.go | 12 ++++++------ render/detailed/links.go | 10 ++++------ render/detailed/node.go | 18 +++++++++--------- render/detailed/summary.go | 13 +++++++------ report/report.go | 6 ++++++ 7 files changed, 46 insertions(+), 38 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index a56a243856..f32cf1cd08 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -556,9 +556,9 @@ func captureReporter(rep Reporter, metricsGraphURL string, f reporterHandler) Ct } } -type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.Report, string, http.ResponseWriter, *http.Request) +type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.RenderContext, http.ResponseWriter, *http.Request) -func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rendererHandler) CtxHandlerFunc { +func (r *Registry) captureRenderer(rep Reporter, metricsGraphURL string, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( topologyID = mux.Vars(req)["topology"] @@ -579,6 +579,6 @@ func (r *Registry) captureRenderer(rep Reporter, metricGraphsURL string, f rende respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, rpt, metricGraphsURL, w, req) + f(ctx, renderer, decorator, report.RenderContext{Report: rpt, MetricsGraphURL: metricsGraphURL}, w, req) } } diff --git a/app/api_topology.go b/app/api_topology.go index a29f052d13..6d97101059 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -29,27 +29,27 @@ type APINode struct { } // Full topology. -func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { +func handleTopology(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { respondWith(w, http.StatusOK, APITopology{ - Nodes: detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL), + Nodes: detailed.Summaries(rc, renderer.Render(rc.Report, decorator)), }) } // Individual nodes. -func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, report report.Report, metricsGraphURL string, w http.ResponseWriter, r *http.Request) { +func handleNode(ctx context.Context, renderer render.Renderer, decorator render.Decorator, rc report.RenderContext, w http.ResponseWriter, r *http.Request) { var ( vars = mux.Vars(r) topologyID = vars["topology"] nodeID = vars["id"] preciousRenderer = render.PreciousNodeRenderer{PreciousNodeID: nodeID, Renderer: renderer} - rendered = preciousRenderer.Render(report, decorator) + rendered = preciousRenderer.Render(rc.Report, decorator) node, ok = rendered[nodeID] ) if !ok { http.NotFound(w, r) return } - respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, report, rendered, node, metricsGraphURL)}) + respondWith(w, http.StatusOK, APINode{Node: detailed.MakeNode(topologyID, rc, rendered, node)}) } // Websocket for the full topology. @@ -114,17 +114,20 @@ func handleWebsocket( // might be interested in implementing in the future. timestampDelta := time.Since(channelOpenedAt) reportTimestamp := startReportingAt.Add(timestampDelta) - report, err := rep.Report(ctx, reportTimestamp) + re, err := rep.Report(ctx, reportTimestamp) if err != nil { log.Errorf("Error generating report: %v", err) return } - renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, r.Form, report) + renderer, decorator, err := topologyRegistry.RendererForTopology(topologyID, r.Form, re) if err != nil { log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries(report, renderer.Render(report, decorator), metricsGraphURL) + newTopo := detailed.Summaries( + report.RenderContext{Report: re, MetricsGraphURL: metricsGraphURL}, + renderer.Render(re, decorator), + ) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/render/detailed/connections.go b/render/detailed/connections.go index 337e565acd..05afcd5f42 100644 --- a/render/detailed/connections.go +++ b/render/detailed/connections.go @@ -121,13 +121,13 @@ func internetAddr(node report.Node, ep report.Node) (string, bool) { return addr, true } -func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool, metricsGraphURL string) []Connection { +func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal bool) []Connection { output := []Connection{} for row, count := range c.counts { // Use MakeNodeSummary to render the id and label of this node // TODO(paulbellamy): Would be cleaner if we hade just a // MakeNodeID(ns[row.remoteNodeID]). As we don't need the whole summary. - summary, _ := MakeNodeSummary(r, ns[row.remoteNodeID], metricsGraphURL) + summary, _ := MakeNodeSummary(report.RenderContext{Report: r}, ns[row.remoteNodeID]) connection := Connection{ ID: fmt.Sprintf("%s-%s-%s-%s", row.remoteNodeID, row.remoteAddr, row.localAddr, row.port), NodeID: summary.ID, @@ -162,7 +162,7 @@ func (c *connectionCounters) rows(r report.Report, ns report.Nodes, includeLocal return output } -func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { +func incomingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { localEndpointIDs, localEndpointIDCopies := endpointChildIDsAndCopyMapOf(n) counts := newConnectionCounters() @@ -188,11 +188,11 @@ func incomingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Inbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), + Connections: counts.rows(r, ns, isInternetNode(n)), } } -func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes, metricsGraphURL string) ConnectionsSummary { +func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Node, ns report.Nodes) ConnectionsSummary { localEndpoints := endpointChildrenOf(n) counts := newConnectionCounters() @@ -220,7 +220,7 @@ func outgoingConnectionsSummary(topologyID string, r report.Report, n report.Nod TopologyID: topologyID, Label: "Outbound", Columns: columnHeaders, - Connections: counts.rows(r, ns, isInternetNode(n), metricsGraphURL), + Connections: counts.rows(r, ns, isInternetNode(n)), } } diff --git a/render/detailed/links.go b/render/detailed/links.go index 957d80bc14..b2c2fba7cf 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -99,8 +99,6 @@ func formatMetricQueries(filter string, ids []string) map[string]string { return queries } -var metricsGraphURL = "" - // RenderMetricURLs sets respective URLs for metrics in a node summary. Missing metrics // where we have a query for will be appended as an empty metric (no values or samples). func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string) NodeSummary { @@ -119,10 +117,10 @@ func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string } query := metricQuery(summary, n, metric.ID) - ms = append(ms, metric) + if query != "" { - ms[len(ms)-1].URL = metricURL(query) + ms[len(ms)-1].URL = metricURL(query, metricsGraphURL) } found[metric.ID] = struct{}{} @@ -143,7 +141,7 @@ func RenderMetricURLs(summary NodeSummary, n report.Node, metricsGraphURL string ms = append(ms, report.MetricRow{ ID: metadata.ID, Label: metadata.Label, - URL: metricURL(query), + URL: metricURL(query, metricsGraphURL), Metric: &report.Metric{}, Priority: maxprio, ValueEmpty: true, @@ -170,7 +168,7 @@ func metricQuery(summary NodeSummary, n report.Node, metricID string) string { } // metricURL builds the URL by embedding it into the configured `metricsGraphURL`. -func metricURL(query string) string { +func metricURL(query, metricsGraphURL string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { return strings.Replace(metricsGraphURL, urlQueryVarName, url.QueryEscape(query), -1) } diff --git a/render/detailed/node.go b/render/detailed/node.go index 1481d99d8c..59e6b00128 100644 --- a/render/detailed/node.go +++ b/render/detailed/node.go @@ -80,15 +80,15 @@ func (c *ControlInstance) CodecDecodeSelf(decoder *codec.Decoder) { // 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, r report.Report, ns report.Nodes, n report.Node, metricsGraphURL string) Node { - summary, _ := MakeNodeSummary(r, n, metricsGraphURL) +func MakeNode(topologyID string, rc report.RenderContext, ns report.Nodes, n report.Node) Node { + summary, _ := MakeNodeSummary(rc, n) return Node{ NodeSummary: summary, - Controls: controls(r, n), - Children: children(r, n, metricsGraphURL), + Controls: controls(rc.Report, n), + Children: children(rc, n), Connections: []ConnectionsSummary{ - incomingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), - outgoingConnectionsSummary(topologyID, r, n, ns, metricsGraphURL), + incomingConnectionsSummary(topologyID, rc.Report, n, ns), + outgoingConnectionsSummary(topologyID, rc.Report, n, ns), }, } } @@ -181,13 +181,13 @@ var nodeSummaryGroupSpecs = []struct { }, } -func children(r report.Report, n report.Node, metricsGraphURL string) []NodeSummaryGroup { +func children(rc report.RenderContext, n report.Node) []NodeSummaryGroup { summaries := map[string][]NodeSummary{} n.Children.ForEach(func(child report.Node) { if child.ID == n.ID { return } - summary, ok := MakeNodeSummary(r, child, metricsGraphURL) + summary, ok := MakeNodeSummary(rc, child) if !ok { return } @@ -216,7 +216,7 @@ func children(r report.Report, n report.Node, metricsGraphURL string) []NodeSumm if len(nodeSummaries) == 0 { continue } - topology, ok := r.Topology(topologyID) + topology, ok := rc.Topology(topologyID) if !ok { continue } diff --git a/render/detailed/summary.go b/render/detailed/summary.go index d302f6b38b..58f0cec322 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -102,21 +102,22 @@ var primaryAPITopology = map[string]string{ } // MakeNodeSummary summarizes a node, if possible. -func MakeNodeSummary(r report.Report, n report.Node, metricsGraphURL string) (NodeSummary, bool) { +func MakeNodeSummary(rc report.RenderContext, n report.Node) (NodeSummary, bool) { + r := rc.Report if renderer, ok := renderers[n.Topology]; ok { // Skip (and don't fall through to fallback) if renderer maps to nil if renderer != nil { summary, b := renderer(baseNodeSummary(r, n), n) - return RenderMetricURLs(summary, n, metricsGraphURL), b + return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b } - } else if _, ok := r.Topology(n.Topology); ok { + } else if _, ok := rc.Topology(n.Topology); ok { summary := baseNodeSummary(r, n) summary.Label = n.ID // This is unlikely to look very good, but is a reasonable fallback return summary, true } if strings.HasPrefix(n.Topology, "group:") { summary, b := groupNodeSummary(baseNodeSummary(r, n), r, n) - return RenderMetricURLs(summary, n, metricsGraphURL), b + return RenderMetricURLs(summary, n, rc.MetricsGraphURL), b } return NodeSummary{}, false } @@ -374,11 +375,11 @@ 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(r report.Report, rns report.Nodes, metricsGraphURL string) NodeSummaries { +func Summaries(rc report.RenderContext, rns report.Nodes) NodeSummaries { result := NodeSummaries{} for id, node := range rns { - if summary, ok := MakeNodeSummary(r, node, metricsGraphURL); ok { + if summary, ok := MakeNodeSummary(rc, node); ok { for i, m := range summary.Metrics { summary.Metrics[i] = m.Summary() } diff --git a/report/report.go b/report/report.go index 9cd6c6908f..84eb885501 100644 --- a/report/report.go +++ b/report/report.go @@ -148,6 +148,12 @@ 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{ From 4779557968de68115bb040d97ba5a6d860e73c15 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Fri, 11 Aug 2017 14:19:16 +0100 Subject: [PATCH 28/31] Introduce WebReporter to hold web-related options --- app/api_topologies.go | 10 +++++----- app/api_topology.go | 6 +----- app/collector.go | 17 +++++++++++++++++ app/router.go | 8 ++++---- prog/app.go | 2 +- 5 files changed, 28 insertions(+), 15 deletions(-) diff --git a/app/api_topologies.go b/app/api_topologies.go index f32cf1cd08..92e4e3d11b 100644 --- a/app/api_topologies.go +++ b/app/api_topologies.go @@ -548,17 +548,17 @@ func (r *Registry) RendererForTopology(topologyID string, values url.Values, rpt return topology.renderer, nil, nil } -type reporterHandler func(context.Context, Reporter, string, http.ResponseWriter, *http.Request) +type reporterHandler func(context.Context, Reporter, http.ResponseWriter, *http.Request) -func captureReporter(rep Reporter, metricsGraphURL string, f reporterHandler) CtxHandlerFunc { +func captureReporter(rep Reporter, f reporterHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) { - f(ctx, rep, metricsGraphURL, w, r) + f(ctx, rep, w, r) } } type rendererHandler func(context.Context, render.Renderer, render.Decorator, report.RenderContext, http.ResponseWriter, *http.Request) -func (r *Registry) captureRenderer(rep Reporter, metricsGraphURL string, f rendererHandler) CtxHandlerFunc { +func (r *Registry) captureRenderer(rep Reporter, f rendererHandler) CtxHandlerFunc { return func(ctx context.Context, w http.ResponseWriter, req *http.Request) { var ( topologyID = mux.Vars(req)["topology"] @@ -579,6 +579,6 @@ func (r *Registry) captureRenderer(rep Reporter, metricsGraphURL string, f rende respondWith(w, http.StatusInternalServerError, err) return } - f(ctx, renderer, decorator, report.RenderContext{Report: rpt, MetricsGraphURL: metricsGraphURL}, w, req) + f(ctx, renderer, decorator, RenderContextForReporter(rep, rpt), w, req) } } diff --git a/app/api_topology.go b/app/api_topology.go index 6d97101059..a30f554bd1 100644 --- a/app/api_topology.go +++ b/app/api_topology.go @@ -56,7 +56,6 @@ func handleNode(ctx context.Context, renderer render.Renderer, decorator render. func handleWebsocket( ctx context.Context, rep Reporter, - metricsGraphURL string, w http.ResponseWriter, r *http.Request, ) { @@ -124,10 +123,7 @@ func handleWebsocket( log.Errorf("Error generating report: %v", err) return } - newTopo := detailed.Summaries( - report.RenderContext{Report: re, MetricsGraphURL: metricsGraphURL}, - renderer.Render(re, decorator), - ) + newTopo := detailed.Summaries(RenderContextForReporter(rep, re), renderer.Render(re, decorator)) diff := detailed.TopoDiff(previousTopo, newTopo) previousTopo = newTopo diff --git a/app/collector.go b/app/collector.go index d4b97dc6ce..434edcd814 100644 --- a/app/collector.go +++ b/app/collector.go @@ -34,6 +34,23 @@ type Reporter interface { UnWait(context.Context, chan struct{}) } +// 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 +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/app/router.go b/app/router.go index 05999d293e..414f45f99f 100644 --- a/app/router.go +++ b/app/router.go @@ -91,7 +91,7 @@ func gzipHandler(h http.HandlerFunc) http.HandlerFunc { } // RegisterTopologyRoutes registers the various topology routes with a http mux. -func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool, metricsGraphURL string) { +func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[string]bool) { get := router.Methods("GET").Subrouter() get.HandleFunc("/api", gzipHandler(requestContextDecorator(apiHandler(r, capabilities)))) @@ -99,15 +99,15 @@ func RegisterTopologyRoutes(router *mux.Router, r Reporter, capabilities map[str gzipHandler(requestContextDecorator(topologyRegistry.makeTopologyList(r)))) get. HandleFunc("/api/topology/{topology}", - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleTopology)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleTopology)))). Name("api_topology_topology") get. HandleFunc("/api/topology/{topology}/ws", - requestContextDecorator(captureReporter(r, metricsGraphURL, handleWebsocket))). // NB not gzip! + requestContextDecorator(captureReporter(r, handleWebsocket))). // NB not gzip! Name("api_topology_topology_ws") get. MatcherFunc(URLMatcher("/api/topology/{topology}/{id}")).HandlerFunc( - gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, metricsGraphURL, handleNode)))). + gzipHandler(requestContextDecorator(topologyRegistry.captureRenderer(r, handleNode)))). Name("api_topology_topology_id") get.HandleFunc("/api/report", gzipHandler(requestContextDecorator(makeRawReportHandler(r)))) diff --git a/prog/app.go b/prog/app.go index f78054c9b8..d4b9ca8aab 100644 --- a/prog/app.go +++ b/prog/app.go @@ -61,7 +61,7 @@ func router(collector app.Collector, controlRouter app.ControlRouter, pipeRouter app.RegisterReportPostHandler(collector, router) app.RegisterControlRoutes(router, controlRouter) app.RegisterPipeRoutes(router, pipeRouter) - app.RegisterTopologyRoutes(router, collector, capabilities, metricsGraphURL) + app.RegisterTopologyRoutes(router, app.WebReporter{Reporter: collector, MetricsGraphURL: metricsGraphURL}, capabilities) uiHandler := http.FileServer(GetFS(externalUI)) router.PathPrefix("/ui").Name("static").Handler( From b47cfbd3b512ee4aac2c1b2fa84e4cb7e05485cf Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 14 Aug 2017 18:40:55 +0100 Subject: [PATCH 29/31] Update tests to new code --- app/api_report_test.go | 2 +- app/api_topologies_test.go | 4 ++-- render/detailed/node_test.go | 8 ++++---- render/detailed/summary.go | 4 +--- render/detailed/summary_test.go | 6 +++--- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/app/api_report_test.go b/app/api_report_test.go index c1da6f8002..1d7bd2b69f 100644 --- a/app/api_report_test.go +++ b/app/api_report_test.go @@ -14,7 +14,7 @@ import ( func topologyServer() *httptest.Server { router := mux.NewRouter().SkipClean(true) - app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}, "") + app.RegisterTopologyRoutes(router, app.StaticCollector(fixture.Report), map[string]bool{"foo_capability": true}) return httptest.NewServer(router) } diff --git a/app/api_topologies_test.go b/app/api_topologies_test.go index 71c6906399..cb2fa6efe7 100644 --- a/app/api_topologies_test.go +++ b/app/api_topologies_test.go @@ -183,14 +183,14 @@ func getTestContainerLabelFilterTopologySummary(t *testing.T, exclude bool) (det return nil, err } - return detailed.Summaries(fixture.Report, renderer.Render(fixture.Report, decorator), ""), nil + return detailed.Summaries(report.RenderContext{Report: fixture.Report}, renderer.Render(fixture.Report, decorator)), nil } func TestAPITopologyAddsKubernetes(t *testing.T) { router := mux.NewRouter() c := app.NewCollector(1 * time.Minute) app.RegisterReportPostHandler(c, router) - app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}, "") + app.RegisterTopologyRoutes(router, c, map[string]bool{"foo_capability": true}) ts := httptest.NewServer(router) defer ts.Close() diff --git a/render/detailed/node_test.go b/render/detailed/node_test.go index b612297599..19fca09b6f 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(fixture.Report, r.Render(fixture.Report, nil)[id], "") + s, ok := detailed.MakeNodeSummary(report.RenderContext{Report: fixture.Report}, r.Render(fixture.Report, nil)[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, nil) renderableNode := renderableNodes[fixture.ClientHostNodeID] - have := detailed.MakeNode("hosts", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("hosts", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) containerImageNodeSummary := child(t, render.ContainerImageRenderer, expected.ClientContainerImageNodeID) containerNodeSummary := child(t, render.ContainerRenderer, fixture.ClientContainerNodeID) @@ -183,7 +183,7 @@ func TestMakeDetailedContainerNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("containers", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("containers", report.RenderContext{Report: fixture.Report}, renderableNodes, renderableNode) serverProcessNodeSummary := child(t, render.ProcessRenderer, fixture.ServerProcessNodeID) serverProcessNodeSummary.Linkable = true @@ -313,7 +313,7 @@ func TestMakeDetailedPodNode(t *testing.T) { if !ok { t.Fatalf("Node not found: %s", id) } - have := detailed.MakeNode("pods", fixture.Report, renderableNodes, renderableNode, "") + have := detailed.MakeNode("pods", report.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 58f0cec322..6f0e508ae2 100644 --- a/render/detailed/summary.go +++ b/render/detailed/summary.go @@ -135,7 +135,7 @@ func (n NodeSummary) SummarizeMetrics() NodeSummary { func baseNodeSummary(r report.Report, n report.Node) NodeSummary { t, _ := r.Topology(n.Topology) - ns := NodeSummary{ + return NodeSummary{ ID: n.ID, Shape: t.GetShape(), Linkable: true, @@ -145,8 +145,6 @@ func baseNodeSummary(r report.Report, n report.Node) NodeSummary { Tables: NodeTables(r, n), Adjacency: n.Adjacency, } - - return ns } func pseudoNodeSummary(base NodeSummary, n report.Node) (NodeSummary, bool) { diff --git a/render/detailed/summary_test.go b/render/detailed/summary_test.go index 4bf843bc46..1065a7ee39 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(fixture.Report, render.ProcessRenderer.Render(fixture.Report, nil), "") + have := detailed.Summaries(report.RenderContext{Report: fixture.Report}, render.ProcessRenderer.Render(fixture.Report, nil)) // The ids of the processes rendered above expectedIDs := []string{ fixture.ClientProcess1NodeID, @@ -51,7 +51,7 @@ func TestSummaries(t *testing.T) { input := fixture.Report.Copy() input.Process.Nodes[fixture.ClientProcess1NodeID].Metrics[process.CPUUsage] = metric - have := detailed.Summaries(input, render.ProcessRenderer.Render(input, nil), "") + have := detailed.Summaries(report.RenderContext{Report: input}, render.ProcessRenderer.Render(input, nil)) node, ok := have[fixture.ClientProcess1NodeID] if !ok { @@ -184,7 +184,7 @@ func TestMakeNodeSummary(t *testing.T) { }, } for _, testcase := range testcases { - have, ok := detailed.MakeNodeSummary(fixture.Report, testcase.input, "") + have, ok := detailed.MakeNodeSummary(report.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 From c059f9a97cb1e3d4700d2701fe62f20f265819ea Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Mon, 14 Aug 2017 19:10:57 +0100 Subject: [PATCH 30/31] Decode WS with %20 instead + See https://github.com/ReactTraining/react-router/commit/fbc109c0218508a301caae36469c8fc5d70de5fa --- render/detailed/links.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index b2c2fba7cf..936788a614 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -170,7 +170,7 @@ func metricQuery(summary NodeSummary, n report.Node, metricID string) string { // metricURL builds the URL by embedding it into the configured `metricsGraphURL`. func metricURL(query, metricsGraphURL string) string { if strings.Contains(metricsGraphURL, urlQueryVarName) { - return strings.Replace(metricsGraphURL, urlQueryVarName, url.QueryEscape(query), -1) + return strings.Replace(metricsGraphURL, urlQueryVarName, queryEscape(query), -1) } params, err := queryParamsAsJSON(query) @@ -182,7 +182,7 @@ func metricURL(query, metricsGraphURL string) string { metricsGraphURL += "/" } - return metricsGraphURL + url.QueryEscape(params) + return metricsGraphURL + queryEscape(params) } // queryParamsAsJSON packs the query into a JSON of the format `{"cells":[{"queries":[$query]}]}`. @@ -203,3 +203,9 @@ func queryParamsAsJSON(query string) (string, error) { return buf.String(), nil } + +// queryEscape uses `%20` instead of `+` to encode whitespaces. Both are +// valid but react-router does not decode `+` properly. +func queryEscape(query string) string { + return url.QueryEscape(strings.Replace(query, " ", "%20", -1)) +} From 44c23fa5ddf0a878ae5ca606c19316b9deead262 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Tue, 15 Aug 2017 12:39:10 +0100 Subject: [PATCH 31/31] Disable net traffic metric links --- render/detailed/links.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render/detailed/links.go b/render/detailed/links.go index 936788a614..6cd0cbab12 100644 --- a/render/detailed/links.go +++ b/render/detailed/links.go @@ -55,7 +55,7 @@ var ( report.Pod: formatMetricQueries( `pod_name="{{label}}"`, - []string{docker.MemoryUsage, docker.CPUTotalUsage, idReceiveBytes, idTransmitBytes}, + []string{docker.MemoryUsage, docker.CPUTotalUsage}, ), // Pod naming: // https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#pod-template-hash-label "__k8s_controllers": formatMetricQueries(`pod_name=~"^{{label}}-[^-]+-[^-]+$"}`, []string{docker.MemoryUsage, docker.CPUTotalUsage}),