Repository: pinpoint-apm/pinpoint
Issue: Remove container agents that are not relevant from Inspector View's agent list
Labels: module:web

#### Description
Since containers are meant to be regularly created and destroyed, new agent ids are usually set up to be created every time a container is created.
This will result in an ever-growing number of agents in Inspector View's agent list as containers are created and destroyed.

Showing only the agents that were running in the queried time period would be a way to keep the agent list relevant and easy to read.

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/batch/job/AgentCountReader.java b/web/src/main/java/com/navercorp/pinpoint/web/batch/job/AgentCountReader.java
index 173fd701c844..63ec81fe8bfe 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/batch/job/AgentCountReader.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/batch/job/AgentCountReader.java
@@ -42,7 +42,8 @@ public class AgentCountReader implements ItemReader<ApplicationAgentsList>, Step
 
     @Override
     public void beforeStep(StepExecution stepExecution) {
-        ApplicationAgentsList applicationAgentList = agentInfoService.getAllApplicationAgentsList();
+        long timestamp = System.currentTimeMillis();
+        ApplicationAgentsList applicationAgentList = agentInfoService.getAllApplicationAgentsList(ApplicationAgentsList.Filter.NONE, timestamp);
         queue.add(applicationAgentList);
     }
 
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/controller/AgentInfoController.java b/web/src/main/java/com/navercorp/pinpoint/web/controller/AgentInfoController.java
index 11ec10f200fe..56ae264db941 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/controller/AgentInfoController.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/controller/AgentInfoController.java
@@ -17,6 +17,7 @@
 package com.navercorp.pinpoint.web.controller;
 
 import com.navercorp.pinpoint.common.PinpointConstants;
+import com.navercorp.pinpoint.common.server.util.AgentLifeCycleState;
 import com.navercorp.pinpoint.common.util.IdValidateUtils;
 import com.navercorp.pinpoint.web.service.AgentEventService;
 import com.navercorp.pinpoint.web.service.AgentInfoService;
