Better handle TCP only resources in the dashboard (#2532)

When a resource only has TCP traffic and no HTTP traffic, the dashboard looks weird in a bunch of places. This branch: - updates the main resource card in the Octopus graph to show TCP stats if no HTTP stats are available - cleans up the resource detail page to show fewer blank tables if the resource only has TCP traffic.
2019-03-26 14:39:54 -07:00 · 2019-03-26 14:39:54 -07:00 · 408cffdc23
parent 9c5bb4ec0c
commit 408cffdc23
4 changed files with 109 additions and 60 deletions
--- a/web/app/js/components/Octopus.jsx
+++ b/web/app/js/components/Octopus.jsx
@ -22,7 +22,7 @@ import _slice from 'lodash/slice';
 import _sortBy from 'lodash/sortBy';
 import _take from 'lodash/take';
 import _times from 'lodash/times';
-import { getSuccessRateClassification } from './util/MetricUtils.jsx' ;
+import { getSuccessRateClassification } from './util/MetricUtils.jsx';
 import { withStyles } from "@material-ui/core/styles";

 const maxNumNeighbors = 6; // max number of neighbor nodes to show in the octopus graph
@ -105,6 +105,12 @@ class Octopus extends React.Component {
    let classification = getSuccessRateClassification(resource.successRate);
    let Progress = StyledProgress(classification);

+    // if the resource only has TCP stats, display those instead
+    let showTcp = false;
+    if (_isNil(resource.successRate) && _isNil(resource.requestRate)) {
+      showTcp = true;
+    }
+
    return (
      <Grid item key={resource.type + "-" + resource.name} >
        <Card className={type === "neighbor" ? classes.neighborNode : classes.centerNode} title={display}>
@ -117,20 +123,7 @@ class Octopus extends React.Component {
            <Progress variant="determinate" value={resource.successRate * 100} />

            <Table padding="dense">
-              <TableBody>
-                <TableRow>
-                  <TableCell><Typography>SR</Typography></TableCell>
-                  <TableCell numeric={true}><Typography>{metricToFormatter["SUCCESS_RATE"](resource.successRate)}</Typography></TableCell>
-                </TableRow>
-                <TableRow>
-                  <TableCell><Typography>RPS</Typography></TableCell>
-                  <TableCell numeric={true}><Typography>{metricToFormatter["NO_UNIT"](resource.requestRate)}</Typography></TableCell>
-                </TableRow>
-                <TableRow>
-                  <TableCell><Typography>P99</Typography></TableCell>
-                  <TableCell numeric={true}><Typography>{metricToFormatter["LATENCY"](_get(resource, "latency.P99"))}</Typography></TableCell>
-                </TableRow>
-              </TableBody>
+              {showTcp ? this.renderTCPStats(resource) : this.renderHttpStats(resource)}
            </Table>
          </CardContent>
        </Card>
@ -138,6 +131,45 @@ class Octopus extends React.Component {
    );
  }

+  renderHttpStats(resource) {
+    return (
+      <TableBody>
+        <TableRow>
+          <TableCell><Typography>SR</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["SUCCESS_RATE"](resource.successRate)}</Typography></TableCell>
+        </TableRow>
+        <TableRow>
+          <TableCell><Typography>RPS</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["NO_UNIT"](resource.requestRate)}</Typography></TableCell>
+        </TableRow>
+        <TableRow>
+          <TableCell><Typography>P99</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["LATENCY"](_get(resource, "latency.P99"))}</Typography></TableCell>
+        </TableRow>
+      </TableBody>
+    );
+  }
+
+  renderTCPStats(resource) {
+    let { tcp } = resource;
+    return (
+      <TableBody>
+        <TableRow>
+          <TableCell><Typography>Conn</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["NO_UNIT"](tcp.openConnections)}</Typography></TableCell>
+        </TableRow>
+        <TableRow>
+          <TableCell><Typography>Read</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["BYTES"](tcp.readRate)}</Typography></TableCell>
+        </TableRow>
+        <TableRow>
+          <TableCell><Typography>Write</Typography></TableCell>
+          <TableCell numeric={true}><Typography>{metricToFormatter["BYTES"](tcp.writeRate)}</Typography></TableCell>
+        </TableRow>
+      </TableBody>
+    );
+  }
+
  renderUnmeshedResources = unmeshedResources => {
    const { classes } = this.props;
    return (
@ -210,7 +242,7 @@ class Octopus extends React.Component {
  }

  render() {
-    let { resource, neighbors, unmeshedSources, classes} = this.props;
+    let { resource, neighbors, unmeshedSources, classes } = this.props;

    if (_isEmpty(resource)) {
      return null;
@ -268,7 +300,7 @@ class Octopus extends React.Component {
              alignItems="center"
              item
              xs={3}>
-              { display.downstreams.displayed.map(n => this.renderResourceCard(n, "neighbor"))}
+              {display.downstreams.displayed.map(n => this.renderResourceCard(n, "neighbor"))}
              {_isEmpty(display.downstreams.collapsed) ? null : this.renderCollapsedNeighbors(display.downstreams.collapsed)}
            </Grid>
          </Grid>
--- a/web/app/js/components/ResourceDetail.jsx
+++ b/web/app/js/components/ResourceDetail.jsx
@ -67,6 +67,7 @@ export class ResourceDetailBase extends React.Component {
      resourceType: resource.type,
      resource,
      lastMetricReceivedTime: Date.now(),
+      isTcpOnly: false, // whether this resource only has TCP traffic
      pollingInterval: 2000,
      resourceMetrics: [],
      podMetrics: [], // metrics for all pods whose owner is this resource
@ -125,7 +126,7 @@ export class ResourceDetailBase extends React.Component {
    this.api.setCurrentRequests([
      // inbound stats for this resource
      this.api.fetchMetrics(
-        `${this.api.urlsForResource(resource.type, resource.namespace)}&resource_name=${resource.name}`
+        `${this.api.urlsForResource(resource.type, resource.namespace, true)}&resource_name=${resource.name}`
      ),
      // list of all pods in this namespace (hack since we can't currently query for all pods in a resource)
      this.api.fetchPods(resource.namespace),
@ -181,8 +182,23 @@ export class ResourceDetailBase extends React.Component {
          resourceIsMeshed = _get(this.state.resourceMetrics, '[0].pods.meshedPods') > 0;
        }

+        let hasHttp = false;
+        let hasTcp = false;
+        let metric = resourceMetrics[0];
+        if (!_isEmpty(metric)) {
+          hasHttp = !_isEmpty(metric.stats) && metric.totalRequests !== 0;
+
+          if (!_isEmpty(metric.tcp)) {
+            let { tcp } = metric;
+            hasTcp = tcp.openConnections > 0 || tcp.readBytes > 0 || tcp.writeBytes > 0;
+          }
+        }
+
+        let isTcpOnly = !hasHttp && hasTcp;
+
+        // figure out when the last traffic this resource received was so we can show a no traffic message
        let lastMetricReceivedTime = this.state.lastMetricReceivedTime;
-        if (!_isEmpty(resourceMetrics[0]) && resourceMetrics[0].totalRequests !== 0) {
+        if (hasHttp || hasTcp) {
          lastMetricReceivedTime = Date.now();
        }

@ -193,6 +209,7 @@ export class ResourceDetailBase extends React.Component {
          upstreamMetrics,
          downstreamMetrics,
          lastMetricReceivedTime,
+          isTcpOnly,
          loaded: true,
          pendingRequests: false,
          error: null,
@ -238,7 +255,8 @@ export class ResourceDetailBase extends React.Component {
      resourceMetrics,
      unmeshedSources,
      resourceIsMeshed,
-      lastMetricReceivedTime
+      lastMetricReceivedTime,
+      isTcpOnly,
    } = this.state;

    let query = {
@ -295,50 +313,40 @@ export class ResourceDetailBase extends React.Component {
          unmeshedSources={Object.values(unmeshedSources)}
          api={this.api} />

-        <TopRoutesTabs
+        {isTcpOnly ? null : <TopRoutesTabs
          query={query}
          pathPrefix={this.props.pathPrefix}
          updateUnmeshedSources={this.updateUnmeshedSources}
          disableTop={!resourceIsMeshed} />
-
-        {_isEmpty(upstreams) ? null : (
-          <React.Fragment>
-            <MetricsTable
-              resource="multi_resource"
-              title="Inbound"
-              metrics={upstreamMetrics} />
-          </React.Fragment>
-        )
        }

-        {_isEmpty(this.state.downstreamMetrics) ? null : (
-          <React.Fragment>
-            <MetricsTable
-              resource="multi_resource"
-              title="Outbound"
-              metrics={downstreamMetrics} />
-          </React.Fragment>
-        )
+        {_isEmpty(upstreams) ? null :
+        <MetricsTable
+          resource="multi_resource"
+          title="Inbound"
+          metrics={upstreamMetrics} />
+        }
+
+        {_isEmpty(this.state.downstreamMetrics) ? null :
+        <MetricsTable
+          resource="multi_resource"
+          title="Outbound"
+          metrics={downstreamMetrics} />
        }

        {
-          this.state.resource.type === "pod" ? null : (
-            <React.Fragment>
-              <MetricsTable
-                resource="pod"
-                title="Pods"
-                metrics={this.state.podMetrics} />
-            </React.Fragment>
-          )
-        }
-
-        <React.Fragment>
+          this.state.resource.type === "pod" || isTcpOnly ? null :
          <MetricsTable
            resource="pod"
-            title="TCP"
-            isTcpTable={true}
+            title="Pods"
            metrics={this.state.podMetrics} />
-        </React.Fragment>
+        }
+
+        <MetricsTable
+          resource="pod"
+          title="TCP"
+          isTcpTable={true}
+          metrics={this.state.podMetrics} />
      </div>
    );
  }
--- a/web/app/js/components/ResourceList.jsx
+++ b/web/app/js/components/ResourceList.jsx
@ -17,7 +17,7 @@ export class ResourceListBase extends React.Component {

  static propTypes = {
    data: PropTypes.arrayOf(metricsPropType.isRequired).isRequired,
-    error:  apiErrorPropType,
+    error: apiErrorPropType,
    loading: PropTypes.bool.isRequired,
    resource: PropTypes.string.isRequired,
  }
@ -48,7 +48,14 @@ export class ResourceListBase extends React.Component {
      <React.Fragment>
        <MetricsTable
          resource={this.props.resource}
-          metrics={processedMetrics} />
+          metrics={processedMetrics}
+          title="HTTP metrics" />
+
+        <MetricsTable
+          resource={this.props.resource}
+          isTcpTable={true}
+          metrics={processedMetrics}
+          title="TCP metrics" />
      </React.Fragment>
    );
  }
@ -68,7 +75,7 @@ export class ResourceListBase extends React.Component {

 export default withREST(
  ResourceListBase,
-  ({api, resource}) => [api.fetchMetrics(api.urlsForResource(resource))],
+  ({ api, resource }) => [api.fetchMetrics(api.urlsForResource(resource, '', true))],
  {
    resetProps: ['resource'],
  },
--- a/web/app/js/components/ResourceList.test.jsx
+++ b/web/app/js/components/ResourceList.test.jsx
@ -25,7 +25,7 @@ describe('Tests for <ResourceListBase>', () => {
    const err = component.find(ErrorBanner);
    expect(err).toHaveLength(1);
    expect(component.find(Spinner)).toHaveLength(0);
-    expect(component.find(MetricsTable)).toHaveLength(1);
+    expect(component.find(MetricsTable)).toHaveLength(2);
    expect(err.props().message.statusText).toEqual(msg);
  });

@ -52,7 +52,7 @@ describe('Tests for <ResourceListBase>', () => {

    expect(component.find(ErrorBanner)).toHaveLength(0);
    expect(component.find(Spinner)).toHaveLength(0);
-    expect(component.find(MetricsTable)).toHaveLength(1);
+    expect(component.find(MetricsTable)).toHaveLength(2);
  });

  it('renders a metrics table', () => {
@ -69,9 +69,11 @@ describe('Tests for <ResourceListBase>', () => {

    expect(component.find(ErrorBanner)).toHaveLength(0);
    expect(component.find(Spinner)).toHaveLength(0);
-    expect(metrics).toHaveLength(1);
+    expect(metrics).toHaveLength(2);

-    expect(metrics.props().resource).toEqual(resource);
-    expect(metrics.props().metrics).toHaveLength(1);
+    expect(metrics.at(0).props().resource).toEqual(resource);
+    expect(metrics.at(1).props().resource).toEqual(resource);
+    expect(metrics.at(0).props().metrics).toHaveLength(1);
+    expect(metrics.at(1).props().metrics).toHaveLength(1);
  });
 });