Show blank lines instead of N/A on service map popovers (elastic#57014)…

… (elastic#58569) * Show blank lines instead of N/A on service map popovers Because RUM agents never show CPU or memory usage, we don't want to always show the metric with N/A. If a metric is null, just hide the lines. Break up the display and fetching components and update the popover stories to show the list. Update some types. * Fix metric typings
smith · Feb 26, 2020 · b1f6cc2 · b1f6cc2
1 parent 1077bf9
commit b1f6cc2
Show file tree

Hide file tree

Showing 6 changed files with 147 additions and 116 deletions.
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Contents.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Contents.tsx
@@ -14,7 +14,7 @@ import cytoscape from 'cytoscape';
 import React from 'react';
 import { Buttons } from './Buttons';
 import { Info } from './Info';
-import { ServiceMetricList } from './ServiceMetricList';
+import { ServiceMetricFetcher } from './ServiceMetricFetcher';
 
 const popoverMinWidth = 280;
 
@@ -49,7 +49,7 @@ export function Contents({
       </EuiFlexItem>
       <EuiFlexItem>
         {isService ? (
-          <ServiceMetricList serviceName={selectedNodeServiceName} />
+          <ServiceMetricFetcher serviceName={selectedNodeServiceName} />
         ) : (
           <Info {...selectedNodeData} />
         )}

diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Popover.stories.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Popover.stories.tsx
@@ -6,44 +6,50 @@
 
 import { storiesOf } from '@storybook/react';
 import React from 'react';
-import {
-  ApmPluginContext,
-  ApmPluginContextValue
-} from '../../../../context/ApmPluginContext';
-import { Contents } from './Contents';
+import { ServiceMetricList } from './ServiceMetricList';
 
-const selectedNodeData = {
-  id: 'opbeans-node',
-  label: 'opbeans-node',
-  href:
-    '#/services/opbeans-node/service-map?rangeFrom=now-24h&rangeTo=now&refreshPaused=true&refreshInterval=0',
-  agentName: 'nodejs',
-  type: 'service'
-};
-
-storiesOf('app/ServiceMap/Popover/Contents', module).add(
-  'example',
-  () => {
-    return (
-      <ApmPluginContext.Provider
-        value={
-          ({ core: { notifications: {} } } as unknown) as ApmPluginContextValue
-        }
-      >
-        <Contents
-          selectedNodeData={selectedNodeData}
-          isService={true}
-          label="opbeans-node"
-          onFocusClick={() => {}}
-          selectedNodeServiceName="opbeans-node"
-        />
-      </ApmPluginContext.Provider>
-    );
-  },
-  {
-    info: {
-      propTablesExclude: [ApmPluginContext.Provider],
-      source: false
-    }
-  }
-);
+storiesOf('app/ServiceMap/Popover/ServiceMetricList', module)
+  .add('example', () => (
+    <ServiceMetricList
+      avgErrorsPerMinute={15.738888706725826}
+      avgTransactionDuration={61634.38905590272}
+      avgRequestsPerMinute={164.47222031860858}
+      avgCpuUsage={0.32809666568309237}
+      avgMemoryUsage={0.5504868173242986}
+      numInstances={2}
+      isLoading={false}
+    />
+  ))
+  .add('loading', () => (
+    <ServiceMetricList
+      avgErrorsPerMinute={null}
+      avgTransactionDuration={null}
+      avgRequestsPerMinute={null}
+      avgCpuUsage={null}
+      avgMemoryUsage={null}
+      numInstances={1}
+      isLoading={true}
+    />
+  ))
+  .add('some null values', () => (
+    <ServiceMetricList
+      avgErrorsPerMinute={7.615972134074397}
+      avgTransactionDuration={238792.54809512055}
+      avgRequestsPerMinute={8.439583235652972}
+      avgCpuUsage={null}
+      avgMemoryUsage={null}
+      numInstances={1}
+      isLoading={false}
+    />
+  ))
+  .add('all null values', () => (
+    <ServiceMetricList
+      avgErrorsPerMinute={null}
+      avgTransactionDuration={null}
+      avgRequestsPerMinute={null}
+      avgCpuUsage={null}
+      avgMemoryUsage={null}
+      numInstances={1}
+      isLoading={false}
+    />
+  ));
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/ServiceMetricFetcher.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/ServiceMetricFetcher.tsx
@@ -0,0 +1,41 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import React from 'react';
+import { ServiceNodeMetrics } from '../../../../../../../../plugins/apm/common/service_map';
+import { useFetcher } from '../../../../hooks/useFetcher';
+import { useUrlParams } from '../../../../hooks/useUrlParams';
+import { ServiceMetricList } from './ServiceMetricList';
+
+interface ServiceMetricFetcherProps {
+  serviceName: string;
+}
+
+export function ServiceMetricFetcher({
+  serviceName
+}: ServiceMetricFetcherProps) {
+  const {
+    urlParams: { start, end, environment }
+  } = useUrlParams();
+
+  const { data = {} as ServiceNodeMetrics, status } = useFetcher(
+    callApmApi => {
+      if (serviceName && start && end) {
+        return callApmApi({
+          pathname: '/api/apm/service-map/service/{serviceName}',
+          params: { path: { serviceName }, query: { start, end, environment } }
+        });
+      }
+    },
+    [serviceName, start, end, environment],
+    {
+      preservePreviousData: false
+    }
+  );
+  const isLoading = status === 'loading';
+
+  return <ServiceMetricList {...data} isLoading={isLoading} />;
+}
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/ServiceMetricList.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/ServiceMetricList.tsx
@@ -5,26 +5,23 @@
  */
 
 import {
+  EuiBadge,
   EuiFlexGroup,
-  EuiLoadingSpinner,
   EuiFlexItem,
-  EuiBadge
+  EuiLoadingSpinner
 } from '@elastic/eui';
 import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
 import { i18n } from '@kbn/i18n';
 import { isNumber } from 'lodash';
 import React from 'react';
 import styled from 'styled-components';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { ServiceNodeMetrics } from '../../../../../../../../plugins/apm/server/lib/service_map/get_service_map_service_node_info';
+import { ServiceNodeMetrics } from '../../../../../../../../plugins/apm/common/service_map';
 import {
   asDuration,
   asPercent,
   toMicroseconds,
   tpmUnit
 } from '../../../../utils/formatters';
-import { useUrlParams } from '../../../../hooks/useUrlParams';
-import { useFetcher } from '../../../../hooks/useFetcher';
 
 function LoadingSpinner() {
   return (
@@ -51,53 +48,19 @@ const ItemDescription = styled('td')`
   text-align: right;
 `;
 
-const na = i18n.translate('xpack.apm.serviceMap.NotAvailableMetric', {
-  defaultMessage: 'N/A'
-});
-
-interface MetricListProps {
-  serviceName: string;
+interface ServiceMetricListProps extends ServiceNodeMetrics {
+  isLoading: boolean;
 }
 
-export function ServiceMetricList({ serviceName }: MetricListProps) {
-  const {
-    urlParams: { start, end, environment }
-  } = useUrlParams();
-
-  const { data = {} as ServiceNodeMetrics, status } = useFetcher(
-    callApmApi => {
-      if (serviceName && start && end) {
-        return callApmApi({
-          pathname: '/api/apm/service-map/service/{serviceName}',
-          params: {
-            path: {
-              serviceName
-            },
-            query: {
-              start,
-              end,
-              environment
-            }
-          }
-        });
-      }
-    },
-    [serviceName, start, end, environment],
-    {
-      preservePreviousData: false
-    }
-  );
-
-  const {
-    avgTransactionDuration,
-    avgRequestsPerMinute,
-    avgErrorsPerMinute,
-    avgCpuUsage,
-    avgMemoryUsage,
-    numInstances
-  } = data;
-  const isLoading = status === 'loading';
-
+export function ServiceMetricList({
+  avgTransactionDuration,
+  avgRequestsPerMinute,
+  avgErrorsPerMinute,
+  avgCpuUsage,
+  avgMemoryUsage,
+  numInstances,
+  isLoading
+}: ServiceMetricListProps) {
   const listItems = [
     {
       title: i18n.translate(
@@ -108,7 +71,7 @@ export function ServiceMetricList({ serviceName }: MetricListProps) {
       ),
       description: isNumber(avgTransactionDuration)
         ? asDuration(toMicroseconds(avgTransactionDuration, 'milliseconds'))
-        : na
+        : null
     },
     {
       title: i18n.translate(
@@ -119,7 +82,7 @@ export function ServiceMetricList({ serviceName }: MetricListProps) {
       ),
       description: isNumber(avgRequestsPerMinute)
         ? `${avgRequestsPerMinute.toFixed(2)} ${tpmUnit('request')}`
-        : na
+        : null
     },
     {
       title: i18n.translate(
@@ -128,13 +91,13 @@ export function ServiceMetricList({ serviceName }: MetricListProps) {
           defaultMessage: 'Errors per minute (avg.)'
         }
       ),
-      description: avgErrorsPerMinute?.toFixed(2) ?? na
+      description: avgErrorsPerMinute?.toFixed(2)
     },
     {
       title: i18n.translate('xpack.apm.serviceMap.avgCpuUsagePopoverMetric', {
         defaultMessage: 'CPU usage (avg.)'
       }),
-      description: isNumber(avgCpuUsage) ? asPercent(avgCpuUsage, 1) : na
+      description: isNumber(avgCpuUsage) ? asPercent(avgCpuUsage, 1) : null
     },
     {
       title: i18n.translate(
@@ -143,7 +106,9 @@ export function ServiceMetricList({ serviceName }: MetricListProps) {
           defaultMessage: 'Memory usage (avg.)'
         }
       ),
-      description: isNumber(avgMemoryUsage) ? asPercent(avgMemoryUsage, 1) : na
+      description: isNumber(avgMemoryUsage)
+        ? asPercent(avgMemoryUsage, 1)
+        : null
     }
   ];
   return isLoading ? (
@@ -165,12 +130,15 @@ export function ServiceMetricList({ serviceName }: MetricListProps) {
 
       <table>
         <tbody>
-          {listItems.map(({ title, description }) => (
-            <ItemRow key={title}>
-              <ItemTitle>{title}</ItemTitle>
-              <ItemDescription>{description}</ItemDescription>
-            </ItemRow>
-          ))}
+          {listItems.map(
+            ({ title, description }) =>
+              description && (
+                <ItemRow key={title}>
+                  <ItemTitle>{title}</ItemTitle>
+                  <ItemDescription>{description}</ItemDescription>
+                </ItemRow>
+              )
+          )}
         </tbody>
       </table>
     </>

diff --git a/x-pack/plugins/apm/common/service_map.ts b/x-pack/plugins/apm/common/service_map.ts
@@ -21,3 +21,12 @@ export interface Connection {
   source: ConnectionNode;
   destination: ConnectionNode;
 }
+
+export interface ServiceNodeMetrics {
+  numInstances: number;
+  avgMemoryUsage: number | null;
+  avgCpuUsage: number | null;
+  avgTransactionDuration: number | null;
+  avgRequestsPerMinute: number | null;
+  avgErrorsPerMinute: number | null;
+}