You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2020/01/23 14:17:19 UTC
[sling-org-apache-sling-commons-metrics] branch master updated:
SLING-9025 - The MetricsService keeps stale metrics after bundles that have
registered metrics are removed
This is an automated email from the ASF dual-hosted git repository.
radu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-metrics.git
The following commit(s) were added to refs/heads/master by this push:
new cf8b6bc SLING-9025 - The MetricsService keeps stale metrics after bundles that have registered metrics are removed
cf8b6bc is described below
commit cf8b6bc619f56e3e485532529a904261c793ff7b
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Thu Jan 23 15:16:55 2020 +0100
SLING-9025 - The MetricsService keeps stale metrics after bundles that have registered metrics are removed
* remove cached metrics from the MetricsService when bundle mapped metrics are removed
---
.../sling/commons/metrics/internal/BundleMetricsMapper.java | 5 ++++-
.../sling/commons/metrics/internal/MetricsServiceImpl.java | 6 +++++-
.../commons/metrics/internal/BundleMetricsMapperTest.java | 4 ++--
.../metrics/internal/InternalMetricsServiceFactoryTest.java | 11 ++++++++---
4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java b/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
index a4ec010..f19bc8d 100644
--- a/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
+++ b/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
@@ -39,9 +39,11 @@ class BundleMetricsMapper implements ObjectNameFactory{
static final String JMX_TYPE_METRICS = "Metrics";
private final Logger log = LoggerFactory.getLogger(getClass());
private final ConcurrentMap<String, Bundle> metricToBundleMapping = new ConcurrentHashMap<>();
+ private final MetricsServiceImpl metricsService;
private final MetricRegistry registry;
- BundleMetricsMapper(MetricRegistry registry) {
+ BundleMetricsMapper(MetricsServiceImpl metricsService, MetricRegistry registry) {
+ this.metricsService = metricsService;
this.registry = registry;
}
@@ -53,6 +55,7 @@ class BundleMetricsMapper implements ObjectNameFactory{
for (String name : registeredNames){
registry.remove(name);
metricToBundleMapping.remove(name);
+ metricsService.remove(name);
}
log.debug("Removed metrics for {}", registeredNames);
}
diff --git a/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java b/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java
index 6d5fd08..094c51a 100644
--- a/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java
+++ b/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java
@@ -53,7 +53,7 @@ public class MetricsServiceImpl implements MetricsService {
private final List<ServiceRegistration> regs = new ArrayList<>();
private final ConcurrentMap<String, Metric> metrics = new ConcurrentHashMap<>();
private final MetricRegistry registry = new MetricRegistry();
- private final BundleMetricsMapper metricsMapper = new BundleMetricsMapper(registry);
+ private final BundleMetricsMapper metricsMapper = new BundleMetricsMapper(this, registry);
private GaugeManager gaugeManager;
@Reference(cardinality = ReferenceCardinality.OPTIONAL)
@@ -130,6 +130,10 @@ public class MetricsServiceImpl implements MetricsService {
return null;
}
+ void remove(String name) {
+ metrics.remove(name);
+ }
+
MetricRegistry getRegistry() {
return registry;
}
diff --git a/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java b/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
index ae08a05..ba1ca9e 100644
--- a/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
+++ b/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
@@ -35,7 +35,7 @@ public class BundleMetricsMapperTest {
public final OsgiContext context = new OsgiContext();
private MetricRegistry registry = new MetricRegistry();
- private BundleMetricsMapper mapper = new BundleMetricsMapper(registry);
+ private BundleMetricsMapper mapper = new BundleMetricsMapper(new MetricsServiceImpl(), registry);
@Test
public void defaultDomainName() throws Exception{
@@ -67,4 +67,4 @@ public class BundleMetricsMapperTest {
}
-}
\ No newline at end of file
+}
diff --git a/src/test/java/org/apache/sling/commons/metrics/internal/InternalMetricsServiceFactoryTest.java b/src/test/java/org/apache/sling/commons/metrics/internal/InternalMetricsServiceFactoryTest.java
index 2d3ee4e..3d68fae 100644
--- a/src/test/java/org/apache/sling/commons/metrics/internal/InternalMetricsServiceFactoryTest.java
+++ b/src/test/java/org/apache/sling/commons/metrics/internal/InternalMetricsServiceFactoryTest.java
@@ -21,6 +21,8 @@ package org.apache.sling.commons.metrics.internal;
import com.codahale.metrics.MetricRegistry;
import javax.management.ObjectName;
+
+import org.apache.sling.commons.metrics.Counter;
import org.apache.sling.commons.metrics.MetricsService;
import org.apache.sling.testing.mock.osgi.MockBundle;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
@@ -36,7 +38,7 @@ public class InternalMetricsServiceFactoryTest {
public final OsgiContext context = new OsgiContext();
private MetricsServiceImpl serviceImpl = new MetricsServiceImpl();
private MetricRegistry registry = serviceImpl.getRegistry();
- private BundleMetricsMapper mapper = new BundleMetricsMapper(registry);
+ private BundleMetricsMapper mapper = new BundleMetricsMapper(serviceImpl, registry);
private InternalMetricsServiceFactory srvFactory = new InternalMetricsServiceFactory(serviceImpl, mapper);
private ServiceRegistration<MetricsService> reg = mock(ServiceRegistration.class);
@@ -66,7 +68,7 @@ public class InternalMetricsServiceFactoryTest {
MetricsService srv2 = srvFactory.getService(bar, reg);
srv1.meter("m1");
- srv1.counter("c1");
+ Counter c1 = srv1.counter("c1");
srv2.meter("m2");
assertTrue(registry.getMeters().containsKey("m1"));
@@ -79,6 +81,9 @@ public class InternalMetricsServiceFactoryTest {
assertFalse(registry.getMeters().containsKey("m1"));
assertFalse(registry.getCounters().containsKey("c1"));
+ assertNotEquals("The MetricsService should not return stale metric references.", c1, serviceImpl.counter("c1"));
+ assertTrue(registry.getCounters().containsKey("c1"));
+
//Metrics from 'bar' bundle should be present
assertTrue(registry.getMeters().containsKey("m2"));
}
@@ -90,4 +95,4 @@ public class InternalMetricsServiceFactoryTest {
}
-}
\ No newline at end of file
+}