fix(manager): set instance to 'unknow' for dynamic monitors on modification

hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java

@@ -28,7 +28,6 @@
 import org.apache.hertzbeat.alert.dao.AlertDefineBindDao;
 import org.apache.hertzbeat.collector.dispatch.DispatchConstants;
 import org.apache.hertzbeat.common.constants.CommonConstants;
-import org.apache.hertzbeat.common.constants.SignConstants;
 import org.apache.hertzbeat.common.entity.grafana.GrafanaDashboard;
 import org.apache.hertzbeat.common.entity.job.Configmap;
 import org.apache.hertzbeat.common.entity.job.Job;
@@ -138,12 +137,11 @@ public class MonitorServiceImpl implements MonitorService {
     @Autowired
     private MetricsFavoriteService metricsFavoriteService;
 
-    /**
-     * Idempotent: an instance already carrying a port is left untouched, so repeated
-     * edits cannot grow it (host:443:443...). A missing instance falls back to the
-     * host param - never concatenated onto null, which produced "null:443" identities.
-     */
-    private void resolveMonitorInstance(Monitor monitor, List<Param> params) {
+    private void resolveMonitorInstance(Monitor monitor, List<Param> params, boolean isStatic) {
+        if (!isStatic) {
+            monitor.setInstance("unknown");
+            return;
+        }
         String instance = monitor.getInstance();
         if (!StringUtils.hasText(instance)) {
             instance = params.stream()
@@ -157,13 +155,38 @@ private void resolveMonitorInstance(Monitor monitor, List<Param> params) {
                 .filter(param -> PARAM_FIELD_PORT.equals(param.getField()))
                 .findFirst()
                 .orElse(null);
-        String portWithMark = (Objects.isNull(portParam) || !StringUtils.hasText(portParam.getParamValue()))
-                ? ""
-                : SignConstants.DOUBLE_MARK + portParam.getParamValue();
-        if (!IpDomainUtil.isHasPortWithMark(instance)) {
-            instance = instance + portWithMark;
+        String port = portParam == null ? null : portParam.getParamValue();
+        String host = removeExplicitPort(instance);
+        if (!StringUtils.hasText(port)) {
+            monitor.setInstance(host);
+        } else if (host.startsWith("[") && host.endsWith("]")) {
+            monitor.setInstance(host + ":" + port);
+        } else if (host.indexOf(':') != host.lastIndexOf(':')) {
+            monitor.setInstance("[" + host + "]:" + port);
+        } else {
+            monitor.setInstance(host + ":" + port);
         }
-        monitor.setInstance(instance);
+    }
+
+    private String removeExplicitPort(String instance) {
+        if (instance.startsWith("[")) {
+            int closingBracket = instance.indexOf(']');
+            if (closingBracket > 0
+                    && closingBracket + 1 < instance.length()
+                    && instance.charAt(closingBracket + 1) == ':'
+                    && IpDomainUtil.validPort(instance.substring(closingBracket + 2))) {
+                return instance.substring(0, closingBracket + 1);
+            }
+            return instance;
+        }
+        int firstColon = instance.indexOf(':');
+        int lastColon = instance.lastIndexOf(':');
+        if (firstColon > 0
+                && firstColon == lastColon
+                && IpDomainUtil.validPort(instance.substring(lastColon + 1))) {
+            return instance.substring(0, lastColon);
+        }
+        return instance;
     }
 
     @Override
@@ -200,7 +223,6 @@ public void addMonitor(Monitor monitor, List<Param> params, String collector, Gr
         Job appDefine = appService.getAppDefine(app);
         if (!isStatic) {
             appDefine.setSd(true);
-            monitor.setInstance("unknow");
         }
         if (CommonConstants.PROMETHEUS.equals(monitor.getApp())) {
             appDefine.setApp(CommonConstants.PROMETHEUS_APP_PREFIX + monitor.getName());
@@ -212,7 +234,7 @@ public void addMonitor(Monitor monitor, List<Param> params, String collector, Gr
         appDefine.setScheduleType(monitor.getScheduleType());
         appDefine.setCronExpression(monitor.getCronExpression());
 
-        resolveMonitorInstance(monitor, params);
+        resolveMonitorInstance(monitor, params, isStatic);
         String instance = monitor.getInstance();
 
         Map<String, String> metadata = Map.of(CommonConstants.LABEL_INSTANCE_NAME, monitor.getName(),
@@ -401,11 +423,11 @@ public void modifyMonitor(Monitor monitor, List<Param> params, String collector,
             labelDao.saveAll(addLabels);
         }
 
-        resolveMonitorInstance(monitor, params);
-        String instance = monitor.getInstance();
-
         boolean isStatic = CommonConstants.SCRAPE_STATIC.equals(monitor.getScrape())
                 || !StringUtils.hasText(monitor.getScrape());
+        resolveMonitorInstance(monitor, params, isStatic);
+        String instance = monitor.getInstance();
+
         if (preMonitor.getStatus() != CommonConstants.MONITOR_PAUSED_CODE) {
             // Construct the collection task Job entity
             String app = isStatic ? monitor.getApp() : monitor.getScrape();

hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java

@@ -678,7 +678,7 @@ void modifyMonitor() {
         } catch (IllegalArgumentException e) {
             assertEquals("Can not modify monitor's app type", e.getMessage());
         }
-        reset();
+        reset(monitorDao);
         Monitor existOkMonitor = Monitor.builder().jobId(1L).intervals(1).app("app").name("memory").instance("host")
                 .id(monitorId).build();
         when(monitorDao.findById(monitorId)).thenReturn(Optional.of(existOkMonitor));
@@ -688,6 +688,59 @@ void modifyMonitor() {
                 () -> monitorService.modifyMonitor(dto.getMonitor(), dto.getParams(), null, null));
     }
 
+    @Test
+    void modifyDynamicMonitorUsesUnknownInstance() {
+        Monitor monitor = modifyPausedMonitor("custom_sd", "stale-instance", null);
+
+        assertEquals("unknown", monitor.getInstance());
+    }
+
+    @ParameterizedTest
+    @CsvSource(nullValues = "NULL", value = {
+            "example.com, NULL, example.com",
+            "example.com, 443, example.com:443",
+            "example.com:80, NULL, example.com",
+            "example.com:80, 443, example.com:443",
+            "127.0.0.1:80, 443, 127.0.0.1:443",
+            "2001:db8::1, NULL, 2001:db8::1",
+            "2001:db8::1, 443, '[2001:db8::1]:443'",
+            "'[2001:db8::1]:80', NULL, '[2001:db8::1]'",
+            "'[2001:db8::1]:80', 443, '[2001:db8::1]:443'"
+    })
+    void modifyStaticMonitorNormalizesInstancePort(String instance, String port, String expected) {
+        Monitor monitor = modifyPausedMonitor(CommonConstants.SCRAPE_STATIC, instance, port);
+
+        assertEquals(expected, monitor.getInstance());
+    }
+
+    private Monitor modifyPausedMonitor(String scrape, String instance, String port) {
+        long monitorId = 99L;
+        Monitor existing = Monitor.builder()
+                .id(monitorId)
+                .jobId(1L)
+                .app("app")
+                .status(CommonConstants.MONITOR_PAUSED_CODE)
+                .build();
+        Monitor monitor = Monitor.builder()
+                .id(monitorId)
+                .app("app")
+                .name("memory")
+                .scrape(scrape)
+                .instance(instance)
+                .intervals(1)
+                .build();
+        List<Param> params = port == null
+                ? Collections.emptyList()
+                : List.of(Param.builder()
+                        .field(MonitorServiceImpl.PARAM_FIELD_PORT)
+                        .paramValue(port)
+                        .build());
+        when(monitorDao.findById(monitorId)).thenReturn(Optional.of(existing));
+
+        monitorService.modifyMonitor(monitor, params, null, null);
+        return monitor;
+    }
+
     @Test
     void testModifyMonitorPreservesLiveStatus() {
         long monitorId = 1L;