Metrics improvements and fixes

bundles/org.openhab.core.io.monitor/bnd.bnd

@@ -7,6 +7,8 @@ Import-Package: \
  org.osgi.service.*,\
  org.slf4j.*,\
  com.google.gson.*;version="[2.8,3)"
-Export-Package: io.micrometer.core.*;-split-package:=merge-first,\
+Export-Package: \
+ com.codahale.metrics.*;-split-package:=merge-first,\
+ io.micrometer.core.*;-split-package:=merge-first,\
  org.HdrHistogram.*;-split-package:=merge-first,\
  org.LatencyUtils.*;-split-package:=merge-first

bundles/org.openhab.core.io.monitor/pom.xml

@@ -25,6 +25,12 @@
       <artifactId>org.openhab.core.automation</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>io.dropwizard.metrics</groupId>
+      <artifactId>metrics-core</artifactId>
+      <version>4.0.7</version>
+      <scope>compile</scope>
+    </dependency>
     <dependency>
       <groupId>io.micrometer</groupId>
       <artifactId>micrometer-core</artifactId>

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/BundleStateMetric.java

@@ -42,8 +42,7 @@ public class BundleStateMetric implements OpenhabCoreMeterBinder, BundleListener
     private static final String BUNDLE_TAG_NAME = "bundle";
     private final Meter.Id commonMeterId;
     private final Map<Meter.Id, AtomicInteger> registeredMeters = new HashMap<>();
-    @Nullable
-    private MeterRegistry meterRegistry = null;
+    private @Nullable MeterRegistry meterRegistry;
     private final BundleContext bundleContext;
 
     public BundleStateMetric(BundleContext bundleContext, Collection<Tag> tags) {
@@ -87,12 +86,13 @@ private void createOrUpdateMetricForBundleState(String bundleName, int state) {
 
     @Override
     public void unbind() {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             return;
         }
         bundleContext.removeBundleListener(this);
         registeredMeters.keySet().forEach(meterRegistry::remove);
         registeredMeters.clear();
-        meterRegistry = null;
+        this.meterRegistry = null;
     }
 }

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/EventCountMetric.java

@@ -45,8 +45,7 @@ public class EventCountMetric implements OpenhabCoreMeterBinder, EventSubscriber
     private final Logger logger = LoggerFactory.getLogger(EventCountMetric.class);
     private static final Tag CORE_EVENT_COUNT_METRIC_TAG = Tag.of("metric", "openhab.core.metric.eventcount");
     private static final String TOPIC_TAG_NAME = "topic";
-    @Nullable
-    private MeterRegistry meterRegistry;
+    private @Nullable MeterRegistry meterRegistry;
     private final Set<Tag> tags = new HashSet<>();
     private ServiceRegistration<?> eventSubscriberRegistration;
     private BundleContext bundleContext;
@@ -70,6 +69,7 @@ public void bindTo(MeterRegistry meterRegistry) {
 
     @Override
     public void unbind() {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             return;
         }
@@ -78,19 +78,18 @@ public void unbind() {
                 meterRegistry.remove(meter);
             }
         }
-        meterRegistry = null;
-        if (this.eventSubscriberRegistration != null) {
-            this.eventSubscriberRegistration.unregister();
+        this.meterRegistry = null;
+
+        ServiceRegistration<?> eventSubscriberRegistration = this.eventSubscriberRegistration;
+        if (eventSubscriberRegistration != null) {
+            eventSubscriberRegistration.unregister();
             this.eventSubscriberRegistration = null;
         }
     }
 
     @Override
     public Set<String> getSubscribedEventTypes() {
-        HashSet<String> subscribedEvents = new HashSet<>();
-        subscribedEvents.add(ItemCommandEvent.TYPE);
-        subscribedEvents.add(ItemStateEvent.TYPE);
-        return subscribedEvents;
+        return Set.of(ItemCommandEvent.TYPE, ItemStateEvent.TYPE);
     }
 
     @Override
