DomGarguilo commented on code in PR #5112:
URL: https://github.com/apache/accumulo/pull/5112#discussion_r1859046386
##########
core/src/main/java/org/apache/accumulo/core/metrics/Metric.java:
##########
@@ -304,36 +308,49 @@ public String getDescription() {
return description;
}
- public MetricCategory getCategory() {
- return category;
+ public MetricDocSection getCategory() {
Review Comment:
What is the motivation behind changing this name? It seems like renaming it
to reference the docs ties it too closely to the document generation context,
potentially limiting its use in other parts of the code. This makes it seem
specific to documentation purposes, even though the Metric enum is used
elsewhere. It appears to mix responsibilities with the DocGen class.
##########
core/src/main/java/org/apache/accumulo/core/metrics/Metric.java:
##########
@@ -304,36 +308,49 @@ public String getDescription() {
return description;
}
- public MetricCategory getCategory() {
- return category;
+ public MetricDocSection getCategory() {
+ return section;
}
public enum MetricType {
GAUGE, COUNTER, TIMER, LONG_TASK_TIMER, DISTRIBUTION_SUMMARY,
FUNCTION_COUNTER, CACHE
}
- public enum MetricCategory {
- GENERAL_SERVER("General Server Metrics"),
- COMPACTION("Compaction Metrics"),
- COMPACTOR("Compactor Metrics"),
- FATE("Fate Metrics"),
- GARBAGE_COLLECTION("Garbage Collection Metrics"),
- TABLET_SERVER("Tablet Server Metrics"),
- SCAN("Scan Metrics"),
- SCAN_SERVER("Scan Server Metrics"),
- THRIFT("Thrift Metrics"),
- BLOCK_CACHE("Block Cache Metrics"),
- MANAGER("Manager Metrics");
+ public enum MetricDocSection {
+ GENERAL_SERVER("General Server Metrics", "Metrics that are generated
across all server types"),
Review Comment:
```suggestion
GENERAL_SERVER("General Server Metrics", "Metrics that are generated
across all server types."),
```
Should make sure that all of the sentences end with a period.
##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsDocGen.java:
##########
@@ -52,21 +52,27 @@ void pageHeader() {
doc.println("---\n");
doc.println("<!-- WARNING: Do not edit this file. It is a generated file"
+ " that is copied from Accumulo build (from
core/target/generated-docs) -->\n");
- doc.println("Below are the metrics used to monitor various components of
Accumulo.\n");
+ doc.println(
+ "Below are the metrics used to monitor various components of Accumulo.
Metrics emitted by Accumulo should"
+ + " contain the folowing tags: 'instance.name', 'resource.group',
'process.name', 'host' and 'port'. Metrics"
+ + " emitted by Accumulo may contain additional tags where we think
it makes sense to capture per-object metrics,"
+ + " for example on a table or tablet basis in the ScanServer and
TabletServer, or on a per-queue basis in the"
+ + " CompactionCoordinator.\n");
}
void generateTableOfContents() {
doc.println("## Table of Contents\n");
- for (var category : Metric.MetricCategory.values()) {
+ for (var category : Metric.MetricDocSection.values()) {
String sectionId = generateSectionId(category.getSectionTitle());
doc.println("- [" + category.getSectionTitle() + "](#" + sectionId +
")");
}
doc.println();
}
- void generateCategorySection(Metric.MetricCategory category, String
sectionTitle) {
+ void generateCategorySection(Metric.MetricDocSection category, String
sectionTitle,
Review Comment:
It could make sense to just accept the `Metric.MetricDocSection category`
as the only argument here and use it to get the title and description. That
would make for a bit cleaner code probably. Also, could update the name of
`category` in the method and var names since its changed to section.
--
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]
