Skip to content

Commit 0219589

Browse files
smithsmith
committed
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.
1 parent 6e90ec9 commit 0219589

6 files changed

Lines changed: 85 additions & 65 deletions

File tree

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,23 @@
55
*/
66

77
import { i18n } from '@kbn/i18n';
8+
import cytoscape from 'cytoscape';
89
import { ILicense } from '../../licensing/public';
910
import {
1011
AGENT_NAME,
1112
SERVICE_ENVIRONMENT,
1213
SERVICE_NAME,
14+
SPAN_DESTINATION_SERVICE_RESOURCE,
1315
SPAN_SUBTYPE,
14-
SPAN_TYPE,
15-
SPAN_DESTINATION_SERVICE_RESOURCE
16+
SPAN_TYPE
1617
} from './elasticsearch_fieldnames';
1718

18-
export interface ServiceConnectionNode {
19+
export interface ServiceConnectionNode extends cytoscape.NodeDataDefinition {
1920
[SERVICE_NAME]: string;
2021
[SERVICE_ENVIRONMENT]: string | null;
2122
[AGENT_NAME]: string;
2223
}
23-
export interface ExternalConnectionNode {
24+
export interface ExternalConnectionNode extends cytoscape.NodeDataDefinition {
2425
[SPAN_DESTINATION_SERVICE_RESOURCE]: string;
2526
[SPAN_TYPE]: string;
2627
[SPAN_SUBTYPE]: string;

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,15 @@ import {
99
SERVICE_ENVIRONMENT,
1010
SERVICE_NAME
1111
} from '../../../common/elasticsearch_fieldnames';
12+
import { getMlIndex } from '../../../common/ml_job_constants';
1213
import { getServicesProjection } from '../../../common/projections/services';
1314
import { mergeProjection } from '../../../common/projections/util/merge_projection';
1415
import { PromiseReturnType } from '../../../typings/common';
16+
import { rangeFilter } from '../helpers/range_filter';
1517
import { Setup, SetupTimeRange } from '../helpers/setup_request';
16-
import { dedupeConnections } from './dedupe_connections';
18+
import { transformServiceMapResponses } from './transform_service_map_responses';
1719
import { getServiceMapFromTraceIds } from './get_service_map_from_trace_ids';
1820
import { getTraceSampleIds } from './get_trace_sample_ids';
19-
import { addAnomaliesToServicesData } from './ml_helpers';
20-
import { getMlIndex } from '../../../common/ml_job_constants';
21-
import { rangeFilter } from '../helpers/range_filter';
2221

2322
export interface IEnvOptions {
2423
setup: Setup & SetupTimeRange;
@@ -179,13 +178,9 @@ export async function getServiceMap(options: IEnvOptions) {
179178
getAnomaliesData(options)
180179
]);
181180

182-
const servicesDataWithAnomalies = addAnomaliesToServicesData(
183-
servicesData,
184-
anomaliesData
185-
);
186-
187-
return dedupeConnections({
181+
return transformServiceMapResponses({
188182
...connectionData,
189-
services: servicesDataWithAnomalies
183+
anomalies: anomaliesData,
184+
services: servicesData
190185
});
191186
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
*/
66

77
import { AnomaliesResponse } from './get_service_map';
8-
import { addAnomaliesToServicesData } from './ml_helpers';
8+
import { addAnomaliesDataToNodes } from './ml_helpers';
99

10-
describe('addAnomaliesToServicesData', () => {
11-
it('adds anomalies to services data', () => {
12-
const servicesData = [
10+
describe('addAnomaliesDataToNodes', () => {
11+
it('adds anomalies to nodes', () => {
12+
const nodes = [
1313
{
1414
'service.name': 'opbeans-ruby',
1515
'agent.name': 'ruby',
@@ -89,8 +89,8 @@ describe('addAnomaliesToServicesData', () => {
8989
];
9090

9191
expect(
92-
addAnomaliesToServicesData(
93-
servicesData,
92+
addAnomaliesDataToNodes(
93+
nodes,
9494
(anomaliesResponse as unknown) as AnomaliesResponse
9595
)
9696
).toEqual(result);

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import {
99
getMlJobServiceName,
1010
getSeverity
1111
} from '../../../common/ml_job_constants';
12-
import { AnomaliesResponse, ServicesResponse } from './get_service_map';
12+
import { ConnectionNode } from '../../../common/service_map';
13+
import { AnomaliesResponse } from './get_service_map';
1314

14-
export function addAnomaliesToServicesData(
15-
servicesData: ServicesResponse,
15+
export function addAnomaliesDataToNodes(
16+
nodes: ConnectionNode[],
1617
anomaliesResponse: AnomaliesResponse
1718
) {
1819
const anomaliesMap = (
@@ -52,7 +53,7 @@ export function addAnomaliesToServicesData(
5253
};
5354
}, {});
5455

55-
const servicesDataWithAnomalies = servicesData.map(service => {
56+
const servicesDataWithAnomalies = nodes.map(service => {
5657
const serviceAnomalies = anomaliesMap[service[SERVICE_NAME]];
5758
if (serviceAnomalies) {
5859
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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,19 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { ServiceMapResponse } from './';
87
import {
9-
SPAN_DESTINATION_SERVICE_RESOURCE,
10-
SERVICE_NAME,
11-
SERVICE_ENVIRONMENT,
128
AGENT_NAME,
13-
SPAN_TYPE,
14-
SPAN_SUBTYPE
15-
} from '../../../../common/elasticsearch_fieldnames';
16-
import { dedupeConnections } from './';
9+
SERVICE_ENVIRONMENT,
10+
SERVICE_NAME,
11+
SPAN_DESTINATION_SERVICE_RESOURCE,
12+
SPAN_SUBTYPE,
13+
SPAN_TYPE
14+
} from '../../../common/elasticsearch_fieldnames';
15+
import { AnomaliesResponse } from './get_service_map';
16+
import {
17+
transformServiceMapResponses,
18+
ServiceMapResponse
19+
} from './transform_service_map_responses';
1720

1821
const nodejsService = {
1922
[SERVICE_NAME]: 'opbeans-node',
@@ -33,9 +36,14 @@ const javaService = {
3336
[AGENT_NAME]: 'java'
3437
};
3538

36-
describe('dedupeConnections', () => {
39+
const anomalies = ({
40+
aggregations: { jobs: { buckets: [] } }
41+
} as unknown) as AnomaliesResponse;
42+
43+
describe('transformServiceMapResponses', () => {
3744
it('maps external destinations to internal services', () => {
3845
const response: ServiceMapResponse = {
46+
anomalies,
3947
services: [nodejsService, javaService],
4048
discoveredServices: [
4149
{
@@ -51,7 +59,7 @@ describe('dedupeConnections', () => {
5159
]
5260
};
5361

54-
const { elements } = dedupeConnections(response);
62+
const { elements } = transformServiceMapResponses(response);
5563

5664
const connection = elements.find(
5765
element => 'source' in element.data && 'target' in element.data
@@ -67,6 +75,7 @@ describe('dedupeConnections', () => {
6775

6876
it('collapses external destinations based on span.destination.resource.name', () => {
6977
const response: ServiceMapResponse = {
78+
anomalies,
7079
services: [nodejsService, javaService],
7180
discoveredServices: [
7281
{
@@ -89,7 +98,7 @@ describe('dedupeConnections', () => {
8998
]
9099
};
91100

92-
const { elements } = dedupeConnections(response);
101+
const { elements } = transformServiceMapResponses(response);
93102

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

@@ -102,6 +111,7 @@ describe('dedupeConnections', () => {
102111

103112
it('picks the first span.type/subtype in an alphabetically sorted list', () => {
104113
const response: ServiceMapResponse = {
114+
anomalies,
105115
services: [javaService],
106116
discoveredServices: [],
107117
connections: [
@@ -126,7 +136,7 @@ describe('dedupeConnections', () => {
126136
]
127137
};
128138

129-
const { elements } = dedupeConnections(response);
139+
const { elements } = transformServiceMapResponses(response);
130140

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

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

141151
it('processes connections without a matching "service" aggregation', () => {
142152
const response: ServiceMapResponse = {
153+
anomalies,
143154
services: [javaService],
144155
discoveredServices: [],
145156
connections: [
@@ -150,7 +161,7 @@ describe('dedupeConnections', () => {
150161
]
151162
};
152163

153-
const { elements } = dedupeConnections(response);
164+
const { elements } = transformServiceMapResponses(response);
154165

155166
expect(elements.length).toBe(3);
156167
});

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

Lines changed: 39 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,19 @@ import {
1010
SPAN_DESTINATION_SERVICE_RESOURCE,
1111
SPAN_TYPE,
1212
SPAN_SUBTYPE
13-
} from '../../../../common/elasticsearch_fieldnames';
13+
} from '../../../common/elasticsearch_fieldnames';
1414
import {
1515
Connection,
1616
ConnectionNode,
1717
ServiceConnectionNode,
1818
ExternalConnectionNode
19-
} from '../../../../common/service_map';
20-
import { ConnectionsResponse, ServicesResponse } from '../get_service_map';
19+
} from '../../../common/service_map';
20+
import {
21+
ConnectionsResponse,
22+
ServicesResponse,
23+
AnomaliesResponse
24+
} from './get_service_map';
25+
import { addAnomaliesDataToNodes } from './ml_helpers';
2126

2227
function getConnectionNodeId(node: ConnectionNode): string {
2328
if ('span.destination.service.resource' in node) {
@@ -34,13 +39,16 @@ function getConnectionId(connection: Connection) {
3439
}
3540

3641
export type ServiceMapResponse = ConnectionsResponse & {
42+
anomalies: AnomaliesResponse;
3743
services: ServicesResponse;
3844
};
3945

40-
export function dedupeConnections(response: ServiceMapResponse) {
41-
const { discoveredServices, services, connections } = response;
46+
export function transformServiceMapResponses(response: ServiceMapResponse) {
47+
const { anomalies, discoveredServices, services, connections } = response;
4248

43-
const allNodes = connections
49+
// Derive the rest of the map nodes from the connections and add the services
50+
// from the services data query
51+
const allNodes: ConnectionNode[] = connections
4452
.flatMap(connection => [connection.source, connection.destination])
4553
.map(node => ({ ...node, id: getConnectionNodeId(node) }))
4654
.concat(
@@ -50,25 +58,21 @@ export function dedupeConnections(response: ServiceMapResponse) {
5058
}))
5159
);
5260

53-
const serviceNodes = allNodes.filter(node => SERVICE_NAME in node) as Array<
54-
ServiceConnectionNode & {
55-
id: string;
56-
}
57-
>;
61+
// List of nodes that are services
62+
const serviceNodes = allNodes.filter(
63+
node => SERVICE_NAME in node
64+
) as ServiceConnectionNode[];
5865

66+
// List of nodes that are externals
5967
const externalNodes = allNodes.filter(
6068
node => SPAN_DESTINATION_SERVICE_RESOURCE in node
61-
) as Array<
62-
ExternalConnectionNode & {
63-
id: string;
64-
}
65-
>;
69+
) as ExternalConnectionNode[];
6670

67-
// 1. maps external nodes to internal services
68-
// 2. collapses external nodes into one node based on span.destination.service.resource
69-
// 3. picks the first available span.type/span.subtype in an alphabetically sorted list
71+
// 1. Map external nodes to internal services
72+
// 2. Collapse external nodes into one node based on span.destination.service.resource
73+
// 3. Pick the first available span.type/span.subtype in an alphabetically sorted list
7074
const nodeMap = allNodes.reduce((map, node) => {
71-
if (map[node.id]) {
75+
if (!node.id || map[node.id]) {
7276
return map;
7377
}
7478

@@ -119,14 +123,14 @@ export function dedupeConnections(response: ServiceMapResponse) {
119123
.sort()[0]
120124
}
121125
};
122-
}, {} as Record<string, ConnectionNode & { id: string }>);
126+
}, {} as Record<string, ConnectionNode>);
123127

124-
// maps destination.address to service.name if possible
128+
// Map destination.address to service.name if possible
125129
function getConnectionNode(node: ConnectionNode) {
126130
return nodeMap[getConnectionNodeId(node)];
127131
}
128132

129-
// build connections with mapped nodes
133+
// Build connections with mapped nodes
130134
const mappedConnections = connections
131135
.map(connection => {
132136
const sourceData = getConnectionNode(connection.source);
@@ -166,7 +170,7 @@ export function dedupeConnections(response: ServiceMapResponse) {
166170
{} as Record<string, ConnectionWithId>
167171
);
168172

169-
// instead of adding connections in two directions,
173+
// Instead of adding connections in two directions,
170174
// we add a `bidirectional` flag to use in styling
171175
const dedupedConnections = (sortBy(
172176
Object.values(connectionsById),
@@ -192,10 +196,18 @@ export function dedupeConnections(response: ServiceMapResponse) {
192196
return prev.concat(connection);
193197
}, []);
194198

199+
// Add anomlies data
200+
const dedupedNodesWithAnomliesData = addAnomaliesDataToNodes(
201+
dedupedNodes,
202+
anomalies
203+
);
204+
195205
// Put everything together in elements, with everything in the "data" property
196-
const elements = [...dedupedConnections, ...dedupedNodes].map(element => ({
197-
data: element
198-
}));
206+
const elements = [...dedupedConnections, ...dedupedNodesWithAnomliesData].map(
207+
element => ({
208+
data: element
209+
})
210+
);
199211

200212
return { elements };
201213
}

0 commit comments

Comments
 (0)