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 63ed4fb68d..9b061731e4 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 @@ -138,6 +138,34 @@ 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) { + String instance = monitor.getInstance(); + if (!StringUtils.hasText(instance)) { + instance = params.stream() + .filter(param -> "host".equals(param.getField())) + .map(Param::getParamValue) + .filter(StringUtils::hasText) + .findFirst() + .orElse(StringUtils.hasText(monitor.getName()) ? monitor.getName() : "unknown"); + } + Param portParam = params.stream() + .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; + } + monitor.setInstance(instance); + } + @Override @Transactional(readOnly = true) public void detectMonitor(Monitor monitor, List params, String collector) throws MonitorDetectException { @@ -184,19 +212,8 @@ public class MonitorServiceImpl implements MonitorService { appDefine.setScheduleType(monitor.getScheduleType()); appDefine.setCronExpression(monitor.getCronExpression()); + resolveMonitorInstance(monitor, params); String instance = monitor.getInstance(); - // The port field may be null - Param portParam = params.stream() - .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; - } - monitor.setInstance(instance); Map metadata = Map.of(CommonConstants.LABEL_INSTANCE_NAME, monitor.getName(), CommonConstants.LABEL_INSTANCE, instance); @@ -384,19 +401,8 @@ public class MonitorServiceImpl implements MonitorService { labelDao.saveAll(addLabels); } + resolveMonitorInstance(monitor, params); String instance = monitor.getInstance(); - // The port field may be null - Param portParam = params.stream() - .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 (Objects.nonNull(instance)) { - instance = instance + portWithMark; - } - monitor.setInstance(instance); boolean isStatic = CommonConstants.SCRAPE_STATIC.equals(monitor.getScrape()) || !StringUtils.hasText(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 3f682f933d..6b3b5c502d 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 @@ -225,6 +225,59 @@ class MonitorServiceTest { assertDoesNotThrow(() -> monitorService.addMonitor(monitor, params, null, null)); } + @Test + void addMonitorWithoutInstanceFallsBackToHostParam() { + Monitor monitor = Monitor.builder() + .intervals(1) + .name("memory") + .app("demoApp") + .build(); + Job job = new Job(); + when(appService.getAppDefine(monitor.getApp())).thenReturn(job); + when(collectJobScheduling.addAsyncCollectJob(job, null)).thenReturn(1L); + when(monitorDao.save(monitor)).thenReturn(monitor); + List params = List.of( + Param.builder().field("host").paramValue("www.example.com").build(), + Param.builder().field("port").paramValue("443").build()); + when(paramDao.saveAll(params)).thenReturn(params); + assertDoesNotThrow(() -> monitorService.addMonitor(monitor, params, null, null)); + assertEquals("www.example.com:443", monitor.getInstance()); + } + + @Test + void addMonitorInstanceStaysStableAcrossRepeatedResolution() { + Monitor monitor = Monitor.builder() + .intervals(1) + .name("memory") + .app("demoApp") + .instance("www.example.com:443") + .build(); + Job job = new Job(); + when(appService.getAppDefine(monitor.getApp())).thenReturn(job); + when(collectJobScheduling.addAsyncCollectJob(job, null)).thenReturn(1L); + when(monitorDao.save(monitor)).thenReturn(monitor); + List params = List.of(Param.builder().field("port").paramValue("443").build()); + when(paramDao.saveAll(params)).thenReturn(params); + assertDoesNotThrow(() -> monitorService.addMonitor(monitor, params, null, null)); + assertEquals("www.example.com:443", monitor.getInstance()); + } + + @Test + void modifyMonitorKeepsInstanceStableAcrossEdits() { + long monitorId = 7L; + Monitor stored = Monitor.builder().jobId(1L).intervals(1).app("demoApp").name("ssl") + .instance("www.example.com:443").id(monitorId).build(); + when(monitorDao.findById(monitorId)).thenReturn(Optional.of(stored)); + List params = List.of(Param.builder().field("port").paramValue("443").build()); + + for (int edit = 0; edit < 2; edit++) { + Monitor dto = Monitor.builder().jobId(1L).intervals(1).app("demoApp").name("ssl") + .instance("www.example.com:443").id(monitorId).build(); + assertDoesNotThrow(() -> monitorService.modifyMonitor(dto, params, null, null)); + assertEquals("www.example.com:443", dto.getInstance()); + } + } + @Test void addMonitorException() { Monitor monitor = Monitor.builder()