Copilot commented on code in PR #4284:
URL: https://github.com/apache/solr/pull/4284#discussion_r3085886007
##########
solr/core/src/java/org/apache/solr/core/ZkContainer.java:
##########
@@ -176,44 +176,44 @@ public void initializeMetrics(
});
ctx.observableLongCounter(
- "solr_zk_read",
+ "solr.zk.read",
"Total bytes read from ZooKeeper",
measurement -> {
measurement.record(metricsListener.getBytesRead(),
attributes);
},
OtelUnit.BYTES);
ctx.observableLongCounter(
- "solr_zk_watches_fired",
+ "solr.zk.watches_fired",
"Total number of ZooKeeper watches fired",
measurement -> {
measurement.record(metricsListener.getWatchesFired(),
attributes);
});
ctx.observableLongCounter(
- "solr_zk_written",
+ "solr.zk.written",
"Total bytes written to ZooKeeper",
measurement -> {
measurement.record(metricsListener.getBytesWritten(),
attributes);
},
OtelUnit.BYTES);
ctx.observableLongCounter(
- "solr_zk_cumulative_multi_ops_total",
+ "solr.zk.cumulative.multi_ops.total",
"Total cumulative multi-operations count",
measurement -> {
measurement.record(metricsListener.getCumulativeMultiOps(), attributes);
});
ctx.observableLongCounter(
- "solr_zk_child_fetches",
+ "solr.zk.child.fetches",
"Total number of ZooKeeper child node fetches",
measurement -> {
measurement.record(metricsListener.getChildFetches(),
attributes);
});
Review Comment:
The metric name `solr.zk.child.fetches` introduces an extra hierarchy level
(`child` → `fetches`) even though the pre-migration name treated this as a
single compound concept (`child_fetches`). In this same block, other compound
leaves keep underscores (e.g. `watches_fired`, `children_fetched`,
`multi_ops`), so this one stands out and makes the naming inconsistent.
Consider using `solr.zk.child_fetches` instead to keep the compound token
intact (and keep the mapping closer to the previous name).
##########
solr/core/src/java/org/apache/solr/search/CaffeineCache.java:
##########
@@ -480,23 +480,23 @@ public void initializeMetrics(
ObservableLongMeasurement cacheLookupsMetric =
solrMetricsContext.longCounterMeasurement(
- metricName + "_lookups", "Number of cumulative cache lookup
results (hits and misses)");
+ metricName + ".lookups", "Number of cumulative cache lookup
results (hits and misses)");
ObservableLongMeasurement cacheOperationMetric =
solrMetricsContext.longCounterMeasurement(
- metricName + "_ops", "Number of cumulative cache operations
(inserts and evictions)");
+ metricName + ".ops", "Number of cumulative cache operations
(inserts and evictions)");
ObservableLongMeasurement sizeMetric =
solrMetricsContext.longGaugeMeasurement(
- metricName + "_size", "Current number cache entries");
+ metricName + ".size", "Current number cache entries");
Review Comment:
The description string for the cache size metric is grammatically incorrect:
"Current number cache entries" is missing "of". Please update it to a correct
phrase (e.g., "Current number of cache entries").
```suggestion
metricName + ".size", "Current number of cache entries");
```
--
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]
