Fix anomalies display on focused APM service map (#65882)

The map anomlies rings display was working on the global map and on the focused service of a focused map, but not on the other services on a focused map.

This is because we were adding the anomlies to the list of services from the initial query, but not to the list of services derived from the connections data.

Make the transformation that add anomalies happen after the derived services nodes are added.

This is done in the function that was called `dedupeConnections`, but since it does much more than dedupe connections has been renamed to `transformServiceMapResponses`.

Also make the node types extend `cytoscape.NodeDataDefinition` in order to simplify the types in the transformation (we were adding `& { id: string }` in some places which this replaces.)

Fixes #65403.

Author: smith

x-pack/plugins/apm/common/service_map.ts

@@ -5,22 +5,23 @@
  */
 
 import { i18n } from '@kbn/i18n';
+import cytoscape from 'cytoscape';
 import { ILicense } from '../../licensing/public';
 import {
   AGENT_NAME,
   SERVICE_ENVIRONMENT,
   SERVICE_NAME,
+  SPAN_DESTINATION_SERVICE_RESOURCE,
   SPAN_SUBTYPE,
-  SPAN_TYPE,
-  SPAN_DESTINATION_SERVICE_RESOURCE
+  SPAN_TYPE
 } from './elasticsearch_fieldnames';
 
-export interface ServiceConnectionNode {
+export interface ServiceConnectionNode extends cytoscape.NodeDataDefinition {
   [SERVICE_NAME]: string;
   [SERVICE_ENVIRONMENT]: string | null;
   [AGENT_NAME]: string;
 }
-export interface ExternalConnectionNode {
+export interface ExternalConnectionNode extends cytoscape.NodeDataDefinition {
   [SPAN_DESTINATION_SERVICE_RESOURCE]: string;
   [SPAN_TYPE]: string;
   [SPAN_SUBTYPE]: string;

x-pack/plugins/apm/server/lib/service_map/get_service_map.ts

@@ -9,16 +9,15 @@ import {
   SERVICE_ENVIRONMENT,
   SERVICE_NAME
 } from '../../../common/elasticsearch_fieldnames';
+import { getMlIndex } from '../../../common/ml_job_constants';
 import { getServicesProjection } from '../../../common/projections/services';
 import { mergeProjection } from '../../../common/projections/util/merge_projection';
 import { PromiseReturnType } from '../../../typings/common';
+import { rangeFilter } from '../helpers/range_filter';
 import { Setup, SetupTimeRange } from '../helpers/setup_request';
-import { dedupeConnections } from './dedupe_connections';
+import { transformServiceMapResponses } from './transform_service_map_responses';
 import { getServiceMapFromTraceIds } from './get_service_map_from_trace_ids';
 import { getTraceSampleIds } from './get_trace_sample_ids';
-import { addAnomaliesToServicesData } from './ml_helpers';
-import { getMlIndex } from '../../../common/ml_job_constants';
-import { rangeFilter } from '../helpers/range_filter';
 
 export interface IEnvOptions {
   setup: Setup & SetupTimeRange;
@@ -179,13 +178,9 @@ export async function getServiceMap(options: IEnvOptions) {
     getAnomaliesData(options)
   ]);
 
-  const servicesDataWithAnomalies = addAnomaliesToServicesData(
-    servicesData,
-    anomaliesData
-  );
-
-  return dedupeConnections({
+  return transformServiceMapResponses({
     ...connectionData,
-    services: servicesDataWithAnomalies
+    anomalies: anomaliesData,
+    services: servicesData
   });
 }

x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts

@@ -5,11 +5,11 @@
  */
 
 import { AnomaliesResponse } from './get_service_map';
-import { addAnomaliesToServicesData } from './ml_helpers';
+import { addAnomaliesDataToNodes } from './ml_helpers';
 
-describe('addAnomaliesToServicesData', () => {
-  it('adds anomalies to services data', () => {
-    const servicesData = [
+describe('addAnomaliesDataToNodes', () => {
+  it('adds anomalies to nodes', () => {
+    const nodes = [
       {
         'service.name': 'opbeans-ruby',
         'agent.name': 'ruby',
@@ -89,8 +89,8 @@ describe('addAnomaliesToServicesData', () => {
     ];
 
     expect(
-      addAnomaliesToServicesData(
-        servicesData,
+      addAnomaliesDataToNodes(
+        nodes,
         (anomaliesResponse as unknown) as AnomaliesResponse
       )
     ).toEqual(result);

x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts

@@ -9,10 +9,11 @@ import {
   getMlJobServiceName,
   getSeverity
 } from '../../../common/ml_job_constants';
-import { AnomaliesResponse, ServicesResponse } from './get_service_map';
+import { ConnectionNode } from '../../../common/service_map';
+import { AnomaliesResponse } from './get_service_map';
 
-export function addAnomaliesToServicesData(
-  servicesData: ServicesResponse,
+export function addAnomaliesDataToNodes(
+  nodes: ConnectionNode[],
   anomaliesResponse: AnomaliesResponse
 ) {
   const anomaliesMap = (
@@ -52,7 +53,7 @@ export function addAnomaliesToServicesData(
     };
   }, {});
 
-  const servicesDataWithAnomalies = servicesData.map(service => {
+  const servicesDataWithAnomalies = nodes.map(service => {
     const serviceAnomalies = anomaliesMap[service[SERVICE_NAME]];
     if (serviceAnomalies) {
       const maxScore = serviceAnomalies.max_score;

x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.test.ts renamed to x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.test.ts

@@ -4,16 +4,19 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { ServiceMapResponse } from './';
 import {
-  SPAN_DESTINATION_SERVICE_RESOURCE,
-  SERVICE_NAME,
-  SERVICE_ENVIRONMENT,
   AGENT_NAME,
-  SPAN_TYPE,
-  SPAN_SUBTYPE
-} from '../../../../common/elasticsearch_fieldnames';
-import { dedupeConnections } from './';
+  SERVICE_ENVIRONMENT,
+  SERVICE_NAME,
+  SPAN_DESTINATION_SERVICE_RESOURCE,
+  SPAN_SUBTYPE,
+  SPAN_TYPE
+} from '../../../common/elasticsearch_fieldnames';
+import { AnomaliesResponse } from './get_service_map';
+import {
+  transformServiceMapResponses,
+  ServiceMapResponse
+} from './transform_service_map_responses';
 
 const nodejsService = {
   [SERVICE_NAME]: 'opbeans-node',
@@ -33,9 +36,14 @@ const javaService = {
   [AGENT_NAME]: 'java'
 };
 
-describe('dedupeConnections', () => {
+const anomalies = ({
+  aggregations: { jobs: { buckets: [] } }
+} as unknown) as AnomaliesResponse;
+
+describe('transformServiceMapResponses', () => {
   it('maps external destinations to internal services', () => {
     const response: ServiceMapResponse = {
+      anomalies,
       services: [nodejsService, javaService],
       discoveredServices: [
         {
@@ -51,7 +59,7 @@ describe('dedupeConnections', () => {
       ]
     };
 
-    const { elements } = dedupeConnections(response);
+    const { elements } = transformServiceMapResponses(response);
 
     const connection = elements.find(
       element => 'source' in element.data && 'target' in element.data
@@ -67,6 +75,7 @@ describe('dedupeConnections', () => {
 
   it('collapses external destinations based on span.destination.resource.name', () => {
     const response: ServiceMapResponse = {
+      anomalies,
       services: [nodejsService, javaService],
       discoveredServices: [
         {
@@ -89,7 +98,7 @@ describe('dedupeConnections', () => {
       ]
     };
 
-    const { elements } = dedupeConnections(response);
+    const { elements } = transformServiceMapResponses(response);
 
     const connections = elements.filter(element => 'source' in element.data);
 
@@ -102,6 +111,7 @@ describe('dedupeConnections', () => {
 
   it('picks the first span.type/subtype in an alphabetically sorted list', () => {
     const response: ServiceMapResponse = {
+      anomalies,
       services: [javaService],
       discoveredServices: [],
       connections: [
@@ -126,7 +136,7 @@ describe('dedupeConnections', () => {
       ]
     };
 
-    const { elements } = dedupeConnections(response);
+    const { elements } = transformServiceMapResponses(response);
 
     const nodes = elements.filter(element => !('source' in element.data));
 
@@ -140,6 +150,7 @@ describe('dedupeConnections', () => {
 
   it('processes connections without a matching "service" aggregation', () => {
     const response: ServiceMapResponse = {
+      anomalies,
       services: [javaService],
       discoveredServices: [],
       connections: [
@@ -150,7 +161,7 @@ describe('dedupeConnections', () => {
       ]
     };
 
-    const { elements } = dedupeConnections(response);
+    const { elements } = transformServiceMapResponses(response);
 
     expect(elements.length).toBe(3);
   });

x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.ts renamed to x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.ts

@@ -10,14 +10,19 @@ import {
   SPAN_DESTINATION_SERVICE_RESOURCE,
   SPAN_TYPE,
   SPAN_SUBTYPE
-} from '../../../../common/elasticsearch_fieldnames';
+} from '../../../common/elasticsearch_fieldnames';
 import {
   Connection,
   ConnectionNode,
   ServiceConnectionNode,
   ExternalConnectionNode
-} from '../../../../common/service_map';
-import { ConnectionsResponse, ServicesResponse } from '../get_service_map';
+} from '../../../common/service_map';
+import {
+  ConnectionsResponse,
+  ServicesResponse,
+  AnomaliesResponse
+} from './get_service_map';
+import { addAnomaliesDataToNodes } from './ml_helpers';
 
 function getConnectionNodeId(node: ConnectionNode): string {
   if ('span.destination.service.resource' in node) {
@@ -34,13 +39,16 @@ function getConnectionId(connection: Connection) {
 }
 
 export type ServiceMapResponse = ConnectionsResponse & {
+  anomalies: AnomaliesResponse;
   services: ServicesResponse;
 };
 
-export function dedupeConnections(response: ServiceMapResponse) {
-  const { discoveredServices, services, connections } = response;
+export function transformServiceMapResponses(response: ServiceMapResponse) {
+  const { anomalies, discoveredServices, services, connections } = response;
 
-  const allNodes = connections
+  // Derive the rest of the map nodes from the connections and add the services
+  // from the services data query
+  const allNodes: ConnectionNode[] = connections
     .flatMap(connection => [connection.source, connection.destination])
     .map(node => ({ ...node, id: getConnectionNodeId(node) }))
     .concat(
@@ -50,25 +58,21 @@ export function dedupeConnections(response: ServiceMapResponse) {
       }))
     );
 
-  const serviceNodes = allNodes.filter(node => SERVICE_NAME in node) as Array<
-    ServiceConnectionNode & {
-      id: string;
-    }
-  >;
+  // List of nodes that are services
+  const serviceNodes = allNodes.filter(
+    node => SERVICE_NAME in node
+  ) as ServiceConnectionNode[];
 
+  // List of nodes that are externals
   const externalNodes = allNodes.filter(
     node => SPAN_DESTINATION_SERVICE_RESOURCE in node
-  ) as Array<
-    ExternalConnectionNode & {
-      id: string;
-    }
-  >;
+  ) as ExternalConnectionNode[];
 
-  // 1. maps external nodes to internal services
-  // 2. collapses external nodes into one node based on span.destination.service.resource
-  // 3. picks the first available span.type/span.subtype in an alphabetically sorted list
+  // 1. Map external nodes to internal services
+  // 2. Collapse external nodes into one node based on span.destination.service.resource
+  // 3. Pick the first available span.type/span.subtype in an alphabetically sorted list
   const nodeMap = allNodes.reduce((map, node) => {
-    if (map[node.id]) {
+    if (!node.id || map[node.id]) {
       return map;
     }
 
@@ -119,14 +123,14 @@ export function dedupeConnections(response: ServiceMapResponse) {
           .sort()[0]
       }
     };
-  }, {} as Record<string, ConnectionNode & { id: string }>);
+  }, {} as Record<string, ConnectionNode>);
 
-  // maps destination.address to service.name if possible
+  // Map destination.address to service.name if possible
   function getConnectionNode(node: ConnectionNode) {
     return nodeMap[getConnectionNodeId(node)];
   }
 
-  // build connections with mapped nodes
+  // Build connections with mapped nodes
   const mappedConnections = connections
     .map(connection => {
       const sourceData = getConnectionNode(connection.source);
@@ -166,7 +170,7 @@ export function dedupeConnections(response: ServiceMapResponse) {
     {} as Record<string, ConnectionWithId>
   );
 
-  // instead of adding connections in two directions,
+  // Instead of adding connections in two directions,
   // we add a `bidirectional` flag to use in styling
   const dedupedConnections = (sortBy(
     Object.values(connectionsById),
@@ -192,10 +196,18 @@ export function dedupeConnections(response: ServiceMapResponse) {
     return prev.concat(connection);
   }, []);
 
+  // Add anomlies data
+  const dedupedNodesWithAnomliesData = addAnomaliesDataToNodes(
+    dedupedNodes,
+    anomalies
+  );
+
   // Put everything together in elements, with everything in the "data" property
-  const elements = [...dedupedConnections, ...dedupedNodes].map(element => ({
-    data: element
-  }));
+  const elements = [...dedupedConnections, ...dedupedNodesWithAnomliesData].map(
+    element => ({
+      data: element
+    })
+  );
 
   return { elements };
 }