Skip to content

Commit

Permalink
Fix anomalies display on focused APM service map (#65882)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@smith
smith authored May 8, 2020
1 parent 4d32610 commit 04f3736
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 65 deletions.
9 changes: 5 additions & 4 deletions x-pack/plugins/apm/common/service_map.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 6 additions & 11 deletions x-pack/plugins/apm/server/lib/service_map/get_service_map.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
});
}
12 changes: 6 additions & 6 deletions x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -89,8 +89,8 @@ describe('addAnomaliesToServicesData', () => {
];

expect(
addAnomaliesToServicesData(
servicesData,
addAnomaliesDataToNodes(
nodes,
(anomaliesResponse as unknown) as AnomaliesResponse
)
).toEqual(result);
Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 24 additions & 13 deletions ...vice_map/dedupe_connections/index.test.ts → ...p/transform_service_map_responses.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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: [
{
Expand All @@ -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
Expand All @@ -67,6 +75,7 @@ describe('dedupeConnections', () => {

it('collapses external destinations based on span.destination.resource.name', () => {
const response: ServiceMapResponse = {
anomalies,
services: [nodejsService, javaService],
discoveredServices: [
{
Expand All @@ -89,7 +98,7 @@ describe('dedupeConnections', () => {
]
};

const { elements } = dedupeConnections(response);
const { elements } = transformServiceMapResponses(response);

const connections = elements.filter(element => 'source' in element.data);

Expand All @@ -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: [
Expand All @@ -126,7 +136,7 @@ describe('dedupeConnections', () => {
]
};

const { elements } = dedupeConnections(response);
const { elements } = transformServiceMapResponses(response);

const nodes = elements.filter(element => !('source' in element.data));

Expand All @@ -140,6 +150,7 @@ describe('dedupeConnections', () => {

it('processes connections without a matching "service" aggregation', () => {
const response: ServiceMapResponse = {
anomalies,
services: [javaService],
discoveredServices: [],
connections: [
Expand All @@ -150,7 +161,7 @@ describe('dedupeConnections', () => {
]
};

const { elements } = dedupeConnections(response);
const { elements } = transformServiceMapResponses(response);

expect(elements.length).toBe(3);
});
Expand Down
66 changes: 39 additions & 27 deletions ...b/service_map/dedupe_connections/index.ts → ...ce_map/transform_service_map_responses.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand All @@ -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 };
}

0 comments on commit 04f3736

Please sign in to comment.