[APM] Optimize service map query

x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx

@@ -13,7 +13,7 @@ import { getCytoscapeElements } from './get_cytoscape_elements';
 import serviceMapResponse from './cytoscape-layout-test-response.json';
 import { iconForNode } from './icons';
 
-const elementsFromResponses = getCytoscapeElements([serviceMapResponse], '');
+const elementsFromResponses = getCytoscapeElements(serviceMapResponse, '');
 
 storiesOf('app/ServiceMap/Cytoscape', module).add(
   'example',

x-pack/legacy/plugins/apm/public/components/app/ServiceMap/get_cytoscape_elements.ts

@@ -4,166 +4,63 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import { ValuesType } from 'utility-types';
-import { sortBy, isEqual } from 'lodash';
-import {
-  Connection,
-  ConnectionNode
-} from '../../../../../../../plugins/apm/common/service_map';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { ServiceMapAPIResponse } from '../../../../../../../plugins/apm/server/lib/service_map/get_service_map';
 import { getAPMHref } from '../../shared/Links/apm/APMLink';
 
-function getConnectionNodeId(node: ConnectionNode): string {
-  if ('destination.address' in node) {
-    // use a prefix to distinguish exernal destination ids from services
-    return `>${node['destination.address']}`;
-  }
-  return node['service.name'];
-}
-
-function getConnectionId(connection: Connection) {
-  return `${getConnectionNodeId(connection.source)}~${getConnectionNodeId(
-    connection.destination
-  )}`;
-}
 export function getCytoscapeElements(
-  responses: ServiceMapAPIResponse[],
+  response: ServiceMapAPIResponse,
   search: string
 ) {
-  const discoveredServices = responses.flatMap(
-    response => response.discoveredServices
-  );
-
-  const serviceNodes = responses
-    .flatMap(response => response.services)
-    .map(service => ({
-      ...service,
-      id: service['service.name']
-    }));
-
-  // maps destination.address to service.name if possible
-  function getConnectionNode(node: ConnectionNode) {
-    let mappedNode: ConnectionNode | undefined;
-
-    if ('destination.address' in node) {
-      mappedNode = discoveredServices.find(map => isEqual(map.from, node))?.to;
-    }
-
-    if (!mappedNode) {
-      mappedNode = node;
-    }
-
-    return {
-      ...mappedNode,
-      id: getConnectionNodeId(mappedNode)
-    };
-  }
-
-  // build connections with mapped nodes
-  const connections = responses
-    .flatMap(response => response.connections)
-    .map(connection => {
-      const source = getConnectionNode(connection.source);
-      const destination = getConnectionNode(connection.destination);
-
-      return {
-        source,
-        destination,
-        id: getConnectionId({ source, destination })
-      };
-    })
-    .filter(connection => connection.source.id !== connection.destination.id);
-
-  const nodes = connections
-    .flatMap(connection => [connection.source, connection.destination])
-    .concat(serviceNodes);
-
-  type ConnectionWithId = ValuesType<typeof connections>;
-  type ConnectionNodeWithId = ValuesType<typeof nodes>;
-
-  const connectionsById = connections.reduce((connectionMap, connection) => {
-    return {
-      ...connectionMap,
-      [connection.id]: connection
-    };
-  }, {} as Record<string, ConnectionWithId>);
+  const { nodes, connections } = response;
 
   const nodesById = nodes.reduce((nodeMap, node) => {
     return {
       ...nodeMap,
       [node.id]: node
     };
-  }, {} as Record<string, ConnectionNodeWithId>);
-
-  const cyNodes = (Object.values(nodesById) as ConnectionNodeWithId[]).map(
-    node => {
-      let data = {};
-
-      if ('service.name' in node) {
-        data = {
-          href: getAPMHref(
-            `/services/${node['service.name']}/service-map`,
-            search
-          ),
-          agentName: node['agent.name'],
-          frameworkName: node['service.framework.name'],
-          type: 'service'
-        };
-      }
-
-      if ('span.type' in node) {
-        data = {
-          // For nodes with span.type "db", convert it to "database". Otherwise leave it as-is.
-          type: node['span.type'] === 'db' ? 'database' : node['span.type'],
-          // Externals should not have a subtype so make it undefined if the type is external.
-          subtype: node['span.type'] !== 'external' && node['span.subtype']
-        };
-      }
-
-      return {
-        group: 'nodes' as const,
-        data: {
-          id: node.id,
-          label:
-            'service.name' in node
-              ? node['service.name']
-              : node['destination.address'],
-          ...data
-        }
+  }, {} as Record<string, ValuesType<typeof nodes>>);
+
+  const cyNodes = (Object.values(nodesById) as Array<
+    ValuesType<typeof nodes>
+  >).map(node => {
+    let data = {};
+
+    if ('service.name' in node) {
+      data = {
+        href: getAPMHref(
+          `/services/${node['service.name']}/service-map`,
+          search
+        ),
+        agentName: node['agent.name'],
+        frameworkName: node['service.framework.name'],
+        type: 'service'
       };
     }
-  );
-
-  // instead of adding connections in two directions,
-  // we add a `bidirectional` flag to use in styling
-  // and hide the inverse edge when rendering
-  const dedupedConnections = (sortBy(
-    Object.values(connectionsById),
-    // make sure that order is stable
-    'id'
-  ) as ConnectionWithId[]).reduce<
-    Array<
-      ConnectionWithId & { bidirectional?: boolean; isInverseEdge?: boolean }
-    >
-  >((prev, connection) => {
-    const reversedConnection = prev.find(
-      c =>
-        c.destination.id === connection.source.id &&
-        c.source.id === connection.destination.id
-    );
 
-    if (reversedConnection) {
-      reversedConnection.bidirectional = true;
-      return prev.concat({
-        ...connection,
-        isInverseEdge: true
-      });
+    if ('span.type' in node) {
+      data = {
+        // For nodes with span.type "db", convert it to "database". Otherwise leave it as-is.
+        type: node['span.type'] === 'db' ? 'database' : node['span.type'],
+        // Externals should not have a subtype so make it undefined if the type is external.
+        subtype: node['span.type'] !== 'external' && node['span.subtype']
+      };
     }
 
-    return prev.concat(connection);
-  }, []);
+    return {
+      group: 'nodes' as const,
+      data: {
+        id: node.id,
+        label:
+          'service.name' in node
+            ? node['service.name']
+            : node['destination.address'],
+        ...data
+      }
+    };
+  });
 
-  const cyEdges = dedupedConnections.map(connection => {
+  const cyEdges = connections.map(connection => {
     return {
       group: 'edges' as const,
       classes: connection.isInverseEdge ? 'invisible' : undefined,

x-pack/legacy/plugins/apm/public/components/app/ServiceMap/index.tsx

@@ -4,26 +4,15 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { EuiBetaBadge } from '@elastic/eui';
 import theme from '@elastic/eui/dist/eui_theme_light.json';
 import { i18n } from '@kbn/i18n';
-import { ElementDefinition } from 'cytoscape';
-import { find, isEqual } from 'lodash';
-import React, {
-  useCallback,
-  useEffect,
-  useMemo,
-  useRef,
-  useState
-} from 'react';
-import { EuiBetaBadge } from '@elastic/eui';
+import React, { useMemo } from 'react';
 import styled from 'styled-components';
 import { isValidPlatinumLicense } from '../../../../../../../plugins/apm/common/service_map';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { ServiceMapAPIResponse } from '../../../../../../../plugins/apm/server/lib/service_map/get_service_map';
-import { useApmPluginContext } from '../../../hooks/useApmPluginContext';
 import { useDeepObjectIdentity } from '../../../hooks/useDeepObjectIdentity';
+import { useFetcher } from '../../../hooks/useFetcher';
 import { useLicense } from '../../../hooks/useLicense';
-import { useLoadingIndicator } from '../../../hooks/useLoadingIndicator';
 import { useLocation } from '../../../hooks/useLocation';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { callApmApi } from '../../../services/rest/createCallApmApi';
@@ -64,13 +53,11 @@ const BetaBadgeContainer = styled.div`
   top: ${theme.gutterTypes.gutterSmall};
   z-index: 1; /* The element containing the cytoscape canvas has z-index = 0. */
 `;
-const MAX_REQUESTS = 5;
 
 export function ServiceMap({ serviceName }: ServiceMapProps) {
   const license = useLicense();
   const { search } = useLocation();
   const { urlParams, uiFilters } = useUrlParams();
-  const { notifications } = useApmPluginContext().core;
   const params = useDeepObjectIdentity({
     start: urlParams.start,
     end: urlParams.end,
@@ -82,95 +69,28 @@ export function ServiceMap({ serviceName }: ServiceMapProps) {
     }
   });
 
-  const renderedElements = useRef<ElementDefinition[]>([]);
-
-  const [responses, setResponses] = useState<ServiceMapAPIResponse[]>([]);
-
-  const { setIsLoading } = useLoadingIndicator();
-
-  const [, _setUnusedState] = useState(false);
-
-  const elements = useMemo(() => getCytoscapeElements(responses, search), [
-    responses,
-    search
-  ]);
-
-  const forceUpdate = useCallback(() => _setUnusedState(value => !value), []);
-
-  const getNext = useCallback(
-    async (input: { reset?: boolean; after?: string | undefined }) => {
-      const { start, end, uiFilters: strippedUiFilters, ...query } = params;
-
-      if (input.reset) {
-        renderedElements.current = [];
-        setResponses([]);
-      }
-
-      if (start && end) {
-        setIsLoading(true);
-        try {
-          const data = await callApmApi({
-            pathname: '/api/apm/service-map',
-            params: {
-              query: {
-                ...query,
-                start,
-                end,
-                uiFilters: JSON.stringify(strippedUiFilters),
-                after: input.after
-              }
-            }
-          });
-          setResponses(resp => resp.concat(data));
-
-          const shouldGetNext =
-            responses.length + 1 < MAX_REQUESTS && data.after;
-
-          if (shouldGetNext) {
-            await getNext({ after: data.after });
-          } else {
-            setIsLoading(false);
+  const { data } = useFetcher(() => {
+    const { start, end } = params;
+    if (start && end) {
+      return callApmApi({
+        pathname: '/api/apm/service-map',
+        params: {
+          query: {
+            ...params,
+            start,
+            end,
+            uiFilters: JSON.stringify(params.uiFilters)
           }
-        } catch (error) {
-          setIsLoading(false);
-          notifications.toasts.addError(error, {
-            title: i18n.translate('xpack.apm.errorServiceMapData', {
-              defaultMessage: `Error loading service connections`
-            })
-          });
         }
-      }
-    },
-    [params, setIsLoading, responses.length, notifications.toasts]
-  );
-
-  useEffect(() => {
-    const loadServiceMaps = async () => {
-      await getNext({ reset: true });
-    };
-
-    loadServiceMaps();
-
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [params]);
-
-  useEffect(() => {
-    if (renderedElements.current.length === 0) {
-      renderedElements.current = elements;
-      return;
+      });
     }
+  }, [params]);
 
-    const newElements = elements.filter(element => {
-      return !find(renderedElements.current, el => isEqual(el, element));
-    });
-
-    if (newElements.length > 0 && renderedElements.current.length > 0) {
-      renderedElements.current = elements;
-      forceUpdate();
-    }
-  }, [elements, forceUpdate]);
+  const elements = useMemo(() => {
+    return data ? getCytoscapeElements(data as any, search) : [];
+  }, [data, search]);
 
-  const { ref: wrapperRef, width, height } = useRefDimensions();
+  const { ref, height, width } = useRefDimensions();
 
   if (!license) {
     return null;
@@ -179,10 +99,10 @@ export function ServiceMap({ serviceName }: ServiceMapProps) {
   return isValidPlatinumLicense(license) ? (
     <div
       style={{ height: height - parseInt(theme.gutterTypes.gutterLarge, 10) }}
-      ref={wrapperRef}
+      ref={ref}
     >
       <Cytoscape
-        elements={renderedElements.current}
+        elements={elements}
         serviceName={serviceName}
         height={height}
         width={width}