diff --git a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java index c3628fcb0d..2ed748bbc5 100644 --- a/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java +++ b/hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java @@ -28,7 +28,6 @@ import lombok.extern.slf4j.Slf4j; 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; @@ -143,12 +142,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 params) { + private void resolveMonitorInstance(Monitor monitor, List params, boolean isStatic) { + if (!isStatic) { + monitor.setInstance("unknown"); + return; + } String instance = monitor.getInstance(); if (!StringUtils.hasText(instance)) { instance = params.stream() @@ -162,13 +160,38 @@ public class MonitorServiceImpl implements MonitorService { .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 @@ -205,7 +228,6 @@ public class MonitorServiceImpl implements MonitorService { 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()); @@ -217,7 +239,7 @@ public class MonitorServiceImpl implements MonitorService { appDefine.setScheduleType(monitor.getScheduleType()); appDefine.setCronExpression(monitor.getCronExpression()); - resolveMonitorInstance(monitor, params); + resolveMonitorInstance(monitor, params, isStatic); String instance = monitor.getInstance(); Map metadata = Map.of(CommonConstants.LABEL_INSTANCE_NAME, monitor.getName(), @@ -470,11 +492,11 @@ public class MonitorServiceImpl implements MonitorService { 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(); diff --git a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java index 17e387cf4d..a1a48a49a2 100644 --- a/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java +++ b/hertzbeat-manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java @@ -918,7 +918,7 @@ class MonitorServiceTest { } 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)); @@ -928,6 +928,59 @@ class MonitorServiceTest { () -> 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 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;