From 03836d3e3ec5f335eda335f093b8135977057868 Mon Sep 17 00:00:00 2001 From: Carlos Crespo Date: Fri, 13 Sep 2024 14:23:14 +0200 Subject: [PATCH 1/7] Attempt to fix service maps --- .../transform_service_map_responses.ts | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index 824ec0e7fcc1b..4a3d4ac36d9cf 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -51,7 +51,7 @@ function addMessagingConnections( const newConnections = messagingDestinations .map((node) => { const matchedService = discoveredServices.find( - ({ from }) => + ({ from, to }) => node[SPAN_DESTINATION_SERVICE_RESOURCE] === from[SPAN_DESTINATION_SERVICE_RESOURCE] )?.to; if (matchedService) { @@ -86,9 +86,25 @@ export function getAllNodes( return allNodes; } -export function getServiceNodes(allNodes: ConnectionNode[]) { +export function getServiceNodes( + allNodes: ConnectionNode[], + discoveredServices: Array<{ + from: ExternalConnectionNode; + to: ServiceConnectionNode; + }> +) { + const connectionFromDiscoveredServices = discoveredServices + .filter(({ from, to }) => { + return ( + allNodes.some((node) => node.id === getConnectionNodeId(from)) && + !allNodes.some((node) => node.id === to[SERVICE_NAME]) + ); + }) + .map(({ to }) => ({ ...to })); // List of nodes that are services - const serviceNodes = allNodes.filter((node) => SERVICE_NAME in node) as ServiceConnectionNode[]; + const serviceNodes = [...allNodes, ...connectionFromDiscoveredServices].filter( + (node) => SERVICE_NAME in node + ) as ServiceConnectionNode[]; return serviceNodes; } @@ -108,12 +124,16 @@ export function transformServiceMapResponses({ const { discoveredServices, services, connections, anomalies } = response; const allConnections = addMessagingConnections(connections, discoveredServices); const allNodes = getAllNodes(services, allConnections); - const serviceNodes = getServiceNodes(allNodes); + const serviceNodes = getServiceNodes(allNodes, discoveredServices); // List of nodes that are externals - const externalNodes = allNodes.filter( - (node) => SPAN_DESTINATION_SERVICE_RESOURCE in node - ) as ExternalConnectionNode[]; + const externalNodes = [ + ...new Set( + allNodes.filter( + (node) => SPAN_DESTINATION_SERVICE_RESOURCE in node + ) as ExternalConnectionNode[] + ), + ]; // 1. Map external nodes to internal services // 2. Collapse external nodes into one node based on span.destination.service.resource @@ -202,9 +222,11 @@ export function transformServiceMapResponses({ }) .filter((connection) => connection.source !== connection.target); - const nodes = mappedConnections - .flatMap((connection) => [connection.sourceData, connection.targetData]) - .concat(serviceNodes); + const nodes = mappedConnections.flatMap((connection) => [ + connection.sourceData, + connection.targetData, + ]); + // .concat(serviceNodes); const dedupedNodes: typeof nodes = []; From aa66780246e6e2de0d11a0020e76b2095cc4f258 Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Tue, 5 Nov 2024 17:29:51 +0000 Subject: [PATCH 2/7] delete not needed code --- .../server/routes/service_map/transform_service_map_responses.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index 4a3d4ac36d9cf..82908132eb5bf 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -226,7 +226,6 @@ export function transformServiceMapResponses({ connection.sourceData, connection.targetData, ]); - // .concat(serviceNodes); const dedupedNodes: typeof nodes = []; From f882bf0d6824426f89ac8833d4113498b4c8dd74 Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Wed, 6 Nov 2024 13:17:15 +0000 Subject: [PATCH 3/7] fix api snapshot test --- .../tests/service_maps/service_maps.spec.ts | 3 --- .../tests/service_maps/service_maps_kuery_filter.spec.ts | 1 - 2 files changed, 4 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts b/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts index 334b73d8db4cb..451996d3a38e8 100644 --- a/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts @@ -153,16 +153,13 @@ export default function serviceMapsApiTests({ getService }: FtrProviderContext) it('returns the correct data', () => { const elements: Array<{ data: Record }> = response.body.elements; - const serviceNames = uniq( elements .filter((element) => element.data['service.name'] !== undefined) .map((element) => element.data['service.name']) ).sort(); - expectSnapshot(serviceNames).toMatchInline(` Array [ - "auditbeat", "opbeans-dotnet", "opbeans-go", "opbeans-java", diff --git a/x-pack/test/apm_api_integration/tests/service_maps/service_maps_kuery_filter.spec.ts b/x-pack/test/apm_api_integration/tests/service_maps/service_maps_kuery_filter.spec.ts index 4be91cbe7bab6..b87e0de70495e 100644 --- a/x-pack/test/apm_api_integration/tests/service_maps/service_maps_kuery_filter.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_maps/service_maps_kuery_filter.spec.ts @@ -41,7 +41,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { } registry.when('Service Map', { config: 'trial', archives: [] }, () => { - // FLAKY: https://github.com/elastic/kibana/issues/176982 describe('optional kuery param', () => { before(async () => { const events = timerange(start, end) From 4c82ffe3c39920ca04ff7667419495e0333bc91f Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Wed, 6 Nov 2024 15:07:06 +0000 Subject: [PATCH 4/7] fix failing test --- .../routes/service_map/transform_service_map_responses.ts | 7 +++---- .../tests/service_maps/service_maps.spec.ts | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index 82908132eb5bf..d2906fa439eb3 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -222,10 +222,9 @@ export function transformServiceMapResponses({ }) .filter((connection) => connection.source !== connection.target); - const nodes = mappedConnections.flatMap((connection) => [ - connection.sourceData, - connection.targetData, - ]); + const nodes = mappedConnections + .flatMap((connection) => [connection.sourceData, connection.targetData]) + .concat(serviceNodes); const dedupedNodes: typeof nodes = []; diff --git a/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts b/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts index 451996d3a38e8..83965595020da 100644 --- a/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_maps/service_maps.spec.ts @@ -160,6 +160,7 @@ export default function serviceMapsApiTests({ getService }: FtrProviderContext) ).sort(); expectSnapshot(serviceNames).toMatchInline(` Array [ + "auditbeat", "opbeans-dotnet", "opbeans-go", "opbeans-java", From fea3a66a6e7e26c6a1f7a177d28e265fb157b6f9 Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Thu, 7 Nov 2024 10:04:49 +0000 Subject: [PATCH 5/7] increase timeout for flaky test, pr comments --- .../e2e/transaction_details/transaction_details.cy.ts | 4 ++-- .../service_map/transform_service_map_responses.ts | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/ftr_e2e/cypress/e2e/transaction_details/transaction_details.cy.ts b/x-pack/plugins/observability_solution/apm/ftr_e2e/cypress/e2e/transaction_details/transaction_details.cy.ts index 0fc1b609b14ba..1632e98eee78b 100644 --- a/x-pack/plugins/observability_solution/apm/ftr_e2e/cypress/e2e/transaction_details/transaction_details.cy.ts +++ b/x-pack/plugins/observability_solution/apm/ftr_e2e/cypress/e2e/transaction_details/transaction_details.cy.ts @@ -15,7 +15,7 @@ const timeRange = { rangeFrom: start, rangeTo: end, }; -// flaky + describe('Transaction details', () => { before(() => { synthtrace.index( @@ -114,7 +114,7 @@ describe('Transaction details', () => { })}` ); - cy.contains('Top 5 errors', { timeout: 30000 }); + cy.contains('Top 5 errors', { timeout: 60000 }); cy.getByTestSubj('topErrorsForTransactionTable').contains('a', '[MockError] Foo').click(); cy.url().should('include', 'opbeans-java/errors'); }); diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index d2906fa439eb3..582c5a74ca006 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -51,7 +51,7 @@ function addMessagingConnections( const newConnections = messagingDestinations .map((node) => { const matchedService = discoveredServices.find( - ({ from, to }) => + ({ from }) => node[SPAN_DESTINATION_SERVICE_RESOURCE] === from[SPAN_DESTINATION_SERVICE_RESOURCE] )?.to; if (matchedService) { @@ -127,13 +127,13 @@ export function transformServiceMapResponses({ const serviceNodes = getServiceNodes(allNodes, discoveredServices); // List of nodes that are externals - const externalNodes = [ - ...new Set( + const externalNodes = Array.from( + new Set( allNodes.filter( (node) => SPAN_DESTINATION_SERVICE_RESOURCE in node ) as ExternalConnectionNode[] - ), - ]; + ) + ); // 1. Map external nodes to internal services // 2. Collapse external nodes into one node based on span.destination.service.resource From 6bd3292e32da351f3d16604699c4b7e7128fea64 Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Fri, 8 Nov 2024 12:12:44 +0000 Subject: [PATCH 6/7] deduplicate per service.name instead of id --- .../routes/service_map/transform_service_map_responses.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index 582c5a74ca006..88ba8544d2efc 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -229,7 +229,7 @@ export function transformServiceMapResponses({ const dedupedNodes: typeof nodes = []; nodes.forEach((node) => { - if (!dedupedNodes.find((dedupedNode) => node.id === dedupedNode.id)) { + if (!dedupedNodes.find((dedupedNode) => node['service.name'] === dedupedNode['service.name'])) { dedupedNodes.push(node); } }); From 04353ce1017487ccea370ffa6a629ef9ae7152bf Mon Sep 17 00:00:00 2001 From: "miriam.aparicio" Date: Fri, 8 Nov 2024 16:06:29 +0000 Subject: [PATCH 7/7] add id to lsit of service nodes --- .../routes/service_map/transform_service_map_responses.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts index 88ba8544d2efc..c57a5bafb56d0 100644 --- a/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts +++ b/x-pack/plugins/observability_solution/apm/server/routes/service_map/transform_service_map_responses.ts @@ -100,7 +100,7 @@ export function getServiceNodes( !allNodes.some((node) => node.id === to[SERVICE_NAME]) ); }) - .map(({ to }) => ({ ...to })); + .map(({ to }) => ({ ...to, id: getConnectionNodeId(to) })); // List of nodes that are services const serviceNodes = [...allNodes, ...connectionFromDiscoveredServices].filter( (node) => SERVICE_NAME in node @@ -229,7 +229,7 @@ export function transformServiceMapResponses({ const dedupedNodes: typeof nodes = []; nodes.forEach((node) => { - if (!dedupedNodes.find((dedupedNode) => node['service.name'] === dedupedNode['service.name'])) { + if (!dedupedNodes.find((dedupedNode) => node.id === dedupedNode.id)) { dedupedNodes.push(node); } });