Skip to content

Commit

Permalink
Miscellaneous improvements to the Top Routes UX (linkerd#1963)
Browse files Browse the repository at this point in the history
* Renames UNKNOWN in the tables to (default) which is less scary (linkerd#1946)
* adds a tooltip explaining what (default) is
* adds url props to the Top Routes page, so that they query can be populated by a url
* fixes a js error that occurs when switching pages
  • Loading branch information
Risha Mars committed Dec 10, 2018
1 parent 0f8bcc9 commit 6214c9a
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 21 deletions.
13 changes: 9 additions & 4 deletions web/app/js/components/BaseTable.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ class BaseTable extends React.Component {
);
}

return _.isNil(col.tooltip) ? tableCell : <Tooltip key={col.key || col.dataIndex} placement="top" title={col.tooltip}>{tableCell}</Tooltip>;
return _.isNil(col.tooltip) ? tableCell :
<Tooltip key={col.key || col.dataIndex} placement="top" title={col.tooltip}>{tableCell}</Tooltip>;
}

render() {
Expand All @@ -116,7 +117,7 @@ class BaseTable extends React.Component {
{
_.map(sortedTableRows, d => {
let key = !rowKey ? d.key : rowKey(d);
return (
let tableRow = (
<TableRow key={key}>
{ _.map(tableColumns, c => (
<TableCell
Expand All @@ -125,10 +126,14 @@ class BaseTable extends React.Component {
numeric={c.isNumeric}>
{c.render ? c.render(d) : _.get(d, c.dataIndex)}
</TableCell>
))}
)
)}
</TableRow>
);
})}
return _.isNil(d.tooltip) ? tableRow :
<Tooltip key={`table-row-${key}`} placement="left" title={d.tooltip}>{tableRow}</Tooltip>;
}
)}
</TableBody>
</Table>
</Paper>
Expand Down
57 changes: 44 additions & 13 deletions web/app/js/components/TopRoutes.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { UrlQueryParamTypes, addUrlProps } from 'react-url-query';
import Button from '@material-ui/core/Button';
import Card from '@material-ui/core/Card';
import CardContent from '@material-ui/core/CardContent';
Expand All @@ -20,6 +21,16 @@ import { groupResourcesByNs } from './util/MetricUtils.jsx';
import { withContext } from './util/AppContext.jsx';
import { withStyles } from '@material-ui/core/styles';

const topRoutesQueryProps = {
resource_name: PropTypes.string,
resource_type: PropTypes.string,
namespace: PropTypes.string,
};
const topRoutesQueryPropType = PropTypes.shape(topRoutesQueryProps);

const urlPropsQueryConfig = _.mapValues(topRoutesQueryProps, () => {
return { type: UrlQueryParamTypes.string };
});

