Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix anomalies display on focused APM service map #65882

Merged
merged 2 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions x-pack/plugins/apm/common/service_map.ts
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
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
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
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
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
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 };
}