ddanielr commented on code in PR #4069:
URL: https://github.com/apache/accumulo/pull/4069#discussion_r1427180590


##########
test/src/main/java/org/apache/accumulo/test/metrics/MetricsIT.java:
##########
@@ -126,14 +150,37 @@ public void confirmMetricsPublished() throws Exception {
             } else if (flakyMetrics.contains(name)) {
               // ignore any flaky metric names seen
               // these aren't always expected, but we shouldn't be surprised 
if we see them
+            } else if (name.startsWith("accumulo.compactor.queue")) {

Review Comment:
   It's out of scope of this PR, but we may want to add a defined metric prefix 
in the `MetricsProducer` for the compaction queue metrics 
   e.g.: `METRICS_COMPACTOR_PREFIX + "queue"` 
   
   This would allow us to directly link metric names to their uses in tests vs 
hardcoded references. 
   



##########
server/manager/src/main/java/org/apache/accumulo/manager/metrics/QueueMetrics.java:
##########
@@ -18,27 +18,89 @@
  */
 package org.apache.accumulo.manager.metrics;
 
-import static org.apache.accumulo.core.metrics.MetricsUtil.formatString;
 import static org.apache.accumulo.core.metrics.MetricsUtil.getCommonTags;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.accumulo.core.metrics.MetricsProducer;
 import org.apache.accumulo.core.spi.compaction.CompactionExecutorId;
 import org.apache.accumulo.core.util.threads.ThreadPools;
+import org.apache.accumulo.manager.compaction.queue.CompactionJobPriorityQueue;
 import org.apache.accumulo.manager.compaction.queue.CompactionJobQueues;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.Sets;
+import com.google.common.collect.Sets.SetView;
 
 import io.micrometer.core.instrument.Gauge;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tags;
 
 public class QueueMetrics implements MetricsProducer {
+
+  private static class QueueMeters {
+    private final Gauge length;
+    private final Gauge jobsQueued;
+    private final Gauge jobsDequeued;
+    private final Gauge jobsRejected;
+    private final Gauge jobsLowestPriority;
+
+    public QueueMeters(MeterRegistry meterRegistry, CompactionExecutorId 
queueId,
+        CompactionJobPriorityQueue queue) {
+      length =
+          Gauge.builder(METRICS_COMPACTOR_JOB_PRIORITY_QUEUE_LENGTH, queue, q 
-> q.getMaxSize())
+              .description("Length of priority queues")
+              .tags(Tags.concat(getCommonTags(), "queue.id", 
queueId.canonical()))
+              .register(meterRegistry);
+
+      jobsQueued = Gauge
+          .builder(METRICS_COMPACTOR_JOB_PRIORITY_QUEUE_JOBS_QUEUED, queue, q 
-> q.getQueuedJobs())
+          .description("Count of queued jobs")
+          .tags(Tags.concat(getCommonTags(), "queue.id", queueId.canonical()))
+          .register(meterRegistry);
+
+      jobsDequeued = Gauge
+          .builder(METRICS_COMPACTOR_JOB_PRIORITY_QUEUE_JOBS_DEQUEUED, queue,
+              q -> q.getDequeuedJobs())
+          .description("Count of jobs dequeued")
+          .tags(Tags.concat(getCommonTags(), "queue.id", queueId.canonical()))
+          .register(meterRegistry);
+
+      jobsRejected = Gauge
+          .builder(METRICS_COMPACTOR_JOB_PRIORITY_QUEUE_JOBS_REJECTED, queue,
+              q -> q.getRejectedJobs())
+          .description("Count of rejected jobs")
+          .tags(Tags.concat(getCommonTags(), "queue.id", queueId.canonical()))
+          .register(meterRegistry);
+
+      jobsLowestPriority = Gauge
+          .builder(METRICS_COMPACTOR_JOB_PRIORITY_QUEUE_JOBS_PRIORITY, queue,
+              q -> q.getLowestPriority())
+          .description("Lowest priority queued job")
+          .tags(Tags.concat(getCommonTags(), "queue.id", queueId.canonical()))
+          .register(meterRegistry);
+    }
+
+    private void removeMeters(MeterRegistry registry) {

Review Comment:
   I like how you solved removing old queue metrics from the registry. 
   This is important in the event that we have a large number of compaction 
queues that are not in constant use.
   
   Since we currently use a statsd MeterRegistry, I've linked the relevant 
micrometer change that supports metric removal in case there is different 
behaviors when using other MeterRegistry types. 
   
   
https://github.com/micrometer-metrics/micrometer/commit/fc37580df416b1438a2f14e0f60b1009a6c42662#diff-599facf4e7892b4245111835b55991f7a84f3400adec737850f3b264952457e1R200-R216



-- 
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]

Reply via email to