const styles = theme => ({
root: {
Expand All @@ -35,24 +46,28 @@ class TopRoutes extends React.Component {
api: PropTypes.shape({
PrefixedLink: PropTypes.func.isRequired,
}).isRequired,
classes: PropTypes.shape({}).isRequired
classes: PropTypes.shape({}).isRequired,
query: topRoutesQueryPropType,
}
static defaultProps = {
query: {
resource_name: '',
resource_type: '',
namespace: '',
},
}

constructor(props) {
super(props);
this.api = this.props.api;

let query = _.merge({}, props.query, _.pick(this.props, _.keys(topRoutesQueryProps)));

this.state = {
query: {
resource_name: '',
namespace: '',
from_name: '',
from_type: '',
from_namespace: ''
},
query: query,
error: null,
services: [],
namespaces: [],
namespaces: ["default"],
resourcesByNs: {},
pollingInterval: 5000,
pendingRequests: false,
Expand All @@ -61,10 +76,12 @@ class TopRoutes extends React.Component {
}

componentDidMount() {
this._isMounted = true; // https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html
this.startServerPolling();
}

componentWillUnmount() {
this._isMounted = false;
this.stopServerPolling();
}

Expand Down Expand Up @@ -124,9 +141,18 @@ class TopRoutes extends React.Component {
});
}

// Each time state.query is updated, this method calls the equivalent
// onChange method to reflect the update in url query params. These onChange
// methods are automatically added to props by react-url-query.
handleUrlUpdate = (name, formVal) => {
this.props[`onChange${_.upperFirst(name)}`](formVal);
}

handleNamespaceSelect = e => {
let query = this.state.query;
query.namespace = _.get(e, 'target.value');
let formVal = _.get(e, 'target.value');
query.namespace = formVal;
this.handleUrlUpdate("namespace", formVal);
this.setState({ query });
};

Expand All @@ -136,6 +162,8 @@ class TopRoutes extends React.Component {
let [resource_type, resource_name] = resource.split("/");
query.resource_name = resource_name;
query.resource_type = resource_type;
this.handleUrlUpdate("resource_name", resource_name);
this.handleUrlUpdate("resource_type", resource_type);
this.setState({ query });
}

Expand Down Expand Up @@ -215,11 +243,14 @@ class TopRoutes extends React.Component {
.map(svc => `service/${svc.name}`).value();
let otherResources = resourcesByNs[query.namespace] || [];


let dropdownOptions = _.sortBy(_.concat(servicesWithPrefix, otherResources));
let dropdownVal = _.isEmpty(query.resource_name) || _.isEmpty(query.resource_type) ? "" :
query.resource_type + "/" + query.resource_name;

if (_.isEmpty(dropdownOptions) && !_.isEmpty(dropdownVal)) {
dropdownOptions = [dropdownVal]; // populate from url if autocomplete hasn't loaded
}

return (
<FormControl className={classes.formControl}>
<InputLabel htmlFor={`${key}-dropdown`}>Resource</InputLabel>
Expand Down Expand Up @@ -255,11 +286,11 @@ class TopRoutes extends React.Component {
{ this.renderRoutesQueryForm() }
{ emptyQuery ? null :
<QueryToCliCmd cmdName="routes" query={query} resource={query.resource_type + "/" + query.resource_name} /> }
{ !this.state.requestInProgress ? null : <TopRoutesModule query={this.state.query} /> }
{ !this.state.requestInProgress || !this._isMounted ? null : <TopRoutesModule query={this.state.query} /> }
</Card>
</div>
);
}
}

export default withContext(withStyles(styles, { withTheme: true })(TopRoutes));
export default addUrlProps({ urlPropsQueryConfig })(withContext(withStyles(styles, { withTheme: true })(TopRoutes)));
5 changes: 3 additions & 2 deletions web/app/js/components/TopRoutesModule.jsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { DefaultRoute, processTopRoutesResults } from './util/MetricUtils.jsx';

import ConfigureProfilesMsg from './ConfigureProfilesMsg.jsx';
import ErrorBanner from './ErrorBanner.jsx';
import PropTypes from 'prop-types';
Expand All @@ -6,7 +8,6 @@ import Spinner from './util/Spinner.jsx';
import TopRoutesTable from './TopRoutesTable.jsx';
import _ from 'lodash';
import { apiErrorPropType } from './util/ApiHelpers.jsx';
import { processTopRoutesResults } from './util/MetricUtils.jsx';
import withREST from './util/withREST.jsx';

class TopRoutesBase extends React.Component {
Expand Down Expand Up @@ -41,7 +42,7 @@ class TopRoutesBase extends React.Component {
render() {
const {data, loading} = this.props;
let metrics = processTopRoutesResults(_.get(data, '[0].routes.rows', []));
let allRoutesUnknown = _.every(metrics, m => m.route === "UNKNOWN");
let allRoutesUnknown = _.every(metrics, m => m.route === DefaultRoute);
let showCallToAction = !loading && (_.isEmpty(metrics) || allRoutesUnknown);

return (
Expand Down
12 changes: 11 additions & 1 deletion web/app/js/components/TopRoutesTable.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { metricToFormatter, numericSort } from './util/Utils.js';
import BaseTable from './BaseTable.jsx';
import { DefaultRoute } from './util/MetricUtils.jsx';
import PropTypes from 'prop-types';
import React from 'react';
import SuccessRateMiniChart from './util/SuccessRateMiniChart.jsx';
Expand All @@ -8,7 +9,16 @@ const routesColumns = [
{
title: "Route",
dataIndex: "route",
sorter: (a, b) => (a.route).localeCompare(b.route)
sorter: (a, b) => {
if (a.route === DefaultRoute) {
return 1;
} else {
if (b.route === DefaultRoute) {
return -1;
}
}
return (a.route).localeCompare(b.route);
}
},
{
title: "Authority",
Expand Down
4 changes: 3 additions & 1 deletion web/app/js/components/util/MetricUtils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,11 @@ const processStatTable = table => {
.value();
};

export const DefaultRoute = "[default]";
export const processTopRoutesResults = rows => {
return _.map(rows, row => ({
route: row.route || "UNKNOWN",
route: row.route || DefaultRoute,
tooltip: !_.isEmpty(row.route) ? null : "Traffic does not match any configured routes",
authority: row.authority,
totalRequests: getTotalRequests(row),
requestRate: getRequestRate(row),
Expand Down

0 comments on commit 6214c9a

Please sign in to comment.