pentium100 commented on code in PR #4101:
URL: https://github.com/apache/hertzbeat/pull/4101#discussion_r3067256915
##########
hertzbeat-manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java:
##########
@@ -382,22 +375,31 @@ public void modifyMonitor(Monitor monitor, List<Param>
params, String collector,
labelDao.saveAll(addLabels);
}
+ boolean isStatic =
CommonConstants.SCRAPE_STATIC.equals(monitor.getScrape())
+ || !StringUtils.hasText(monitor.getScrape());
+ if (!isStatic && !StringUtils.hasText(monitor.getInstance())) {
+ monitor.setInstance("unknown");
+ }
+
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);
+ .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();
+ ? ""
+ : SignConstants.DOUBLE_MARK + portParam.getParamValue();
+ if (IpDomainUtil.isHasPortWithMark(instance)){
Review Comment:
The instance is not edited directly on the frontend; instead, the host is
assigned to the instance during submission, and the port parameter is appended
on the backend to form the final instance. There are two scenarios:
1. When the scrape type is "static," the frontend host is mandatory. The
frontend assigns the host to the instance, and the backend appends the port to
form the `host:port` format, which is then assigned to the instance.
2. When the scrape type is "sd," the frontend host is empty, resulting in an
empty instance. This is passed to the backend, but the modification logic does
not handle this case, leading to an error at the following line:
```java
Map<String, String> metadata = Map.of(CommonConstants.LABEL_INSTANCE_NAME,
monitor.getName(),
CommonConstants.LABEL_INSTANCE, monitor.getInstance());
```
Therefore, we need to add a check: if this is an "sd" type monitor, set the
instance to "unknown." Given the current situation, it is true that the
instance cannot contain a port. Are you suggesting that we should append the
port directly without checking if the instance already contains one?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]