From 3dda215e7fdb977bb4a95468a7d5602834f5e42b Mon Sep 17 00:00:00 2001 From: Scott Hraban Date: Sun, 2 Jul 2023 00:31:02 -0700 Subject: [PATCH] Always use ThingUid as the unique identifier for thing status metrics (#3674) * Always use ThingUid as the unique identifier for thing status metrics At bind time, ThingId was being used, but then ThingUid was being used when changes occured, causing multiple meters to be created for the same Thing, with the status always being 0 for the meter created at bind time. This change uses the full ThingUid, for both binding and event processing, since ThingUid is the globally unique value. Fixes #3672 Signed-off-by: Scott Hraban GitOrigin-RevId: 505e51210e11f5a2ab1016a5c2098c7a28f22959 --- .../internal/metrics/ThingStateMetric.java | 21 +++-- .../metrics/ThingStateMetricTest.java | 91 +++++++++++++++++++ 2 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 bundles/org.opensmarthouse.core.io.monitor/src/test/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetricTest.java diff --git a/bundles/org.opensmarthouse.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java b/bundles/org.opensmarthouse.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java index b74cd99416a..4501bbacfed 100644 --- a/bundles/org.opensmarthouse.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java +++ b/bundles/org.opensmarthouse.core.io.monitor/src/main/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetric.java @@ -40,16 +40,18 @@ import io.micrometer.core.instrument.Tags; /** - * The {@link ThingStateMetric} class implements a metric for the openHAB things states. + * The {@link ThingStateMetric} class implements a metric for the openHAB things + * states. * * @author Robert Bach - Initial contribution + * @author Scott Hraban - Create Meter using thingUid instead of thingId during + * bind phase */ @NonNullByDefault public class ThingStateMetric implements OpenhabCoreMeterBinder, EventSubscriber { private final Logger logger = LoggerFactory.getLogger(ThingStateMetric.class); public static final String METRIC_NAME = "openhab.thing.state"; private static final String THING_TAG_NAME = "thing"; - private static final String THINGSTATUS_TOPIC_PREFIX = "openhab/things/"; private final ThingRegistry thingRegistry; private final Meter.Id commonMeterId; private final Map registeredMeters = new HashMap<>(); @@ -70,7 +72,7 @@ public void bindTo(@NonNullByDefault({}) MeterRegistry meterRegistry) { logger.debug("ThingStateMetric is being bound..."); this.meterRegistry = meterRegistry; thingRegistry.getAll().forEach( - thing -> createOrUpdateMetricForBundleState(thing.getUID().getId(), thing.getStatus().ordinal())); + thing -> createOrUpdateMetricForBundleState(thing.getUID().getAsString(), thing.getStatus().ordinal())); eventSubscriberRegistration = this.bundleContext.registerService(EventSubscriber.class.getName(), this, null); } @@ -111,10 +113,13 @@ public Set getSubscribedEventTypes() { @Override public void receive(Event event) { - logger.trace("Received ThingStatusInfo(Changed)Event..."); - String thingId = event.getTopic().substring(THINGSTATUS_TOPIC_PREFIX.length(), - event.getTopic().lastIndexOf('/')); - ThingStatus status = gson.fromJson(event.getPayload(), ThingStatusInfo.class).getStatus(); - createOrUpdateMetricForBundleState(thingId, status.ordinal()); + if (event instanceof ThingStatusInfoEvent thingEvent) { + logger.trace("Received ThingStatusInfo(Changed)Event..."); + String thingUid = thingEvent.getThingUID().getAsString(); + ThingStatus status = gson.fromJson(event.getPayload(), ThingStatusInfo.class).getStatus(); + createOrUpdateMetricForBundleState(thingUid, status.ordinal()); + } else { + logger.trace("Received unsubscribed for event type {}", event.getClass().getSimpleName()); + } } } diff --git a/bundles/org.opensmarthouse.core.io.monitor/src/test/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetricTest.java b/bundles/org.opensmarthouse.core.io.monitor/src/test/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetricTest.java new file mode 100644 index 00000000000..61118050546 --- /dev/null +++ b/bundles/org.opensmarthouse.core.io.monitor/src/test/java/org/openhab/core/io/monitor/internal/metrics/ThingStateMetricTest.java @@ -0,0 +1,91 @@ +/** + * Copyright (c) 2010-2023 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.io.monitor.internal.metrics; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +import java.util.Collections; +import java.util.HashSet; +import java.util.List; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; +import org.openhab.core.thing.Thing; +import org.openhab.core.thing.ThingRegistry; +import org.openhab.core.thing.ThingStatus; +import org.openhab.core.thing.ThingStatusDetail; +import org.openhab.core.thing.ThingStatusInfo; +import org.openhab.core.thing.ThingTypeUID; +import org.openhab.core.thing.ThingUID; +import org.openhab.core.thing.events.ThingEventFactory; +import org.openhab.core.thing.internal.ThingImpl; +import org.osgi.framework.BundleContext; + +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; + +/** + * Tests for ThingStateMetric class + * + * @author Scott Hraban - Initial contribution + */ +@ExtendWith(MockitoExtension.class) +@NonNullByDefault +public class ThingStateMetricTest { + + @Test + public void testThingUidAlwaysUsedToCreateMeter() { + final String strThingTypeUid = "sonos:Amp"; + + final String strThingUid = strThingTypeUid + ":RINCON_347E5C0D150501400"; + ThingUID thingUid = new ThingUID(strThingUid); + Thing thing = new ThingImpl(new ThingTypeUID(strThingTypeUid), thingUid); + + final String strThingUid2 = strThingTypeUid + ":foo"; + ThingUID thingUid2 = new ThingUID(strThingUid2); + + ThingRegistry thingRegistry = mock(ThingRegistry.class); + + SimpleMeterRegistry meterRegistry = new SimpleMeterRegistry(); + + ThingStateMetric thingStateMetric = new ThingStateMetric(mock(BundleContext.class), thingRegistry, + new HashSet()); + + // Only one meter registered at bind time + doReturn(Collections.singleton(thing)).when(thingRegistry).getAll(); + thingStateMetric.bindTo(meterRegistry); + + List meters = meterRegistry.getMeters(); + assertEquals(1, meters.size()); + assertEquals(strThingUid, meters.get(0).getId().getTag("thing")); + + // Still only one meter registered after receiving an event + ThingStatusInfo thingStatusInfo = new ThingStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, null); + thingStateMetric.receive(ThingEventFactory.createStatusInfoEvent(thingUid, thingStatusInfo)); + + meters = meterRegistry.getMeters(); + assertEquals(1, meters.size()); + assertEquals(strThingUid, meters.get(0).getId().getTag("thing")); + + // Now another one is added + thingStateMetric.receive(ThingEventFactory.createStatusInfoEvent(thingUid2, thingStatusInfo)); + + meters = meterRegistry.getMeters(); + assertEquals(2, meters.size()); + } +}