Copilot commented on code in PR #3989:
URL: https://github.com/apache/hertzbeat/pull/3989#discussion_r2707270319
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -310,14 +309,11 @@ public Map<String, List<Value>>
getHistoryMetricData(String instance, String app
+ SignConstants.BLANK + encodedAuth);
}
HttpEntity<Void> httpEntity = new HttpEntity<>(headers);
- Instant end = Instant.now();
- Duration duration =
Duration.ofHours(Long.parseLong(history.replace("h", "")));
- Instant start = end.minus(duration);
- String exportUrl = vmClusterProps.select().url() +
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), EXPORT_PATH);
+ String exportUrl = vmClusterProps.select().url() +
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), EXPORT_PATH);
URI uri = UriComponentsBuilder.fromUriString(exportUrl)
- .queryParam("match", URLEncoder.encode("{" +
timeSeriesSelector + "}", StandardCharsets.UTF_8))
- .queryParam("start",
String.valueOf(start.getEpochSecond()))
- .queryParam("end", String.valueOf(end.getEpochSecond()))
+ .queryParam(URLEncoder.encode("match[]",
StandardCharsets.UTF_8), URLEncoder.encode("{" + timeSeriesSelector + "}",
StandardCharsets.UTF_8))
Review Comment:
The parameter name should not be URL-encoded when using
UriComponentsBuilder.queryParam(). This method already handles URL encoding
internally, so pre-encoding the parameter name "match[]" will result in
double-encoding. The parameter name should be passed as-is:
.queryParam("match[]", URLEncoder.encode("{" + timeSeriesSelector + "}",
StandardCharsets.UTF_8))
```suggestion
.queryParam("match[]", URLEncoder.encode("{" +
timeSeriesSelector + "}", StandardCharsets.UTF_8))
```
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -455,7 +451,7 @@ public Map<String, List<Value>>
getHistoryIntervalMetricData(String instance, St
}
// max
uri = UriComponentsBuilder.fromUriString(rangeUrl)
- .queryParam("query", URLEncoder.encode("max_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
+ .queryParam(URLEncoder.encode("query",
StandardCharsets.UTF_8), URLEncoder.encode("max_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
Review Comment:
The parameter name should not be URL-encoded when using
UriComponentsBuilder.queryParam(). This method already handles URL encoding
internally, so pre-encoding the parameter name "query" will result in
double-encoding. The parameter name should be passed as-is:
.queryParam("query", URLEncoder.encode("max_over_time({" + timeSeriesSelector +
"})", StandardCharsets.UTF_8))
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -310,14 +309,11 @@ public Map<String, List<Value>>
getHistoryMetricData(String instance, String app
+ SignConstants.BLANK + encodedAuth);
}
HttpEntity<Void> httpEntity = new HttpEntity<>(headers);
- Instant end = Instant.now();
- Duration duration =
Duration.ofHours(Long.parseLong(history.replace("h", "")));
- Instant start = end.minus(duration);
- String exportUrl = vmClusterProps.select().url() +
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), EXPORT_PATH);
+ String exportUrl = vmClusterProps.select().url() +
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), EXPORT_PATH);
URI uri = UriComponentsBuilder.fromUriString(exportUrl)
- .queryParam("match", URLEncoder.encode("{" +
timeSeriesSelector + "}", StandardCharsets.UTF_8))
- .queryParam("start",
String.valueOf(start.getEpochSecond()))
- .queryParam("end", String.valueOf(end.getEpochSecond()))
+ .queryParam(URLEncoder.encode("match[]",
StandardCharsets.UTF_8), URLEncoder.encode("{" + timeSeriesSelector + "}",
StandardCharsets.UTF_8))
+ .queryParam("start", URLEncoder.encode("now-" + history,
StandardCharsets.UTF_8))
Review Comment:
The value "now-" + history does not require URL encoding as it only contains
alphanumeric characters and a hyphen, which are URL-safe. The
URLEncoder.encode() call is unnecessary here and can be removed:
.queryParam("start", "now-" + history)
```suggestion
.queryParam("start", "now-" + history)
```
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -412,9 +408,9 @@ public Map<String, List<Value>>
getHistoryIntervalMetricData(String instance, St
+ SignConstants.BLANK + encodedAuth);
}
HttpEntity<Void> httpEntity = new HttpEntity<>(headers);
- String rangeUrl =
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), QUERY_RANGE_PATH);
+ String rangeUrl = vmClusterProps.select().url() +
VM_SELECT_BASE_PATH.formatted(vmClusterProps.accountID(), QUERY_RANGE_PATH);
URI uri = UriComponentsBuilder.fromUriString(rangeUrl)
- .queryParam("query", URLEncoder.encode("{" +
timeSeriesSelector + "}", StandardCharsets.UTF_8))
+ .queryParam(URLEncoder.encode("query",
StandardCharsets.UTF_8), URLEncoder.encode("{" + timeSeriesSelector + "}",
StandardCharsets.UTF_8))
Review Comment:
The parameter name should not be URL-encoded when using
UriComponentsBuilder.queryParam(). This method already handles URL encoding
internally, so pre-encoding the parameter name "query" will result in
double-encoding. The parameter name should be passed as-is:
.queryParam("query", URLEncoder.encode("{" + timeSeriesSelector + "}",
StandardCharsets.UTF_8))
```suggestion
.queryParam("query", URLEncoder.encode("{" +
timeSeriesSelector + "}", StandardCharsets.UTF_8))
```
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -533,7 +529,7 @@ public Map<String, List<Value>>
getHistoryIntervalMetricData(String instance, St
}
// avg
uri = UriComponentsBuilder.fromUriString(rangeUrl)
- .queryParam("query", URLEncoder.encode("avg_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
+ .queryParam(URLEncoder.encode("query",
StandardCharsets.UTF_8), URLEncoder.encode("avg_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
Review Comment:
The parameter name should not be URL-encoded when using
UriComponentsBuilder.queryParam(). This method already handles URL encoding
internally, so pre-encoding the parameter name "query" will result in
double-encoding. The parameter name should be passed as-is:
.queryParam("query", URLEncoder.encode("avg_over_time({" + timeSeriesSelector +
"})", StandardCharsets.UTF_8))
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/vm/VictoriaMetricsClusterDataStorage.java:
##########
@@ -494,7 +490,7 @@ public Map<String, List<Value>>
getHistoryIntervalMetricData(String instance, St
}
// min
uri = UriComponentsBuilder.fromUriString(rangeUrl)
- .queryParam("query", URLEncoder.encode("min_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
+ .queryParam(URLEncoder.encode("query",
StandardCharsets.UTF_8), URLEncoder.encode("min_over_time({" +
timeSeriesSelector + "})", StandardCharsets.UTF_8))
Review Comment:
The parameter name should not be URL-encoded when using
UriComponentsBuilder.queryParam(). This method already handles URL encoding
internally, so pre-encoding the parameter name "query" will result in
double-encoding. The parameter name should be passed as-is:
.queryParam("query", URLEncoder.encode("min_over_time({" + timeSeriesSelector +
"})", StandardCharsets.UTF_8))
--
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]
