Repository: pinpoint-apm/pinpoint
Issue: Add APIs for managing registered agents
Labels: enhancement

There is currently not a lot of ways to manage agent ids once they're mapped to an application. These data tend to linger around until the TTL has passed and ends up clogging the application index.
There needs to be APIs for doing 2 things:
1. Detect inactive agents (those that were registered, but are no longer in use) and provide a way to delete them (and their application name mapping)
2. Identify duplicate agent ids that are mapped to more than 1 application name.


Issue comments:
~~Reopening to add service type to request parameter.~~
Reopening to modify response for `/getInactiveAgents`.

Reopening to add API for removing all inactive agents from index.


Pull request review comments:

Patch that resolved the issue:
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/controller/AdminController.java b/web/src/main/java/com/navercorp/pinpoint/web/controller/AdminController.java
index 037b02b4882e..75f3e86475b1 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/controller/AdminController.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/controller/AdminController.java
@@ -71,6 +71,19 @@ public String removeAgentId(
         }
     }
 
+    @RequestMapping(value = "/removeInactiveAgents")
+    @ResponseBody
+    public String removeInactiveAgents(@RequestParam(value = "durationDays", defaultValue = "30") int durationDays) {
+        logger.info("removing inactive agents for the last {} days.", durationDays);
+        try {
+            this.adminService.removeInactiveAgents(durationDays);
+            return "OK";
+        } catch (Exception e) {
+            logger.error("error while removing inactive agents for the last " + durationDays + " days.", e);
+            return e.getMessage();
+        }
+    }
+
     @RequestMapping(value = "/agentIdMap")
     @ResponseBody
     public Map<String, List<Application>> agentIdMap() {
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/ApplicationIndexDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/ApplicationIndexDao.java
index 59810d1eea99..08e17e54c855 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/dao/ApplicationIndexDao.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/dao/ApplicationIndexDao.java
@@ -17,6 +17,7 @@
 package com.navercorp.pinpoint.web.dao;
 
 import java.util.List;
+import java.util.Map;
 
 import com.navercorp.pinpoint.web.vo.Application;
 
@@ -32,5 +33,7 @@ public interface ApplicationIndexDao {
 
     void deleteApplicationName(String applicationName);
 
+    void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap);
+
     void deleteAgentId(String applicationName, String agentId);
 }
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationIndexDao.java b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationIndexDao.java
index bc11abf47b0f..3de442c31d58 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationIndexDao.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/dao/hbase/HbaseApplicationIndexDao.java
@@ -18,7 +18,10 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.Get;
@@ -84,6 +87,34 @@ public void deleteApplicationName(String applicationName) {
         hbaseOperations2.delete(HBaseTables.APPLICATION_INDEX, delete);
     }
 