@@ -100,6 +99,7 @@ public Set<String> getSubscribedEventTypes() {
 
     @Override
     public void receive(Event event) {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             logger.trace("Measurement not started. Skipping event processing");
             return;

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/JVMMetric.java

@@ -39,8 +39,7 @@ public class JVMMetric implements OpenhabCoreMeterBinder {
     private final Logger logger = LoggerFactory.getLogger(JVMMetric.class);
     private static final Tag CORE_JVM_METRIC_TAG = Tag.of("metric", "openhab.core.metric.jvm");
     private final Set<Tag> tags = new HashSet<>();
-    @Nullable
-    private MeterRegistry meterRegistry;
+    private @Nullable MeterRegistry meterRegistry;
 
     public JVMMetric(Collection<Tag> tags) {
         this.tags.addAll(tags);
@@ -61,6 +60,7 @@ public void bindTo(MeterRegistry registry) {
 
     @Override
     public void unbind() {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             return;
         }
@@ -69,6 +69,6 @@ public void unbind() {
                 meterRegistry.remove(meter);
             }
         }
-        meterRegistry = null;
+        this.meterRegistry = null;
     }
 }

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java

@@ -71,34 +71,33 @@ public void bindTo(MeterRegistry meterRegistry) {
         this.meterRegistry = meterRegistry;
         Dictionary<String, Object> properties = new Hashtable<>();
         properties.put(SUBSCRIPTION_PROPERTY_TOPIC, RULES_TOPIC_FILTER);
-        this.eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this,
+        eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this,
                 properties);
     }
 
     @Override
     public void unbind() {
-        MeterRegistry mReg = meterRegistry;
-        if (mReg == null) {
+        MeterRegistry meterRegistry = this.meterRegistry;
+        if (meterRegistry == null) {
             return;
-        } else {
-            for (Meter meter : mReg.getMeters()) {
-                if (meter.getId().getTags().contains(CORE_RULE_METRIC_TAG)) {
-                    mReg.remove(meter);
-                }
-            }
-            meterRegistry = null;
-            if (this.eventSubscriberRegistration != null) {
-                this.eventSubscriberRegistration.unregister();
-                this.eventSubscriberRegistration = null;
+        }
+        for (Meter meter : meterRegistry.getMeters()) {
+            if (meter.getId().getTags().contains(CORE_RULE_METRIC_TAG)) {
+                meterRegistry.remove(meter);
             }
         }
+        this.meterRegistry = null;
+
+        ServiceRegistration<?> eventSubscriberRegistration = this.eventSubscriberRegistration;
+        if (eventSubscriberRegistration != null) {
+            eventSubscriberRegistration.unregister();
+            this.eventSubscriberRegistration = null;
+        }
     }
 
     @Override
     public Set<String> getSubscribedEventTypes() {
-        HashSet<String> subscribedEvents = new HashSet<>();
-        subscribedEvents.add(RuleStatusInfoEvent.TYPE);
-        return subscribedEvents;
+        return Set.of(RuleStatusInfoEvent.TYPE);
     }
 
     @Override
@@ -108,34 +107,31 @@ public Set<String> getSubscribedEventTypes() {
 
     @Override
     public void receive(Event event) {
-        MeterRegistry mReg = meterRegistry;
-        if (mReg == null) {
+        MeterRegistry meterRegistry = this.meterRegistry;
+        if (meterRegistry == null) {
             logger.trace("Measurement not started. Skipping rule event processing");
             return;
-        } else {
-            String topic = event.getTopic();
-            String ruleId = topic.substring(RULES_TOPIC_PREFIX.length(), topic.indexOf(RULES_TOPIC_SUFFIX));
-            if (!event.getPayload().contains(RuleStatus.RUNNING.name())) {
-                logger.trace("Skipping rule status info with status other than RUNNING {}", event.getPayload());
-                return;
-            }
+        }
 
-            logger.debug("Rule {} RUNNING - updating metric.", ruleId);
-            Set<Tag> tagsWithRule = new HashSet<>(tags);
-            tagsWithRule.add(Tag.of(RULE_ID_TAG_NAME, ruleId));
-            String ruleName = getRuleName(ruleId);
-            if (ruleName != null) {
-                tagsWithRule.add(Tag.of(RULE_NAME_TAG_NAME, ruleName));
-            }
-            mReg.counter(METRIC_NAME, tagsWithRule).increment();
+        String topic = event.getTopic();
+        String ruleId = topic.substring(RULES_TOPIC_PREFIX.length(), topic.lastIndexOf(RULES_TOPIC_SUFFIX));
+        if (!event.getPayload().contains(RuleStatus.RUNNING.name())) {
+            logger.trace("Skipping rule status info with status other than RUNNING {}", event.getPayload());
+            return;
+        }
+
+        logger.debug("Rule {} RUNNING - updating metric.", ruleId);
+        Set<Tag> tagsWithRule = new HashSet<>(tags);
+        tagsWithRule.add(Tag.of(RULE_ID_TAG_NAME, ruleId));
+        String ruleName = getRuleName(ruleId);
+        if (ruleName != null) {
+            tagsWithRule.add(Tag.of(RULE_NAME_TAG_NAME, ruleName));
         }
+        meterRegistry.counter(METRIC_NAME, tagsWithRule).increment();
     }
 
     private String getRuleName(String ruleId) {
         Rule rule = ruleRegistry.get(ruleId);
-        if (rule != null) {
-            return rule.getName();
-        }
-        return null;
+        return rule == null ? null : rule.getName();
     }
 }

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java

@@ -74,7 +74,7 @@ public void bindTo(@io.micrometer.core.lang.NonNull MeterRegistry meterRegistry)
                 thing -> createOrUpdateMetricForBundleState(thing.getUID().getId(), thing.getStatus().ordinal()));
         Dictionary<String, Object> properties = new Hashtable<>();
         properties.put("event.topics", "openhab/things/*");
-        this.eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this,
+        eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this,
                 properties);
     }
 
@@ -92,16 +92,20 @@ private void createOrUpdateMetricForBundleState(String thingUid, int thingStatus
 
     @Override
     public void unbind() {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             return;
         }
+
+        ServiceRegistration<?> eventSubscriberRegistration = this.eventSubscriberRegistration;
         if (eventSubscriberRegistration != null) {
             eventSubscriberRegistration.unregister();
-            eventSubscriberRegistration = null;
+            this.eventSubscriberRegistration = null;
         }
         registeredMeters.keySet().forEach(meterRegistry::remove);
         registeredMeters.clear();
-        meterRegistry = null;
+
+        this.meterRegistry = null;
     }
 
     @Override

bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThreadPoolMetric.java

@@ -38,8 +38,7 @@ public class ThreadPoolMetric implements OpenhabCoreMeterBinder {
     public static final Tag CORE_THREADPOOL_METRIC_TAG = Tag.of("metric", "openhab.core.metric.threadpools");
     private static final String POOLNAME_TAG_NAME = "pool";
     private final Set<Tag> tags = new HashSet<>();
-    @Nullable
-    private MeterRegistry meterRegistry;
+    private @Nullable MeterRegistry meterRegistry;
 
     public ThreadPoolMetric(Collection<Tag> tags) {
         this.tags.addAll(tags);
@@ -70,6 +69,7 @@ private void addPoolMetrics(String poolName) {
 
     @Override
     public void unbind() {
+        MeterRegistry meterRegistry = this.meterRegistry;
         if (meterRegistry == null) {
             return;
         }
@@ -78,6 +78,6 @@ public void unbind() {
                 meterRegistry.remove(meter);
             }
         }
-        meterRegistry = null;
+        this.meterRegistry = null;
     }
 }

bundles/org.openhab.core/src/main/java/org/openhab/core/common/ThreadPoolManager.java

@@ -12,6 +12,7 @@
  */
 package org.openhab.core.common;
 
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -166,10 +167,10 @@ public static ExecutorService getPool(String poolName) {
 
     protected static int getConfig(String poolName) {
         Integer cfg = configs.get(poolName);
-        return (cfg != null) ? cfg : DEFAULT_THREAD_POOL_SIZE;
+        return cfg != null ? cfg : DEFAULT_THREAD_POOL_SIZE;
     }
 
     public static Set<String> getPoolNames() {
-        return pools.keySet();
+        return new HashSet<>(pools.keySet());
     }
 }