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 <github@maindrain.net>
This commit is contained in:
Wouter Born 2021-09-15 22:32:53 +02:00 committed by GitHub
parent 14bbbb6a92
commit 41a535bf1c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 71 additions and 62 deletions

View File

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

View File

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

View File

@ -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 @@ 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;
}
}

View File

@ -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 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<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 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;

View File

@ -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 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;
}
}

View File

@ -71,34 +71,33 @@ public class RuleMetric implements OpenhabCoreMeterBinder, EventSubscriber {
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 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<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();
}
}

View File

@ -74,7 +74,7 @@ public class ThingStateMetric implements OpenhabCoreMeterBinder, EventSubscriber
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 @@ 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

View File

@ -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 @@ 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;
}
}

View File

@ -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<String> getPoolNames() {
return pools.keySet();
return new HashSet<>(pools.keySet());
}
}