From 8188b7aed2b6bd936eade7a6112512b6e07af5e6 Mon Sep 17 00:00:00 2001 From: Roland Schilter Date: Wed, 19 Jul 2017 11:01:47 +0200 Subject: [PATCH] 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 | 184 +++++++++++------- render/detailed/links_test.go | 102 ++++++---- render/detailed/summary.go | 43 ++-- report/metric_row.go | 79 ++++---- 23 files changed, 518 insertions(+), 290 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 7826c54ad..9c6793951 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 c34277b8f..61f4ada49 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 000000000..315919e7b --- /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 dc1a6e8e7..fc3368dba 100644 --- a/client/app/scripts/components/node-details.js +++ b/client/app/scripts/components/node-details.js @@ -54,28 +54,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}`; @@ -187,8 +165,6 @@ class NodeDetails extends React.Component { } }; - const { metrics, metricLinks } = NodeDetails.collectMetrics(details); - return (
{tools} @@ -214,11 +190,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 89cb1131a..2004016ec 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 690ba5854..b2417249c 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 36c6ff6de..4372d87de 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 e323d75bb..5ceddca6b 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 b0dc2b79d..000000000 --- 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 2f17cd33c..4b910d943 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 6fff5f049..d5f41f270 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 f802d329c..b11e12262 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 6ac3f5d44..4058534fd 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 676d7af04..2f9b08274 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 4350bddb6..e14547d62 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 3cacf9200..860989829 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; @@ -928,18 +932,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; @@ -948,10 +956,12 @@ } &-link-item { - @extend .btn-opacity; + @extend .palable; cursor: pointer; opacity: $link-opacity-default; width: 33%; + display: flex; + color: inherit; .label { text-transform: uppercase; @@ -1105,6 +1115,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 5d971a1c9..3245191ef 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 8518dc6ff..a4cc86908 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 5a078a545..acb2210f9 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{ - 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. + // 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]))`), }, - 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. + + // 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]))`), + }, + // 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,40 @@ 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] - if len(queries) == 0 { - return nil - } - - links := []MetricLink{} - for _, link := range linkTemplates { - if _, ok := queries[link.ID]; ok { - links = append(links, link) - } - } - - return links -} - -// 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 { +// 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 } - links := []MetricLink{} + queries := getTopologyQueries(n.Topology) + if len(queries) == 0 { + return summary + } + + var maxprio float64 var bs bytes.Buffer - for _, link := range summary.MetricLinks { - tpl := queries[link.ID] + 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 } @@ -107,19 +126,47 @@ func RenderMetricLinks(summary NodeSummary, n report.Node, metricsGraphURL strin continue } - link.URL = buildURL(bs.String(), metricsGraphURL) - links = append(links, link) + ms = append(ms, metric) + ms[len(ms)-1].URL = buildURL(bs.String(), metricsGraphURL) + found[metric.ID] = struct{}{} } - summary.MetricLinks = links + + // 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 + } + + bs.Reset() + if err := tpl.Execute(&bs, summary); err != nil { + continue + } + + 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.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 29328d7d2..811f51fc5 100644 --- a/render/detailed/links_test.go +++ b/render/detailed/links_test.go @@ -10,54 +10,82 @@ import ( "github.com/stretchr/testify/assert" ) -var ( - sampleReport = report.Report{} - samplePodNode = report.MakeNode("noo").WithTopology(report.Pod) - sampleUnknownNode = report.MakeNode("???").WithTopology("foo") +const ( + sampleMetricsGraphURL = "/prom/:orgID/notebook/new" ) -func TestNodeMetricLinks_UnknownTopology(t *testing.T) { - links := detailed.NodeMetricLinks(sampleReport, sampleUnknownNode) - assert.Nil(t, links) -} - -func TestNodeMetricLinks(t *testing.T) { - expected := []detailed.MetricLink{ - {ID: docker.CPUTotalUsage, Label: "CPU", Priority: 1, URL: ""}, - {ID: docker.MemoryUsage, Label: "Memory", Priority: 2, URL: ""}, +var ( + sampleUnknownNode = report.MakeNode("???").WithTopology("foo") + samplePodNode = report.MakeNode("noo").WithTopology(report.Pod) + sampleMetrics = []report.MetricRow{ + {ID: docker.MemoryUsage}, + {ID: docker.CPUTotalUsage}, } +) - links := detailed.NodeMetricLinks(sampleReport, samplePodNode) - assert.Equal(t, expected, 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 TestRenderMetricLinks_UnknownTopology(t *testing.T) { - summary := detailed.NodeSummary{} +func TestRenderMetricURLs_UnknownTopology(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, sampleUnknownNode, sampleMetricsGraphURL) - result := detailed.RenderMetricLinks(summary, sampleUnknownNode, "") - assert.Equal(t, summary, result) + assert.Empty(t, result.Metrics[0].URL) + assert.Empty(t, result.Metrics[1].URL) } -func TestRenderMetricLinks_Pod(t *testing.T) { - summary := detailed.NodeSummary{Label: "woo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} +func TestRenderMetricURLs(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, samplePodNode, sampleMetricsGraphURL) - 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) + 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_QueryReplacement(t *testing.T) { - summary := detailed.NodeSummary{Label: "boo", MetricLinks: detailed.NodeMetricLinks(sampleReport, samplePodNode)} +func TestRenderMetricURLs_EmptyMetrics(t *testing.T) { + result := detailed.RenderMetricURLs(detailed.NodeSummary{}, samplePodNode, sampleMetricsGraphURL) - 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) + 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) + + 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 TestRenderMetricURLs_QueryReplacement(t *testing.T) { + s := detailed.NodeSummary{Label: "foo", Metrics: sampleMetrics} + result := detailed.RenderMetricURLs(s, samplePodNode, "http://example.test/?q=:query") + + 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 697e7ecdd..d302f6b38 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 b33b37851..2d61e0218 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, } }