Copilot commented on code in PR #3776:
URL: https://github.com/apache/hertzbeat/pull/3776#discussion_r2365160555
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/greptime/GreptimeDbDataStorage.java:
##########
@@ -193,60 +194,147 @@ public void saveData(CollectRep.MetricsData metricsData)
{
@Override
public Map<String, List<Value>> getHistoryMetricData(Long monitorId,
String app, String metrics, String metric,
String label, String
history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ return getHistoryData(start, end, step, monitorId, app, metrics,
metric);
+ }
+
+ private String getTableName(String metrics) {
+ return metrics;
+ }
+
+ @Override
+ public Map<String, List<Value>> getHistoryIntervalMetricData(Long
monitorId, String app, String metrics,
+ String
metric, String label, String history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ Map<String, List<Value>> instanceValuesMap = getHistoryData(start,
end, step, monitorId, app, metrics, metric);
+
+ // Queries below this point may yield inconsistent results due to
exceeding the valid data range.
+ // Therefore, we restrict the valid range by obtaining the post-query
timeframe.
+ // Since `gretime`'s `end` excludes the specified time, we add 4 hours.
+ List<Value> values =
instanceValuesMap.get(instanceValuesMap.keySet().stream().toList().get(0));
Review Comment:
This code assumes `instanceValuesMap` is not empty and will throw an
exception if it is. Add a null/empty check before accessing the first key.
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/greptime/GreptimeDbDataStorage.java:
##########
@@ -193,60 +194,147 @@ public void saveData(CollectRep.MetricsData metricsData)
{
@Override
public Map<String, List<Value>> getHistoryMetricData(Long monitorId,
String app, String metrics, String metric,
String label, String
history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ return getHistoryData(start, end, step, monitorId, app, metrics,
metric);
+ }
+
+ private String getTableName(String metrics) {
+ return metrics;
+ }
+
+ @Override
+ public Map<String, List<Value>> getHistoryIntervalMetricData(Long
monitorId, String app, String metrics,
+ String
metric, String label, String history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ Map<String, List<Value>> instanceValuesMap = getHistoryData(start,
end, step, monitorId, app, metrics, metric);
+
+ // Queries below this point may yield inconsistent results due to
exceeding the valid data range.
+ // Therefore, we restrict the valid range by obtaining the post-query
timeframe.
+ // Since `gretime`'s `end` excludes the specified time, we add 4 hours.
+ List<Value> values =
instanceValuesMap.get(instanceValuesMap.keySet().stream().toList().get(0));
+ // effective time
+ long effectiveStart = values.get(0).getTime() / 1000;
+ long effectiveEnd = values.get(values.size() - 1).getTime() / 1000 +
60 * 60 * 4;
Review Comment:
The magic number `60 * 60 * 4` (4 hours in seconds) should be extracted to a
named constant like `EFFECTIVE_END_BUFFER_SECONDS` for better readability and
maintainability.
##########
hertzbeat-warehouse/src/main/java/org/apache/hertzbeat/warehouse/store/history/tsdb/greptime/GreptimeDbDataStorage.java:
##########
@@ -193,60 +194,147 @@ public void saveData(CollectRep.MetricsData metricsData)
{
@Override
public Map<String, List<Value>> getHistoryMetricData(Long monitorId,
String app, String metrics, String metric,
String label, String
history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ return getHistoryData(start, end, step, monitorId, app, metrics,
metric);
+ }
+
+ private String getTableName(String metrics) {
+ return metrics;
+ }
+
+ @Override
+ public Map<String, List<Value>> getHistoryIntervalMetricData(Long
monitorId, String app, String metrics,
+ String
metric, String label, String history) {
+ Map<String, Long> timeRange = getTimeRange(history);
+ Long start = timeRange.get(LABEL_KEY_START_TIME);
+ Long end = timeRange.get(LABEL_KEY_END_TIME);
+
+ String step = getTimeStep(start, end);
+
+ Map<String, List<Value>> instanceValuesMap = getHistoryData(start,
end, step, monitorId, app, metrics, metric);
+
+ // Queries below this point may yield inconsistent results due to
exceeding the valid data range.
+ // Therefore, we restrict the valid range by obtaining the post-query
timeframe.
+ // Since `gretime`'s `end` excludes the specified time, we add 4 hours.
+ List<Value> values =
instanceValuesMap.get(instanceValuesMap.keySet().stream().toList().get(0));
+ // effective time
+ long effectiveStart = values.get(0).getTime() / 1000;
+ long effectiveEnd = values.get(values.size() - 1).getTime() / 1000 +
60 * 60 * 4;
+
+ String name = getTableName(metrics);
+ String timeSeriesSelector = name + "{" + LABEL_KEY_INSTANCE + "=\"" +
monitorId + "\"";
+ if (!CommonConstants.PROMETHEUS.equals(app)) {
+ timeSeriesSelector = timeSeriesSelector + "," + LABEL_KEY_FIELD +
"=\"" + metric + "\"}";
+ }
Review Comment:
The closing brace `}` is only added when the condition is true, but the
opening brace `{` is always added on line 230. This creates unbalanced braces
when the condition is false.
```suggestion
timeSeriesSelector = timeSeriesSelector + "," + LABEL_KEY_FIELD
+ "=\"" + metric + "\"";
}
timeSeriesSelector = timeSeriesSelector + "}";
```
--
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]
