Skip to content

Commit

Permalink
Addressed all the UI comments (@davkal + @fons)
Browse files Browse the repository at this point in the history
  • Loading branch information
fbarl committed Mar 23, 2017
1 parent 117df1f commit dd7107b
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 68 deletions.
1 change: 0 additions & 1 deletion client/app/scripts/actions/app-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ export function clickResumeUpdate() {
function updateTopology(dispatch, getState) {
const state = getState();
// If we're in the resource view, get the snapshot of all the relevant node topologies.
// TODO: Consider updating the state to always have a pinned metric.
if (isResourceViewModeSelector(state)) {
getResourceViewNodesSnapshot(getState, dispatch);
}
Expand Down
6 changes: 1 addition & 5 deletions client/app/scripts/components/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ import { focusSearch, pinNextMetric, hitBackspace, hitEnter, hitEsc, unpinMetric
import Details from './details';
import Nodes from './nodes';
import ViewModeSelector from './view-mode-selector';
import MetricSelector from './metric-selector';
import NetworkSelector from './networks-selector';
import DebugToolbar, { showingDebugToolbar, toggleDebugToolbar } from './debug-toolbar';
import { getRouter, getUrlState } from '../utils/router-utils';
import { availableMetricsSelector } from '../selectors/node-metric';
import { availableNetworksSelector } from '../selectors/node-networks';
import {
activeTopologyOptionsSelector,
Expand Down Expand Up @@ -110,7 +108,7 @@ class App extends React.Component {

render() {
const { isTableViewMode, isGraphViewMode, isResourceViewMode, showingDetails, showingHelp,
showingMetricsSelector, showingNetworkSelector, showingTroubleshootingMenu } = this.props;
showingNetworkSelector, showingTroubleshootingMenu } = this.props;
const isIframe = window !== window.top;

return (
Expand All @@ -137,7 +135,6 @@ class App extends React.Component {
<Nodes />

<Sidebar classNames={isTableViewMode ? 'sidebar-gridmode' : ''}>
{showingMetricsSelector && isGraphViewMode && <MetricSelector />}
{showingNetworkSelector && isGraphViewMode && <NetworkSelector />}
{!isResourceViewMode && <Status />}
{!isResourceViewMode && <TopologyOptions />}
Expand All @@ -162,7 +159,6 @@ function mapStateToProps(state) {
showingDetails: state.get('nodeDetails').size > 0,
showingHelp: state.get('showingHelp'),
showingTroubleshootingMenu: state.get('showingTroubleshootingMenu'),
showingMetricsSelector: availableMetricsSelector(state).count() > 0,
showingNetworkSelector: availableNetworksSelector(state).count() > 0,
showingTerminal: state.get('controlPipes').size > 0,
urlState: getUrlState(state)
Expand Down
9 changes: 2 additions & 7 deletions client/app/scripts/components/metric-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { connect } from 'react-redux';

import { selectMetric } from '../actions/app-actions';
import { availableMetricsSelector, pinnedMetricSelector } from '../selectors/node-metric';
import { availableMetricsSelector } from '../selectors/node-metric';
import MetricSelectorItem from './metric-selector-item';

class MetricSelector extends React.Component {
Expand All @@ -17,8 +17,7 @@ class MetricSelector extends React.Component {
}

render() {
const { pinnedMetric, alwaysPinned, availableMetrics } = this.props;
const shouldPinMetric = alwaysPinned && !pinnedMetric && !availableMetrics.isEmpty();
const { alwaysPinned, availableMetrics } = this.props;

return (
<div className="metric-selector">
Expand All @@ -31,9 +30,6 @@ class MetricSelector extends React.Component {
/>
))}
</div>
{shouldPinMetric && <span className="metric-selector-message">
&laquo; Select a metric
</span>}
</div>
);
}
Expand All @@ -42,7 +38,6 @@ class MetricSelector extends React.Component {
function mapStateToProps(state) {
return {
availableMetrics: availableMetricsSelector(state),
pinnedMetric: pinnedMetricSelector(state),
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,19 @@
import React from 'react';

import { formatMetricSvg } from '../../utils/string-utils';


export default class NodeResourcesMetricBoxInfo extends React.Component {
humanizedMetricInfo() {
const metricSummary = this.props.metricSummary.toJS();
const showExtendedInfo = metricSummary.showCapacity && metricSummary.format !== 'percent';
const totalCapacity = formatMetricSvg(metricSummary.totalCapacity, metricSummary);
const absoluteConsumption = formatMetricSvg(metricSummary.absoluteConsumption, metricSummary);
const relativeConsumption = formatMetricSvg(100.0 * metricSummary.relativeConsumption,
{ format: 'percent' });
const { humanizedTotalCapacity, humanizedAbsoluteConsumption,
humanizedRelativeConsumption, showCapacity, format } = this.props.metricSummary.toJS();
const showExtendedInfo = showCapacity && format !== 'percent';

return (
<span>
<strong>
{showExtendedInfo ? relativeConsumption : absoluteConsumption}
</strong> consumed
{showExtendedInfo ? humanizedRelativeConsumption : humanizedAbsoluteConsumption}
</strong> used
{showExtendedInfo && <i>{' - '}
({absoluteConsumption} / <strong>{totalCapacity}</strong>)
({humanizedAbsoluteConsumption} / <strong>{humanizedTotalCapacity}</strong>)
</i>}
</span>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class NodeResourcesMetricBox extends React.Component {
render() {
const { x, y, width } = this.state;
const { label, color, metricSummary } = this.props;
const { showCapacity, relativeConsumption } = metricSummary.toJS();
const { showCapacity, relativeConsumption, type } = metricSummary.toJS();

const showInfo = width >= RESOURCES_LABEL_MIN_SIZE;
const showNode = width >= 1; // hide the thin nodes
Expand All @@ -80,8 +80,13 @@ class NodeResourcesMetricBox extends React.Component {
// TODO: Show `+ 31 nodes` kind of tag in their stead.
if (!showNode) return null;

const resourceUsageTooltipInfo = showCapacity ?
metricSummary.get('humanizedRelativeConsumption') :
metricSummary.get('humanizedAbsoluteConsumption');

return (
<g className="node-resources-metric-box">
<title>{label} - {type} usage at {resourceUsageTooltipInfo}</title>
{showCapacity && <rect className="frame" {...this.defaultRectProps()} />}
<rect className="bar" fill={color} {...this.defaultRectProps(relativeConsumption)} />
{showInfo && <NodeResourcesMetricBoxInfo
Expand Down
4 changes: 3 additions & 1 deletion client/app/scripts/components/view-mode-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import classNames from 'classnames';
import MetricSelector from './metric-selector';
import { setGraphView, setTableView, setResourceView } from '../actions/app-actions';
import { layersTopologyIdsSelector } from '../selectors/resource-view/layout';
import { availableMetricsSelector } from '../selectors/node-metric';
import {
isGraphViewModeSelector,
isTableViewModeSelector,
Expand Down Expand Up @@ -45,7 +46,7 @@ class ViewModeSelector extends React.Component {
{Item('fa fa-bar-chart', 'Resources', isResourceViewMode, this.props.setResourceView,
hasResourceView)}
</div>
{isResourceViewMode && <MetricSelector alwaysPinned />}
<MetricSelector alwaysPinned={isResourceViewMode} />
</div>
);
}
Expand All @@ -57,6 +58,7 @@ function mapStateToProps(state) {
isTableViewMode: isTableViewModeSelector(state),
isResourceViewMode: isResourceViewModeSelector(state),
hasResourceView: !layersTopologyIdsSelector(state).isEmpty(),
showingMetricsSelector: availableMetricsSelector(state).count() > 0,
};
}

Expand Down
19 changes: 17 additions & 2 deletions client/app/scripts/constants/resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,23 @@ export const RESOURCE_VIEW_MAX_LAYERS = 3;

// TODO: Consider fetching these from the backend.
export const TOPOLOGIES_WITH_CAPACITY = ['hosts'];

// TODO: These too should ideally be provided by the backend. Currently, we are showing
// the same layers for all the topologies, because their number is small, but later on
// we might be interested in fully customizing the layers' hierarchy per topology.
export const RESOURCE_VIEW_LAYERS = {
hosts: ['hosts', 'containers', 'processes'],
containers: ['containers', 'processes'],
processes: ['processes'],
containers: ['hosts', 'containers', 'processes'],
processes: ['hosts', 'containers', 'processes'],
};

// TODO: These are all the common metrics that appear across all the current resource view
// topologies. The reason for taking them only is that we want to get meaningful data for all
// the layers. These should be taken directly from the backend report, but as their info is
// currently only contained in the nodes data, it would be hard to determine them before all
// the nodes for all the layers have been loaded, so we'd need to change the routing logic
// since the requirement is that one these is always pinned in the resource view.
export const RESOURCE_VIEW_METRICS = [
{ label: 'CPU', id: 'host_cpu_usage_percent' },
{ label: 'Memory', id: 'host_mem_usage_bytes' },
];
13 changes: 12 additions & 1 deletion client/app/scripts/decorators/node.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Map as makeMap } from 'immutable';

import { getNodeColor } from '../utils/color-utils';
import { formatMetricSvg } from '../utils/string-utils';
import { RESOURCES_LAYER_HEIGHT } from '../constants/styles';


Expand Down Expand Up @@ -34,10 +35,20 @@ export function nodeMetricSummaryDecoratorByType(metricType, showCapacity) {
const absoluteConsumption = metric.get('value');
const totalCapacity = showCapacity ? metric.get('max') : absoluteConsumption;
const relativeConsumption = absoluteConsumption / totalCapacity;
const defaultMetric = { format: metric.get('format') };
const percentMetric = { format: 'percent' };
const format = metric.get('format');

return node.set('metricSummary', makeMap({
showCapacity, totalCapacity, absoluteConsumption, relativeConsumption, format
showCapacity,
type: metricType,
humanizedTotalCapacity: formatMetricSvg(totalCapacity, defaultMetric),
humanizedAbsoluteConsumption: formatMetricSvg(absoluteConsumption, defaultMetric),
humanizedRelativeConsumption: formatMetricSvg(100 * relativeConsumption, percentMetric),
totalCapacity,
absoluteConsumption,
relativeConsumption,
format,
}));
};
}
Expand Down
2 changes: 1 addition & 1 deletion client/app/scripts/selectors/graph-view/graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { canvasWidthSelector, canvasHeightSelector } from '../canvas';
import { activeTopologyOptionsSelector } from '../topology';
import { shownNodesSelector } from '../node-filters';
import { doLayout } from '../../charts/nodes-layout';
import timer from '../utils/timer-utils';
import timer from '../../utils/timer-utils';

const log = debug('scope:nodes-chart');

Expand Down
39 changes: 25 additions & 14 deletions client/app/scripts/selectors/node-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,41 @@ import { createSelector } from 'reselect';
import { createMapSelector } from 'reselect-map';
import { fromJS, Map as makeMap, List as makeList } from 'immutable';

import {
isResourceViewModeSelector,
cachedCurrentTopologyNodesSelector,
} from '../selectors/topology';
import { isGraphViewModeSelector, isResourceViewModeSelector } from '../selectors/topology';
import { RESOURCE_VIEW_METRICS } from '../constants/resources';


// Resource view uses the metrics of the nodes from the cache, while the graph and table
// view are looking at the current nodes (which are among other things filtered by topology
// options which are currently ignored in the resource view).
export const availableMetricsSelector = createSelector(
[
isGraphViewModeSelector,
isResourceViewModeSelector,
cachedCurrentTopologyNodesSelector,
state => state.get('nodes'),
],
(isResourceView, cachedCurrentTopologyNodes, freshNodes) => (
(isResourceView ? cachedCurrentTopologyNodes : freshNodes)
.valueSeq()
.flatMap(n => n.get('metrics', makeList()))
.map(m => makeMap({ id: m.get('id'), label: m.get('label') }))
.toSet()
.toList()
.sortBy(m => m.get('label'))
)
(isGraphView, isResourceView, nodes) => {
// In graph view, we always look through the fresh state
// of topology nodes to get all the available metrics.
if (isGraphView) {
return nodes
.valueSeq()
.flatMap(n => n.get('metrics', makeList()))
.map(m => makeMap({ id: m.get('id'), label: m.get('label') }))
.toSet()
.toList()
.sortBy(m => m.get('label'));
}

// In resource view, we're displaying only the hardcoded CPU and Memory metrics.
// TODO: Make this dynamic as well.
if (isResourceView) {
return fromJS(RESOURCE_VIEW_METRICS);
}

// Don't show any metrics in the table view mode.
return makeList();
}
);

export const pinnedMetricSelector = createSelector(
Expand Down
16 changes: 3 additions & 13 deletions client/app/scripts/selectors/topology.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { createSelector } from 'reselect';
import { Map as makeMap } from 'immutable';

import {
RESOURCE_VIEW_MODE,
Expand Down Expand Up @@ -31,17 +30,6 @@ export const isResourceViewModeSelector = createSelector(
viewMode => viewMode === RESOURCE_VIEW_MODE
);

// This is used by the resource view where we're always taking the nodes from the cache,
// so that polling doesn't affect the layout. Once we implement a more robust polling
// mechanism that could poll multiple topologies at once, we'll be able to get rid of this.
export const cachedCurrentTopologyNodesSelector = createSelector(
[
state => state.get('nodesByTopology'),
state => state.get('currentTopologyId'),
],
(nodesByTopology, currentTopologyId) => nodesByTopology.get(currentTopologyId, makeMap())
);

// Checks if graph complexity is high. Used to trigger
// table view on page load and decide on animations.
export const graphExceedsComplexityThreshSelector = createSelector(
Expand Down Expand Up @@ -78,6 +66,8 @@ export const activeTopologyZoomCacheKeyPathSelector = createSelector(
['zoomCache', viewMode, topologyId, topologyOptions] :
// Otherwise we're in the resource view where the options are hidden (for now),
// but pinning different metrics can result in very different layouts.
['zoomCache', viewMode, topologyId, pinnedMetricType]
// TODO: Take `topologyId` into account once the resource
// view layouts start differing between the topologies.
['zoomCache', viewMode, pinnedMetricType]
)
);
6 changes: 3 additions & 3 deletions client/app/scripts/utils/web-api-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ export function getAllNodes(getState, dispatch) {
*/
export function getResourceViewNodesSnapshot(getState, dispatch) {
const topologyIds = layersTopologyIdsSelector(getState());
// TODO: Replace this with polling once we figure how to make resource view dynamic
// (from the UI point of view, the challenge is to make it stable).
// TODO: Remove the timeout and replace it with normal polling once we figure how to make
// resource view dynamic (from the UI point of view, the challenge is to make it stable).
setTimeout(() => {
getNodesForTopologies(getState, dispatch, topologyIds);
}, 100);
}, 1200);
}

export function getTopologies(options, dispatch, initialPoll) {
Expand Down
8 changes: 0 additions & 8 deletions client/app/styles/_base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -1256,14 +1256,6 @@
}
}

.metric-selector-message {
animation: blinking 1.0s infinite $base-ease;
color: $text-tertiary-color;
display: inline-block;
margin: 0;
padding: 0 10px;
}

.view-mode-selector {
margin-top: 8px;
margin-left: 20px;
Expand Down

0 comments on commit dd7107b

Please sign in to comment.