From 41a535bf1cb63d736ea869a6b2f3fbb60515c592 Mon Sep 17 00:00:00 2001 From: Wouter Born Date: Wed, 15 Sep 2021 22:32:53 +0200 Subject: [PATCH] Metrics improvements and fixes (#2486) * Fix StringIndexOutOfBoundsException when using rules file name that starts with 'state' (Constant Warnings in RuleMetric #2472) * Null annotations fixes/improvements * Add transitive com.codahale.metrics dependency which is used by io.micrometer.core.instrument.dropwizard.DropwizardMeterRegistry. This fixes a NoClassDefFoundError when implementing metrics exporters that use a MeterRegistry extending DropwizardMeterRegistry (e.g. JmxMeterRegistry). * Prevent ConcurrentModificationException in ThreadPoolMetric at startup: java.util.ConcurrentModificationException: null at java.util.WeakHashMap$HashIterator.nextEntry(WeakHashMap.java:807) ~[?:?] at java.util.WeakHashMap$KeyIterator.next(WeakHashMap.java:840) ~[?:?] at java.lang.Iterable.forEach(Iterable.java:74) ~[?:?] at org.openhab.core.io.monitor.internal.metrics.ThreadPoolMetric.bindTo(ThreadPoolMetric.java:55) ~[?:?] at org.openhab.core.io.monitor.internal.DefaultMetricsRegistration.lambda$0(DefaultMetricsRegistration.java:97) ~[?:?] at java.lang.Iterable.forEach(Iterable.java:75) ~[?:?] at org.openhab.core.io.monitor.internal.DefaultMetricsRegistration.registerMeters(DefaultMetricsRegistration.java:97) ~[?:?] at org.openhab.core.io.monitor.internal.DefaultMetricsRegistration.onReadyMarkerAdded(DefaultMetricsRegistration.java:115) ~[?:?] at org.openhab.core.internal.service.ReadyServiceImpl.lambda$0(ReadyServiceImpl.java:52) ~[?:?] at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) ~[?:?] at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195) ~[?:?] at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:177) ~[?:?] at java.util.HashMap$EntrySpliterator.forEachRemaining(HashMap.java:1746) ~[?:?] at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?] at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?] at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) ~[?:?] at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) ~[?:?] at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?] at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) ~[?:?] at org.openhab.core.internal.service.ReadyServiceImpl.notifyTrackers(ReadyServiceImpl.java:79) ~[?:?] at org.openhab.core.internal.service.ReadyServiceImpl.markReady(ReadyServiceImpl.java:52) ~[?:?] at org.openhab.core.service.StartLevelService.setStartLevel(StartLevelService.java:248) ~[?:?] at org.openhab.core.service.StartLevelService.lambda$0(StartLevelService.java:125) ~[?:?] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?] at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) [?:?] at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) [?:?] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?] at java.lang.Thread.run(Thread.java:829) [?:?] Most likely another issue is that the ThreadPoolMetric does not add metrics for thread pools that are created on demand. Fixes #2472 Signed-off-by: Wouter Born --- bundles/org.openhab.core.io.monitor/bnd.bnd | 4 +- bundles/org.openhab.core.io.monitor/pom.xml | 6 ++ .../internal/metrics/BundleStateMetric.java | 6 +- .../internal/metrics/EventCountMetric.java | 18 ++--- .../monitor/internal/metrics/JVMMetric.java | 6 +- .../monitor/internal/metrics/RuleMetric.java | 72 +++++++++---------- .../internal/metrics/ThingStateMetric.java | 10 ++- .../internal/metrics/ThreadPoolMetric.java | 6 +- .../core/common/ThreadPoolManager.java | 5 +- 9 files changed, 71 insertions(+), 62 deletions(-) diff --git a/bundles/org.openhab.core.io.monitor/bnd.bnd b/bundles/org.openhab.core.io.monitor/bnd.bnd index e5016fefe2..185b8d95c8 100644 --- a/bundles/org.openhab.core.io.monitor/bnd.bnd +++ b/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 diff --git a/bundles/org.openhab.core.io.monitor/pom.xml b/bundles/org.openhab.core.io.monitor/pom.xml index 21961b4ab4..456ac2d309 100644 --- a/bundles/org.openhab.core.io.monitor/pom.xml +++ b/bundles/org.openhab.core.io.monitor/pom.xml @@ -25,6 +25,12 @@ org.openhab.core.automation ${project.version} + + io.dropwizard.metrics + metrics-core + 4.0.7 + compile + io.micrometer micrometer-core diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/BundleStateMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/BundleStateMetric.java index 7028cf8e2f..6245a23a61 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/BundleStateMetric.java +++ b/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 registeredMeters = new HashMap<>(); - @Nullable - private MeterRegistry meterRegistry = null; + private @Nullable MeterRegistry meterRegistry; private final BundleContext bundleContext; public BundleStateMetric(BundleContext bundleContext, Collection tags) { @@ -87,12 +86,13 @@ public class BundleStateMetric implements OpenhabCoreMeterBinder, BundleListener @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; } } diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/EventCountMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/EventCountMetric.java index 3df6644a39..fc12163b88 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/EventCountMetric.java +++ b/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 tags = new HashSet<>(); private ServiceRegistration eventSubscriberRegistration; private BundleContext bundleContext; @@ -70,6 +69,7 @@ public class EventCountMetric implements OpenhabCoreMeterBinder, EventSubscriber @Override public void unbind() { + MeterRegistry meterRegistry = this.meterRegistry; if (meterRegistry == null) { return; } @@ -78,19 +78,18 @@ public class EventCountMetric implements OpenhabCoreMeterBinder, EventSubscriber 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 getSubscribedEventTypes() { - HashSet 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 class EventCountMetric implements OpenhabCoreMeterBinder, EventSubscriber @Override public void receive(Event event) { + MeterRegistry meterRegistry = this.meterRegistry; if (meterRegistry == null) { logger.trace("Measurement not started. Skipping event processing"); return; diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/JVMMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/JVMMetric.java index b8db225a4b..c0c44f96a4 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/JVMMetric.java +++ b/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 tags = new HashSet<>(); - @Nullable - private MeterRegistry meterRegistry; + private @Nullable MeterRegistry meterRegistry; public JVMMetric(Collection tags) { this.tags.addAll(tags); @@ -61,6 +60,7 @@ public class JVMMetric implements OpenhabCoreMeterBinder { @Override public void unbind() { + MeterRegistry meterRegistry = this.meterRegistry; if (meterRegistry == null) { return; } @@ -69,6 +69,6 @@ public class JVMMetric implements OpenhabCoreMeterBinder { meterRegistry.remove(meter); } } - meterRegistry = null; + this.meterRegistry = null; } } diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java index 3605eb1e86..1ae2192569 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java +++ b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/RuleMetric.java @@ -71,34 +71,33 @@ public class RuleMetric implements OpenhabCoreMeterBinder, EventSubscriber { this.meterRegistry = meterRegistry; Dictionary 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 getSubscribedEventTypes() { - HashSet subscribedEvents = new HashSet<>(); - subscribedEvents.add(RuleStatusInfoEvent.TYPE); - return subscribedEvents; + return Set.of(RuleStatusInfoEvent.TYPE); } @Override @@ -108,34 +107,31 @@ public class RuleMetric implements OpenhabCoreMeterBinder, EventSubscriber { @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 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 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(); } } diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java index d8b9c32c53..dfd32747bb 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java +++ b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java @@ -74,7 +74,7 @@ public class ThingStateMetric implements OpenhabCoreMeterBinder, EventSubscriber thing -> createOrUpdateMetricForBundleState(thing.getUID().getId(), thing.getStatus().ordinal())); Dictionary 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 @@ public class ThingStateMetric implements OpenhabCoreMeterBinder, EventSubscriber @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 diff --git a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThreadPoolMetric.java b/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThreadPoolMetric.java index 4a8dc58419..930523b8d7 100644 --- a/bundles/org.openhab.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThreadPoolMetric.java +++ b/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 tags = new HashSet<>(); - @Nullable - private MeterRegistry meterRegistry; + private @Nullable MeterRegistry meterRegistry; public ThreadPoolMetric(Collection tags) { this.tags.addAll(tags); @@ -70,6 +69,7 @@ public class ThreadPoolMetric implements OpenhabCoreMeterBinder { @Override public void unbind() { + MeterRegistry meterRegistry = this.meterRegistry; if (meterRegistry == null) { return; } @@ -78,6 +78,6 @@ public class ThreadPoolMetric implements OpenhabCoreMeterBinder { meterRegistry.remove(meter); } } - meterRegistry = null; + this.meterRegistry = null; } } diff --git a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/ThreadPoolManager.java b/bundles/org.openhab.core/src/main/java/org/openhab/core/common/ThreadPoolManager.java index 66e9a0d0bb..84a905246a 100644 --- a/bundles/org.openhab.core/src/main/java/org/openhab/core/common/ThreadPoolManager.java +++ b/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 class ThreadPoolManager { 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 getPoolNames() { - return pools.keySet(); + return new HashSet<>(pools.keySet()); } }