+    @Override
+    public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {
+        if (MapUtils.isEmpty(applicationAgentIdMap)) {
+            return;
+        }
+
+        List<Delete> deletes = new ArrayList<>(applicationAgentIdMap.size());
+
+        for (Map.Entry<String, List<String>> entry : applicationAgentIdMap.entrySet()) {
+            String applicationName = entry.getKey();
+            List<String> agentIds = entry.getValue();
+            if (StringUtils.isEmpty(applicationName) || CollectionUtils.isEmpty(agentIds)) {
+                continue;
+            }
+            Delete delete = new Delete(Bytes.toBytes(applicationName));
+            for (String agentId : agentIds) {
+                if (!StringUtils.isEmpty(agentId)) {
+                    delete.addColumns(HBaseTables.APPLICATION_INDEX_CF_AGENTS, Bytes.toBytes(agentId));
+                }
+            }
+            // don't delete if nothing has been specified except row
+            if (!delete.getFamilyCellMap().isEmpty()) {
+                deletes.add(delete);
+            }
+        }
+        hbaseOperations2.delete(HBaseTables.APPLICATION_INDEX, deletes);
+    }
+
     @Override
     public void deleteAgentId(String applicationName, String agentId) {
         if (StringUtils.isEmpty(applicationName)) {
@@ -95,7 +126,7 @@ public void deleteAgentId(String applicationName, String agentId) {
         byte[] rowKey = Bytes.toBytes(applicationName);
         Delete delete = new Delete(rowKey);
         byte[] qualifier = Bytes.toBytes(agentId);
-        delete.addColumn(HBaseTables.APPLICATION_INDEX_CF_AGENTS, qualifier);
+        delete.addColumns(HBaseTables.APPLICATION_INDEX_CF_AGENTS, qualifier);
         hbaseOperations2.delete(HBaseTables.APPLICATION_INDEX, delete);
     }
 }
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AdminService.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AdminService.java
index f2228ca3d9a1..734aa3ba68d6 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AdminService.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AdminService.java
@@ -31,6 +31,8 @@ public interface AdminService {
 
     void removeAgentId(String applicationName, String agentId);
 
+    void removeInactiveAgents(int durationDays);
+
     Map<String, List<Application>> getAgentIdMap();
 
     Map<String, List<Application>> getDuplicateAgentIdMap();
diff --git a/web/src/main/java/com/navercorp/pinpoint/web/service/AdminServiceImpl.java b/web/src/main/java/com/navercorp/pinpoint/web/service/AdminServiceImpl.java
index e4650b133953..a8bc5c2997d0 100644
--- a/web/src/main/java/com/navercorp/pinpoint/web/service/AdminServiceImpl.java
+++ b/web/src/main/java/com/navercorp/pinpoint/web/service/AdminServiceImpl.java
@@ -21,6 +21,8 @@
 import com.navercorp.pinpoint.web.vo.Application;
 import com.navercorp.pinpoint.web.vo.Range;
 import org.apache.commons.collections.CollectionUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
@@ -31,7 +33,9 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.TreeSet;
 
 /**
  * @author netspider
@@ -40,6 +44,10 @@
 @Service
 public class AdminServiceImpl implements AdminService {
 
+    private static final int MIN_DURATION_DAYS_FOR_INACTIVITY = 30;
+
+    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
     @Autowired
     private ApplicationIndexDao applicationIndexDao;
 
@@ -56,6 +64,33 @@ public void removeAgentId(String applicationName, String agentId) {
         applicationIndexDao.deleteAgentId(applicationName, agentId);
     }
 
+    @Override
+    public void removeInactiveAgents(int durationDays) {
+        if (durationDays < MIN_DURATION_DAYS_FOR_INACTIVITY) {
+            throw new IllegalArgumentException("duration may not be less than " + MIN_DURATION_DAYS_FOR_INACTIVITY + " days");
+        }
+        Map<String, List<String>> inactiveAgentMap = new TreeMap<>(Ordering.usingToString());
+
+        List<Application> applications = this.applicationIndexDao.selectAllApplicationNames();
+        Set<String> applicationNames = new TreeSet<>(Ordering.usingToString());
+        // remove duplicates (same application name but different service type)
+        for (Application application : applications) {
+            applicationNames.add(application.getName());
+        }
+        for (String applicationName : applicationNames) {
+            List<String> agentIds = this.applicationIndexDao.selectAgentIds(applicationName);
+            Collections.sort(agentIds);
+            List<String> inactiveAgentIds = filterInactiveAgents(agentIds, durationDays);
+            if (!CollectionUtils.isEmpty(inactiveAgentIds)) {
+                inactiveAgentMap.put(applicationName, inactiveAgentIds);
+            }
+        }
+        // map may become big, but realistically won't cause OOM
+        // if it becomes an issue, consider deleting inside the loop above
+        logger.info("deleting {}", inactiveAgentMap);
+        this.applicationIndexDao.deleteAgentIds(inactiveAgentMap);
+    }
+
     @Override
     public Map<String, List<Application>> getAgentIdMap() {
         Map<String, List<Application>> agentIdMap = new TreeMap<>(Ordering.usingToString());
@@ -91,34 +126,40 @@ public Map<String, List<Application>> getInactiveAgents(String applicationName,
         if (applicationName == null) {
             throw new NullPointerException("applicationName must not be null");
         }
-        if (durationDays < 30) {
-            throw new IllegalArgumentException("duration may not be less than 30 days");
-        }
-        if (durationDays > 180) {
-            throw new IllegalArgumentException("duration may not be greater than 180 days");
+        if (durationDays < MIN_DURATION_DAYS_FOR_INACTIVITY) {
+            throw new IllegalArgumentException("duration may not be less than " + MIN_DURATION_DAYS_FOR_INACTIVITY + " days");
         }
         List<String> agentIds = this.applicationIndexDao.selectAgentIds(applicationName);
         if (CollectionUtils.isEmpty(agentIds)) {
             return Collections.emptyMap();
         }
+        Map<String, List<Application>> agentIdMap = this.getAgentIdMap();
+        Map<String, List<Application>> inactiveAgentMap = new TreeMap<>(Ordering.usingToString());
+        List<String> inactiveAgentIds = filterInactiveAgents(agentIds, durationDays);
+        for (String inactiveAgentId : inactiveAgentIds) {
+            List<Application> applications = agentIdMap.get(inactiveAgentId);
+            inactiveAgentMap.put(inactiveAgentId, applications);
+        }
+        return inactiveAgentMap;
+    }
 
+    private List<String> filterInactiveAgents(List<String> agentIds, int durationDays) {
+        if (CollectionUtils.isEmpty(agentIds)) {
+            return Collections.emptyList();
+        }
+        List<String> inactiveAgentIds = new ArrayList<>();
         final long toTimestamp = System.currentTimeMillis();
         Calendar cal = Calendar.getInstance();
         cal.add(Calendar.DATE, durationDays * -1);
         final long fromTimestamp = cal.getTimeInMillis();
         Range queryRange = new Range(fromTimestamp, toTimestamp);
-
-        Map<String, List<Application>> agentIdMap = this.getAgentIdMap();
-
-        Map<String, List<Application>> inactiveAgentMap = new TreeMap<>(Ordering.usingToString());
         for (String agentId : agentIds) {
             boolean dataExists = this.agentStatDao.agentStatExists(agentId, queryRange);
             if (!dataExists) {
-                List<Application> applications = agentIdMap.get(agentId);
-                inactiveAgentMap.put(agentId, applications);
+                inactiveAgentIds.add(agentId);
             }
         }
-        return inactiveAgentMap;
+        return inactiveAgentIds;
     }
 
 }


Test patch:
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/alarm/AlarmPartitionerTest.java b/web/src/test/java/com/navercorp/pinpoint/web/alarm/AlarmPartitionerTest.java
index ed0f1e1fffd8..1621879c85f4 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/alarm/AlarmPartitionerTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/alarm/AlarmPartitionerTest.java
@@ -64,6 +64,10 @@ public List<String> selectAgentIds(String applicationName) {
             public void deleteApplicationName(String applicationName) {
             }
 
+            @Override
+            public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {
+            }
+
             @Override
             public void deleteAgentId(String applicationName, String agentId) {
             }
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/alarm/ReaderTest.java b/web/src/test/java/com/navercorp/pinpoint/web/alarm/ReaderTest.java
index 2bd1a807b987..1beb5bca60f1 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/alarm/ReaderTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/alarm/ReaderTest.java
@@ -117,6 +117,10 @@ public List<Application> selectAllApplicationNames() {
 
             @Override public List<String> selectAgentIds(String applicationName) {return null;}
             @Override public void deleteApplicationName(String applicationName) { }
+
+            @Override
+            public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {}
+
             @Override public void deleteAgentId(String applicationName, String agentId) {}
             
         };
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/GcCountCheckerTest.java b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/GcCountCheckerTest.java
index 50a5ca025d25..68c3c7bd46f4 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/GcCountCheckerTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/GcCountCheckerTest.java
@@ -18,6 +18,7 @@
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.BeforeClass;
 
@@ -34,9 +35,9 @@ public class GcCountCheckerTest {
     private static final String SERVICE_TYPE = "tomcat";
 
     private static ApplicationIndexDao applicationIndexDao;
-    
+
     private static AgentStatDao agentStatDao;
-    
+
     @BeforeClass
     public static void before() {
         agentStatDao = new AgentStatDao() {
@@ -59,7 +60,7 @@ public boolean agentStatExists(String agentId, Range range) {
                 return true;
             }
         };
-        
+
         applicationIndexDao = new ApplicationIndexDao() {
 
             @Override
@@ -74,7 +75,7 @@ public List<String> selectAgentIds(String applicationName) {
                     agentIds.add("local_tomcat");
                     return agentIds;
                 }
-                
+
                 throw new IllegalArgumentException();
             }
 
@@ -83,11 +84,16 @@ public void deleteApplicationName(String applicationName) {
                 throw new UnsupportedOperationException();
             }
 
+            @Override
+            public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {
+                throw new UnsupportedOperationException();
+            }
+
             @Override
             public void deleteAgentId(String applicationName, String agentId) {
                 throw new UnsupportedOperationException();
             }
-            
+
         };
     }
 
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/HeapUsageRateCheckerTest.java b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/HeapUsageRateCheckerTest.java
index a95897a3a3c5..a7546fd85b56 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/HeapUsageRateCheckerTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/HeapUsageRateCheckerTest.java
@@ -21,6 +21,7 @@
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -94,6 +95,11 @@ public void deleteApplicationName(String applicationName) {
                 throw new UnsupportedOperationException();
             }
 
+            @Override
+            public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {
+                throw new UnsupportedOperationException();
+            }
+
             @Override
             public void deleteAgentId(String applicationName, String agentId) {
                 throw new UnsupportedOperationException();
diff --git a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/JvmCpuUsageRateCheckerTest.java b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/JvmCpuUsageRateCheckerTest.java
index 2819bd30edd9..69e6e35340b8 100644
--- a/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/JvmCpuUsageRateCheckerTest.java
+++ b/web/src/test/java/com/navercorp/pinpoint/web/alarm/checker/JvmCpuUsageRateCheckerTest.java
@@ -20,6 +20,7 @@
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -91,6 +92,11 @@ public void deleteApplicationName(String applicationName) {
                 throw new UnsupportedOperationException();
             }
 
+            @Override
+            public void deleteAgentIds(Map<String, List<String>> applicationAgentIdMap) {
+                throw new UnsupportedOperationException();
+            }
+
             @Override
             public void deleteAgentId(String applicationName, String agentId) {
                 throw new UnsupportedOperationException();