Copilot commented on code in PR #3761:
URL: https://github.com/apache/hertzbeat/pull/3761#discussion_r2362324519
##########
hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/prometheus/parser/OnlineParser.java:
##########
@@ -81,6 +81,94 @@ public static Map<String, MetricFamily>
parseMetrics(InputStream inputStream) th
return metricFamilyMap;
}
+ public static Map<String, MetricFamily> parseMetrics(InputStream
inputStream, String metric) throws IOException {
+ Map<String, MetricFamily> metricFamilyMap = new
ConcurrentHashMap<>(10);
Review Comment:
This new public method lacks documentation. Consider adding a JavaDoc
comment explaining the purpose, parameters, return value, and how it differs
from the existing parseMetrics method.
```suggestion
/**
* Parses Prometheus metrics from the given {@link InputStream}, but
only for the specified metric name.
* <p>
* This method differs from {@link #parseMetrics(InputStream)} in that
it filters and parses only the metric
* with the given name, rather than parsing all available metrics from
the input stream.
*
* @param inputStream the input stream containing Prometheus metrics data
* @param metric the name of the metric to filter and parse
(case-insensitive)
* @return a map of metric family names to {@link MetricFamily} objects,
or {@code null} if parsing fails
* @throws IOException if an I/O error occurs while reading from the
input stream
*/
public static Map<String, MetricFamily> parseMetrics(InputStream
inputStream, String metric) throws IOException {
```
##########
hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/http/HttpCollectImpl.java:
##########
@@ -627,33 +627,35 @@ private void parseResponseByPromQl(String resp,
List<String> aliasFields, HttpPr
}
private void parseResponseByPrometheusExporter(InputStream content,
List<String> aliasFields, CollectRep.MetricsData.Builder builder) throws
IOException {
- Map<String, MetricFamily> metricFamilyMap =
OnlineParser.parseMetrics(content);
+ String metrics = builder.getMetrics();
+ Map<String, MetricFamily> metricFamilyMap =
OnlineParser.parseMetrics(content, metrics);
if (metricFamilyMap == null || metricFamilyMap.isEmpty()) {
return;
}
- String metrics = builder.getMetrics();
- if (metricFamilyMap.containsKey(metrics)) {
- MetricFamily metricFamily = metricFamilyMap.get(metrics);
- for (MetricFamily.Metric metric : metricFamily.getMetricList()) {
- Map<String, String> labelMap = metric.getLabels()
- .stream()
- .collect(Collectors.toMap(MetricFamily.Label::getName,
MetricFamily.Label::getValue));
- CollectRep.ValueRow.Builder valueRowBuilder =
CollectRep.ValueRow.newBuilder();
- for (String aliasField : aliasFields) {
- String columnValue = labelMap.get(aliasField);
- if (columnValue != null) {
- valueRowBuilder.addColumn(columnValue);
- } else if (CommonConstants.PROM_VALUE.equals(aliasField)
|| CommonConstants.PROM_METRIC_VALUE.equals(aliasField)) {
-
valueRowBuilder.addColumn(String.valueOf(metric.getValue()));
- } else {
- valueRowBuilder.addColumn(CommonConstants.NULL_VALUE);
- }
+ MetricFamily metricFamily = metricFamilyMap.get(metrics);
Review Comment:
[nitpick] Consider extracting the metrics name to a variable before the
parseMetrics call for better readability and to avoid the redundant
getMetrics() call that was moved up.
##########
hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/prometheus/parser/OnlineParser.java:
##########
@@ -81,6 +81,94 @@ public static Map<String, MetricFamily>
parseMetrics(InputStream inputStream) th
return metricFamilyMap;
}
+ public static Map<String, MetricFamily> parseMetrics(InputStream
inputStream, String metric) throws IOException {
+ Map<String, MetricFamily> metricFamilyMap = new
ConcurrentHashMap<>(10);
+ try {
+ int i = getChar(inputStream);
+ while (i != -1) {
+ if (i == '#' || i == '\n') {
+ skipToLineEnd(inputStream).maybeEol().maybeEof().noElse();
+ } else {
+ StringBuilder stringBuilder = new StringBuilder();
+ stringBuilder.append((char) i);
+
+ // parse the metricName to filter.
+ int next = parseMetricName(inputStream,
stringBuilder).maybeSpace().maybeLeftBracket().noElse();
+ String metricName = stringBuilder.toString();
+ stringBuilder.delete(0, stringBuilder.length());
+
+ // step2: Determine whether this metric should be parsed.
+ if (metric.equalsIgnoreCase(metricName)) {
+ parseMetricFromName(inputStream, stringBuilder,
metricFamilyMap, metricName, next);
+ } else {
+
skipToLineEnd(inputStream).maybeEol().maybeEof().noElse();
+ }
+ }
+ i = getChar(inputStream);
+ // To address the `\n\r` scenario, it is necessary to skip
+ if (i == '\r') {
+ i = getChar(inputStream);
+ }
+ }
+ } catch (FormatException e) {
+ log.error("prometheus parser failed because of wrong input format.
{}", e.getMessage());
+ return null;
+ }
+ return metricFamilyMap;
+ }
+
+ /**
+ * Start parsing the complete metric from the already parsed metric name.
Review Comment:
The method documentation is incomplete. It should include @param
descriptions for all parameters (inputStream, stringBuilder, metricFamilyMap,
metricName, next) and @return description for the CharChecker return type.
```suggestion
* Start parsing the complete metric from the already parsed metric name.
*
* @param inputStream the input stream containing the metric data
* @param stringBuilder the StringBuilder used for parsing and
temporary storage
* @param metricFamilyMap the map to store parsed MetricFamily objects,
keyed by metric name
* @param metricName the name of the metric to parse
* @param next the next character to process from the input
stream
* @return a CharChecker containing the next character after parsing the
metric
* @throws IOException if an I/O error occurs while reading the
input stream
* @throws FormatException if the input format is invalid
```
##########
hertzbeat-collector/hertzbeat-collector-basic/src/main/java/org/apache/hertzbeat/collector/collect/prometheus/parser/OnlineParser.java:
##########
@@ -81,6 +81,94 @@ public static Map<String, MetricFamily>
parseMetrics(InputStream inputStream) th
return metricFamilyMap;
}
+ public static Map<String, MetricFamily> parseMetrics(InputStream
inputStream, String metric) throws IOException {
+ Map<String, MetricFamily> metricFamilyMap = new
ConcurrentHashMap<>(10);
+ try {
+ int i = getChar(inputStream);
+ while (i != -1) {
+ if (i == '#' || i == '\n') {
+ skipToLineEnd(inputStream).maybeEol().maybeEof().noElse();
+ } else {
+ StringBuilder stringBuilder = new StringBuilder();
+ stringBuilder.append((char) i);
+
+ // parse the metricName to filter.
+ int next = parseMetricName(inputStream,
stringBuilder).maybeSpace().maybeLeftBracket().noElse();
+ String metricName = stringBuilder.toString();
+ stringBuilder.delete(0, stringBuilder.length());
+
+ // step2: Determine whether this metric should be parsed.
+ if (metric.equalsIgnoreCase(metricName)) {
Review Comment:
Using equalsIgnoreCase for metric name matching may not be appropriate for
Prometheus metrics, which are typically case-sensitive. Consider using equals()
instead to maintain strict metric name matching.
```suggestion
if (metric.equals(metricName)) {
```
--
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: notifications-unsubscr...@hertzbeat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@hertzbeat.apache.org
For additional commands, e-mail: notifications-h...@hertzbeat.apache.org