@@ -57,7 +58,8 @@ public class AgentInfoController {
     @RequestMapping(value = "/getAgentList", method = RequestMethod.GET, params = {"!application"})
     @ResponseBody
     public ApplicationAgentsList getAgentList() {
-        return this.agentInfoService.getAllApplicationAgentsList();
+        long timestamp = System.currentTimeMillis();
+        return getAgentList(timestamp);
     }
 
     @RequestMapping(value = "/getAgentList", method = RequestMethod.GET, params = {"!application", "from", "to"})
@@ -65,7 +67,8 @@ public ApplicationAgentsList getAgentList() {
     public ApplicationAgentsList getAgentList(
             @RequestParam("from") long from,
             @RequestParam("to") long to) {
-        return this.agentInfoService.getAllApplicationAgentsList(to);
+        long timestamp = to;
+        return this.agentInfoService.getAllApplicationAgentsList(ApplicationAgentsList.Filter.NONE, timestamp);
     }
 
     @PreAuthorize("hasPermission(#applicationName, 'application', 'inspector')")
@@ -73,14 +76,14 @@ public ApplicationAgentsList getAgentList(
     @ResponseBody
     public ApplicationAgentsList getAgentList(
             @RequestParam("timestamp") long timestamp) {
-        return this.agentInfoService.getAllApplicationAgentsList(timestamp);
+        return this.agentInfoService.getAllApplicationAgentsList(ApplicationAgentsList.Filter.NONE, timestamp);
     }
 
     @RequestMapping(value = "/getAgentList", method = RequestMethod.GET, params = {"application"})
     @ResponseBody
-    public ApplicationAgentsList getAgentList(
-            @RequestParam("application") String applicationName) {
-        return this.agentInfoService.getApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, applicationName);
+    public ApplicationAgentsList getAgentList(@RequestParam("application") String applicationName) {
+        long timestamp = System.currentTimeMillis();
+        return getAgentList(applicationName, timestamp);
     }
 
     @PreAuthorize("hasPermission(#applicationName, 'application', 'inspector')")
@@ -90,7 +93,17 @@ public ApplicationAgentsList getAgentList(
             @RequestParam("application") String applicationName,
             @RequestParam("from") long from,
             @RequestParam("to") long to) {
-        return this.agentInfoService.getApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, applicationName, to);
+        ApplicationAgentsList.Filter containerFilter = agentInfo -> {
+            if (agentInfo.isContainer()) {
+                AgentStatus agentStatus = agentInfo.getStatus();
+                if (agentStatus == null || agentStatus.getEventTimestamp() < from) {
+                    return ApplicationAgentsList.Filter.REJECT;
+                }
+            }
+            return ApplicationAgentsList.Filter.ACCEPT;
+        };
+        long timestamp = to;
+        return this.agentInfoService.getApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, containerFilter, applicationName, timestamp);
     }
 
     @PreAuthorize("hasPermission(#applicationName, 'application', 'inspector')")
@@ -99,7 +112,16 @@ public ApplicationAgentsList getAgentList(
     public ApplicationAgentsList getAgentList(
             @RequestParam("application") String applicationName,
             @RequestParam("timestamp") long timestamp) {
-        return this.agentInfoService.getApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, applicationName, timestamp);
+        ApplicationAgentsList.Filter runningContainerFilter = agentInfo -> {
+            if (agentInfo.isContainer()) {
+                AgentStatus agentStatus = agentInfo.getStatus();
+                if (agentStatus == null || agentStatus.getState() != AgentLifeCycleState.RUNNING) {
+                    return ApplicationAgentsList.Filter.REJECT;
+                }
+            }
+            return ApplicationAgentsList.Filter.ACCEPT;
+        };
+        return this.agentInfoService.getApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, runningContainerFilter, applicationName, timestamp);
     }
 
     @RequestMapping(value = "/getAgentInfo", method = RequestMethod.GET)
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
index d3f96c542189..78c42b4542fb 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoService.java
@@ -33,13 +33,9 @@
  */
 public interface AgentInfoService {
 
-    ApplicationAgentsList getAllApplicationAgentsList();
+    ApplicationAgentsList getAllApplicationAgentsList(ApplicationAgentsList.Filter filter, long timestamp);
 
-    ApplicationAgentsList getAllApplicationAgentsList(long timestamp);
-
-    ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy key, String applicationName);
-
-    ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy key, String applicationName, long timestamp);
+    ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy key, ApplicationAgentsList.Filter filter, String applicationName, long timestamp);
 
     ApplicationAgentHostList getApplicationAgentHostList(int offset, int limit);
 
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
index 636b2c87b725..e8dae1c1c31a 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AgentInfoServiceImpl.java
@@ -85,35 +85,25 @@ public class AgentInfoServiceImpl implements AgentInfoService {
     private AgentDownloadInfoDao agentDownloadInfoDao;
 
     @Override
-    public ApplicationAgentsList getAllApplicationAgentsList() {
-        return getAllApplicationAgentsList(System.currentTimeMillis());
-    }
-
-    @Override
-    public ApplicationAgentsList getAllApplicationAgentsList(long timestamp) {
+    public ApplicationAgentsList getAllApplicationAgentsList(ApplicationAgentsList.Filter filter, long timestamp) {
         ApplicationAgentsList.GroupBy groupBy = ApplicationAgentsList.GroupBy.APPLICATION_NAME;
-        ApplicationAgentsList applicationAgentList = new ApplicationAgentsList(groupBy);
+        ApplicationAgentsList applicationAgentList = new ApplicationAgentsList(groupBy, filter);
         List<Application> applications = applicationIndexDao.selectAllApplicationNames();
         for (Application application : applications) {
-            applicationAgentList.merge(getApplicationAgentsList(groupBy, application.getName(), timestamp));
+            applicationAgentList.merge(getApplicationAgentsList(groupBy, filter, application.getName(), timestamp));
         }
         return applicationAgentList;
     }
 
     @Override
-    public ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy groupBy, String applicationName) {
-        return getApplicationAgentsList(groupBy, applicationName, System.currentTimeMillis());
-    }
-
-    @Override
-    public ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy groupBy, String applicationName, long timestamp) {
+    public ApplicationAgentsList getApplicationAgentsList(ApplicationAgentsList.GroupBy groupBy, ApplicationAgentsList.Filter filter, String applicationName, long timestamp) {
         if (applicationName == null) {
             throw new NullPointerException("applicationName must not be null");
         }
         if (groupBy == null) {
             throw new NullPointerException("groupBy must not be null");
         }
-        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(groupBy);
+        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(groupBy, filter);
         Set<AgentInfo> agentInfos = getAgentsByApplicationName(applicationName, timestamp);
         if (agentInfos.isEmpty()) {
             logger.warn("agent list is empty for application:{}", applicationName);
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsList.java b/web/src/main/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsList.java
index c8245c7a3f05..62529b25c1bc 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsList.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsList.java
@@ -56,15 +56,29 @@ private interface GroupingKey<T extends GroupingKey<T>> extends Comparable<T> {
         String value();
     }
 
+    public interface Filter {
+
+        boolean ACCEPT = true;
+        boolean REJECT = false;
+
+        boolean filter(AgentInfo agentInfo);
+
+        Filter NONE = agentInfo -> ACCEPT;
+    }
+
     private final GroupBy groupBy;
-    private final SortedMap<GroupingKey, List<AgentInfo>> agentsMap;
+    private final Filter filter;
+    private final SortedMap<GroupingKey, List<AgentInfo>> agentsMap = new TreeMap<>();
 
-    public ApplicationAgentsList(GroupBy groupBy) {
+    public ApplicationAgentsList(GroupBy groupBy, Filter filter) {
         this.groupBy = Objects.requireNonNull(groupBy, "groupBy must not be null");
-        this.agentsMap = new TreeMap<>();
+        this.filter = Objects.requireNonNull(filter, "filter must not be null");
     }
 
     public void add(AgentInfo agentInfo) {
+        if (filter.filter(agentInfo) == Filter.REJECT) {
+            return;
+        }
         GroupingKey key = groupBy.extractKey(agentInfo);
         List<AgentInfo> agentInfos = agentsMap.computeIfAbsent(key, k -> new ArrayList<>());
         agentInfos.add(agentInfo);


Test patch:
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsListTest.java b/web/src/test/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsListTest.java
index 0f95c92c4e5c..db813b604f05 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsListTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/vo/ApplicationAgentsListTest.java
@@ -30,7 +30,7 @@ public class ApplicationAgentsListTest {
 
     @Test
     public void groupByApplicationName() {
-        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.APPLICATION_NAME);
+        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.APPLICATION_NAME, ApplicationAgentsList.Filter.NONE);
         AgentInfo app1Agent1 = createAgentInfo("APP_1", "app1-agent1", "Host11", true);
         AgentInfo app1Agent2 = createAgentInfo("APP_1", "app1-agent2", "Host12", false);
         AgentInfo app2Agent1 = createAgentInfo("APP_2", "app2-agent1", "Host21", false);
@@ -58,7 +58,7 @@ public void groupByApplicationName() {
 
     @Test
     public void groupByHostNameShouldHaveContainersLastAndGroupedSeparately() {
-        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME);
+        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, ApplicationAgentsList.Filter.NONE);
         AgentInfo host1Agent1 = createAgentInfo("APP_1", "host1-agent1", "Host1", false);
         AgentInfo host2Agent1 = createAgentInfo("APP_1", "host2-agent1", "Host2", false);
         AgentInfo containerAgent1 = createAgentInfo("APP_1", "container-agent1", "Host3", true);
@@ -97,9 +97,9 @@ public void mergeLists() {
         AgentInfo containerAgent2 = createAgentInfo("APP_1", "container-agent2", "Host4", true);
         List<AgentInfo> agentInfos = shuffleAgentInfos(containerAgent1, host1Agent1, host2Agent1, containerAgent2);
 
-        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME);
+        ApplicationAgentsList applicationAgentsList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, ApplicationAgentsList.Filter.NONE);
         applicationAgentsList.addAll(agentInfos.subList(0, agentInfos.size() / 2));
-        ApplicationAgentsList applicationAgentsListToMerge = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME);
+        ApplicationAgentsList applicationAgentsListToMerge = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, ApplicationAgentsList.Filter.NONE);
         applicationAgentsListToMerge.addAll(agentInfos.subList(agentInfos.size() / 2, agentInfos.size()));
 
         applicationAgentsList.merge(applicationAgentsListToMerge);
@@ -132,9 +132,9 @@ public void mergeListsGroupedDifferently() {
         AgentInfo agent1 = createAgentInfo("APP_1", "app1-agent1", "Host1", false);
         AgentInfo agent2 = createAgentInfo("APP_2", "app2-agent1", "Host2", false);
         AgentInfo agent3 = createAgentInfo("APP_2", "app2-agent2", "Host2", true);
-        ApplicationAgentsList groupedByHostnameList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME);
+        ApplicationAgentsList groupedByHostnameList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.HOST_NAME, ApplicationAgentsList.Filter.NONE);
         groupedByHostnameList.add(agent1);
-        ApplicationAgentsList groupedByApplicationNameList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.APPLICATION_NAME);
+        ApplicationAgentsList groupedByApplicationNameList = new ApplicationAgentsList(ApplicationAgentsList.GroupBy.APPLICATION_NAME, ApplicationAgentsList.Filter.NONE);
         groupedByApplicationNameList.add(agent2);
         groupedByApplicationNameList.add(agent3);