addressing PR comment

elastic · Sep 29, 2020 · 54bb62a · 54bb62a
1 parent db3ab30
commit 54bb62a
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 27 deletions.
diff --git a/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts b/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.test.ts
@@ -17,14 +17,23 @@ function getConnectionsPairs(connections: Connection[]) {
         : conn.destination['span.type'];
       return `${source} -> ${destination}`;
     })
-    .filter((_) => _)
-    .sort();
+    .filter((_) => _);
 }
 
 describe('getConnections', () => {
   describe('with environments defined', () => {
     const paths = [
       [
+        {
+          'service.environment': 'testing',
+          'service.name': 'opbeans-ruby',
+          'agent.name': 'ruby',
+        },
+        {
+          'service.environment': null,
+          'service.name': 'opbeans-node',
+          'agent.name': 'nodejs',
+        },
         {
           'service.environment': 'production',
           'service.name': 'opbeans-go',
@@ -44,13 +53,13 @@ describe('getConnections', () => {
       [
         {
           'service.environment': 'testing',
-          'service.name': 'opbeans-python',
-          'agent.name': 'python',
+          'service.name': 'opbeans-ruby',
+          'agent.name': 'ruby',
         },
         {
           'service.environment': 'testing',
-          'service.name': 'opbeans-node',
-          'agent.name': 'nodejs',
+          'service.name': 'opbeans-python',
+          'agent.name': 'python',
         },
         {
           'span.subtype': 'http',
@@ -66,12 +75,15 @@ describe('getConnections', () => {
           serviceName: undefined,
           environment: undefined,
         });
+
         const connectionsPairs = getConnectionsPairs(connections);
         expect(connectionsPairs).toEqual([
+          'opbeans-ruby:testing -> opbeans-node:null',
+          'opbeans-node:null -> opbeans-go:production',
           'opbeans-go:production -> opbeans-java:production',
           'opbeans-java:production -> external',
-          'opbeans-node:testing -> external',
-          'opbeans-python:testing -> opbeans-node:testing',
+          'opbeans-ruby:testing -> opbeans-python:testing',
+          'opbeans-python:testing -> external',
         ]);
       });
     });
@@ -87,6 +99,8 @@ describe('getConnections', () => {
         const connectionsPairs = getConnectionsPairs(connections);
 
         expect(connectionsPairs).toEqual([
+          'opbeans-ruby:testing -> opbeans-node:null',
+          'opbeans-node:null -> opbeans-go:production',
           'opbeans-go:production -> opbeans-java:production',
           'opbeans-java:production -> external',
         ]);
@@ -102,8 +116,8 @@ describe('getConnections', () => {
         const connectionsPairs = getConnectionsPairs(connections);
 
         expect(connectionsPairs).toEqual([
-          'opbeans-node:testing -> external',
-          'opbeans-python:testing -> opbeans-node:testing',
+          'opbeans-ruby:testing -> opbeans-python:testing',
+          'opbeans-python:testing -> external',
         ]);
       });
     });
@@ -119,8 +133,10 @@ describe('getConnections', () => {
         const connectionsPairs = getConnectionsPairs(connections);
 
         expect(connectionsPairs).toEqual([
-          'opbeans-node:testing -> external',
-          'opbeans-python:testing -> opbeans-node:testing',
+          'opbeans-ruby:testing -> opbeans-node:null',
+          'opbeans-node:null -> opbeans-go:production',
+          'opbeans-go:production -> opbeans-java:production',
+          'opbeans-java:production -> external',
         ]);
       });
     });
@@ -136,8 +152,12 @@ describe('getConnections', () => {
         const connectionsPairs = getConnectionsPairs(connections);
 
         expect(connectionsPairs).toEqual([
-          'opbeans-node:testing -> external',
-          'opbeans-python:testing -> opbeans-node:testing',
+          'opbeans-ruby:testing -> opbeans-node:null',
+          'opbeans-node:null -> opbeans-go:production',
+          'opbeans-go:production -> opbeans-java:production',
+          'opbeans-java:production -> external',
+          'opbeans-ruby:testing -> opbeans-python:testing',
+          'opbeans-python:testing -> external',
         ]);
       });
       it('shows all connections for production environment', () => {
@@ -150,6 +170,8 @@ describe('getConnections', () => {
         const connectionsPairs = getConnectionsPairs(connections);
 
         expect(connectionsPairs).toEqual([
+          'opbeans-ruby:testing -> opbeans-node:null',
+          'opbeans-node:null -> opbeans-go:production',
           'opbeans-go:production -> opbeans-java:production',
           'opbeans-java:production -> external',
         ]);
@@ -160,6 +182,23 @@ describe('getConnections', () => {
   describe('environment is "not defined"', () => {
     it('shows all connections where environment is not set', () => {
       const environmentNotDefinedPaths = [
+        [
+          {
+            'service.environment': 'production',
+            'service.name': 'opbeans-go',
+            'agent.name': 'go',
+          },
+          {
+            'service.environment': 'production',
+            'service.name': 'opbeans-java',
+            'agent.name': 'java',
+          },
+          {
+            'span.subtype': 'http',
+            'span.destination.service.resource': '172.18.0.6:3000',
+            'span.type': 'external',
+          },
+        ],
         [
           {
             'service.environment': null,
@@ -205,8 +244,8 @@ describe('getConnections', () => {
       expect(connectionsPairs).toEqual([
         'opbeans-go:null -> opbeans-java:null',
         'opbeans-java:null -> external',
-        'opbeans-node:null -> external',
         'opbeans-python:null -> opbeans-node:null',
+        'opbeans-node:null -> external',
       ]);
     });
   });

diff --git a/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts b/x-pack/plugins/apm/server/lib/service_map/get_service_map_from_trace_ids.ts
@@ -28,21 +28,28 @@ export function getConnections({
 
   if (serviceName || environment) {
     paths = paths.filter((path) => {
-      return path.some((node) => {
-        if (serviceName && node[SERVICE_NAME] !== serviceName) {
-          return false;
-        }
+      return (
+        path
+          // Only apply the filter on node that contains service name, this filters out external nodes
+          .filter((node) => {
+            return node[SERVICE_NAME];
+          })
+          .some((node) => {
+            if (serviceName && node[SERVICE_NAME] !== serviceName) {
+              return false;
+            }
 
-        if (!environment) {
-          return true;
-        }
+            if (!environment) {
+              return true;
+            }
 
-        if (environment === ENVIRONMENT_NOT_DEFINED.value) {
-          return !node[SERVICE_ENVIRONMENT];
-        }
+            if (environment === ENVIRONMENT_NOT_DEFINED.value) {
+              return !node[SERVICE_ENVIRONMENT];
+            }
 
-        return node[SERVICE_ENVIRONMENT] === environment;
-      });
+            return node[SERVICE_ENVIRONMENT] === environment;
+          })
+      );
     });
   }