You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2017/11/07 09:22:57 UTC
[sling-org-apache-sling-commons-metrics] 07/13: SLING-5443 - Review
the naming conventions used by JMXReporter to register the Metric MBeans
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to annotated tag org.apache.sling.commons.metrics-1.2.0
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-metrics.git
commit f9f9947db0c951969f3bd82fa97ddefdd6f4fefb
Author: Chetan Mehrotra <ch...@apache.org>
AuthorDate: Thu Nov 17 05:08:27 2016 +0000
SLING-5443 - Review the naming conventions used by JMXReporter to register the Metric MBeans
Switch to ServiceFactory for MetricService and use Bundle to determine JMX domain name for metrics created from using bundle.
git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/commons/metrics@1770119 13f79535-47bb-0310-9956-ffa450edef68
---
.../metrics/internal/BundleMetricsMapper.java | 102 +++++++++++++++++++++
.../metrics/internal/MetricsServiceFactory.java | 102 +++++++++++++++++++++
.../metrics/internal/MetricsServiceImpl.java | 23 +++--
.../metrics/internal/BundleMetricsMapperTest.java | 77 ++++++++++++++++
.../internal/MetricsServiceFactoryTest.java | 100 ++++++++++++++++++++
5 files changed, 394 insertions(+), 10 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
new file mode 100644
index 0000000..9a74a1e
--- /dev/null
+++ b/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.commons.metrics.internal;
+
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import javax.management.ObjectName;
+
+import com.codahale.metrics.DefaultObjectNameFactory;
+import com.codahale.metrics.MetricRegistry;
+import com.codahale.metrics.ObjectNameFactory;
+import org.osgi.framework.Bundle;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class BundleMetricsMapper implements ObjectNameFactory{
+ public static final String HEADER_DOMAIN_NAME = "Sling-Metrics-Domain";
+ public static final String DEFAULT_DOMAIN_NAME = "org.apache.sling";
+ private final Logger log = LoggerFactory.getLogger(getClass());
+ private final ConcurrentMap<String, Bundle> metricToBundleMapping = new ConcurrentHashMap<>();
+ private final MetricRegistry registry;
+ private final ObjectNameFactory defaultFactory = new DefaultObjectNameFactory();
+
+ BundleMetricsMapper(MetricRegistry registry) {
+ this.registry = registry;
+ }
+
+ public void addMapping(String name, Bundle bundle) {
+ metricToBundleMapping.putIfAbsent(name, bundle);
+ }
+
+ public void unregister(Set<String> registeredNames) {
+ for (String name : registeredNames){
+ registry.remove(name);
+ metricToBundleMapping.remove(name);
+ }
+ log.debug("Removed metrics for {}", registeredNames);
+ }
+
+ @Override
+ public ObjectName createName(String type, String domain, String name) {
+ String mappedDomainName = safeDomainName(getDomainName(name));
+ if (mappedDomainName == null) {
+ mappedDomainName = domain;
+ }
+ return defaultFactory.createName(type, mappedDomainName, name);
+ }
+
+ private String getDomainName(String name) {
+ Bundle bundle = metricToBundleMapping.get(name);
+ return getDomainName(bundle);
+ }
+
+ private String getDomainName(Bundle bundle) {
+ if (bundle == null){
+ return null;
+ }
+
+ String domainNameHeader = bundle.getHeaders().get(HEADER_DOMAIN_NAME);
+ if (domainNameHeader != null){
+ return domainNameHeader;
+ }
+
+ //Fallback to symbolic name
+ return bundle.getSymbolicName();
+ }
+
+ static String safeDomainName(String name){
+ if (name == null){
+ return null;
+ }
+
+ name = name.trim();
+
+ //Taken from javax.management.ObjectName.isDomain()
+ //Following are special chars in domain name
+ name = name.replace(':', '_');
+ name = name.replace('*', '_');
+ name = name.replace('?', '_');
+ name = name.replace('\n', '_');
+ return name;
+ }
+}
diff --git a/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java b/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java
new file mode 100644
index 0000000..b9d8bb6
--- /dev/null
+++ b/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.commons.metrics.internal;
+
+import java.util.Collections;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.sling.commons.metrics.Counter;
+import org.apache.sling.commons.metrics.Histogram;
+import org.apache.sling.commons.metrics.Meter;
+import org.apache.sling.commons.metrics.MetricsService;
+import org.apache.sling.commons.metrics.Timer;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.ServiceFactory;
+import org.osgi.framework.ServiceRegistration;
+
+class MetricsServiceFactory implements ServiceFactory<MetricsService> {
+ private final MetricsService delegate;
+ private final BundleMetricsMapper metricsMapper;
+
+ public MetricsServiceFactory(MetricsService delegate, BundleMetricsMapper metricsMapper) {
+ this.delegate = delegate;
+ this.metricsMapper = metricsMapper;
+ }
+
+ @Override
+ public MetricsService getService(Bundle bundle, ServiceRegistration<MetricsService> registration) {
+ return new BundleMetricService(bundle);
+ }
+
+ @Override
+ public void ungetService(Bundle bundle, ServiceRegistration<MetricsService> registration, MetricsService service) {
+ if (service instanceof BundleMetricService){
+ ((BundleMetricService) service).unregister();
+ }
+ }
+
+ private class BundleMetricService implements MetricsService {
+ private final Bundle bundle;
+ private Set<String> registeredNames = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());
+
+ public BundleMetricService(Bundle bundle) {
+ this.bundle = bundle;
+ }
+
+ @Override
+ public Timer timer(String name) {
+ addMapping(name);
+ return delegate.timer(name);
+ }
+
+ @Override
+ public Histogram histogram(String name) {
+ addMapping(name);
+ return delegate.histogram(name);
+ }
+
+ @Override
+ public Counter counter(String name) {
+ addMapping(name);
+ return delegate.counter(name);
+ }
+
+ @Override
+ public Meter meter(String name) {
+ addMapping(name);
+ return delegate.meter(name);
+ }
+
+ @Override
+ public <A> A adaptTo(Class<A> type) {
+ return delegate.adaptTo(type);
+ }
+
+ void unregister(){
+ metricsMapper.unregister(registeredNames);
+ }
+
+ private void addMapping(String name) {
+ metricsMapper.addMapping(name, bundle);
+ registeredNames.add(name);
+ }
+ }
+}
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 d2f2728..03efcee 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
@@ -50,9 +50,10 @@ import org.osgi.framework.ServiceRegistration;
@Component
public class MetricsServiceImpl implements MetricsService {
- private final List<ServiceRegistration> regs = new ArrayList<ServiceRegistration>();
- private final ConcurrentMap<String, Metric> metrics = new ConcurrentHashMap<String, Metric>();
+ 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);
@Reference(cardinality = ReferenceCardinality.OPTIONAL_UNARY)
private MBeanServer server;
@@ -63,12 +64,13 @@ public class MetricsServiceImpl implements MetricsService {
private void activate(BundleContext context, Map<String, Object> config) {
enableJMXReporter();
- final Dictionary<String, String> svcProps = new Hashtable<String, String>();
+ final Dictionary<String, String> svcProps = new Hashtable<>();
svcProps.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Metrics Service");
svcProps.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
- regs.add(context.registerService(MetricsService.class.getName(), this, svcProps));
+ regs.add(context.registerService(MetricsService.class.getName(),
+ new MetricsServiceFactory(this, metricsMapper), svcProps));
- final Dictionary<String, String> regProps = new Hashtable<String, String>();
+ final Dictionary<String, String> regProps = new Hashtable<>();
regProps.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Metrics Registry");
regProps.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation");
regProps.put("name", "sling");
@@ -118,6 +120,10 @@ public class MetricsServiceImpl implements MetricsService {
return null;
}
+ MetricRegistry getRegistry() {
+ return registry;
+ }
+
@SuppressWarnings("unchecked")
private <T extends Metric> T getOrAdd(String name, MetricBuilder<T> builder) {
final Metric metric = metrics.get(name);
@@ -202,16 +208,13 @@ public class MetricsServiceImpl implements MetricsService {
}
private void enableJMXReporter() {
- //TODO Domain name should be based on calling bundle
- //For that we can register ServiceFactory and make use of calling
- //bundle symbolic name to determine the mapping
-
if (server == null){
server = ManagementFactory.getPlatformMBeanServer();
}
reporter = JmxReporter.forRegistry(registry)
- .inDomain("org.apache.sling")
+ .inDomain(BundleMetricsMapper.DEFAULT_DOMAIN_NAME)
+ .createsObjectNamesWith(metricsMapper)
.registerWith(server)
.build();
reporter.start();
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
new file mode 100644
index 0000000..d34357a
--- /dev/null
+++ b/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.commons.metrics.internal;
+
+import javax.management.ObjectName;
+
+import com.codahale.metrics.MetricRegistry;
+import com.google.common.collect.ImmutableMap;
+import org.apache.sling.testing.mock.osgi.MockBundle;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class BundleMetricsMapperTest {
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+ private MetricRegistry registry = new MetricRegistry();
+
+ private BundleMetricsMapper mapper = new BundleMetricsMapper(registry);
+
+ @Test
+ public void defaultDomainName() throws Exception{
+ ObjectName name = mapper.createName("counter", "foo", "bar");
+ assertEquals("foo", name.getDomain());
+ }
+
+ @Test
+ public void mappedName_SymbolicName() throws Exception{
+ MockBundle bundle = new MockBundle(context.bundleContext());
+ bundle.setSymbolicName("com.example");
+
+ mapper.addMapping("bar", bundle);
+
+ ObjectName name = mapper.createName("counter", "foo", "bar");
+ assertEquals("com.example", name.getDomain());
+ }
+
+ @Test
+ public void mappedName_Header() throws Exception{
+ MockBundle bundle = new MockBundle(context.bundleContext());
+ bundle.setSymbolicName("com.example");
+ bundle.setHeaders(ImmutableMap.of(BundleMetricsMapper.HEADER_DOMAIN_NAME, "com.test"));
+
+ mapper.addMapping("bar", bundle);
+
+ ObjectName name = mapper.createName("counter", "foo", "bar");
+ assertEquals("com.test", name.getDomain());
+ }
+
+ @Test
+ public void safeDomainName() throws Exception{
+ assertEquals("com.foo", BundleMetricsMapper.safeDomainName("com.foo"));
+ assertEquals("com_foo", BundleMetricsMapper.safeDomainName("com:foo"));
+ assertEquals("com_foo", BundleMetricsMapper.safeDomainName("com?foo"));
+ assertEquals("com_foo", BundleMetricsMapper.safeDomainName("com*foo"));
+ }
+
+}
\ No newline at end of file
diff --git a/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java b/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java
new file mode 100644
index 0000000..71d7fe4
--- /dev/null
+++ b/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.commons.metrics.internal;
+
+import java.util.Collections;
+
+import javax.management.ObjectName;
+
+import com.codahale.metrics.MetricRegistry;
+import org.apache.sling.commons.metrics.MetricsService;
+import org.apache.sling.testing.mock.osgi.MockBundle;
+import org.apache.sling.testing.mock.osgi.MockOsgi;
+import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.ServiceRegistration;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.mock;
+
+public class MetricsServiceFactoryTest {
+ @Rule
+ public final OsgiContext context = new OsgiContext();
+ private MetricsServiceImpl serviceImpl = new MetricsServiceImpl();
+ private MetricRegistry registry = serviceImpl.getRegistry();
+ private BundleMetricsMapper mapper = new BundleMetricsMapper(registry);
+ private MetricsServiceFactory srvFactory = new MetricsServiceFactory(serviceImpl, mapper);
+ private ServiceRegistration<MetricsService> reg = mock(ServiceRegistration.class);
+
+ @Test
+ public void basicWorking() throws Exception{
+ MetricsService service = srvFactory.getService(cb("foo"), reg);
+ service.meter("m1");
+ service.timer("t1");
+ service.histogram("h1");
+ service.counter("c1");
+ assertTrue(registry.getMeters().containsKey("m1"));
+ assertTrue(registry.getTimers().containsKey("t1"));
+ assertTrue(registry.getHistograms().containsKey("h1"));
+ assertTrue(registry.getCounters().containsKey("c1"));
+
+ ObjectName name = mapper.createName("meter", "com.foo", "m1");
+
+ //Domain name should be bundle symbolic name
+ assertEquals("foo", name.getDomain());
+ }
+
+ @Test
+ public void unRegistration() throws Exception{
+ Bundle foo = cb("foo");
+ Bundle bar = cb("bar");
+ MetricsService srv1 = srvFactory.getService(foo, reg);
+ MetricsService srv2 = srvFactory.getService(bar, reg);
+
+ srv1.meter("m1");
+ srv1.counter("c1");
+
+ srv2.meter("m2");
+ assertTrue(registry.getMeters().containsKey("m1"));
+ assertTrue(registry.getMeters().containsKey("m2"));
+ assertTrue(registry.getCounters().containsKey("c1"));
+
+ srvFactory.ungetService(foo, reg, srv1);
+
+ //Metrics from 'foo' bundle i.e. m1 and c1 must be removed
+ assertFalse(registry.getMeters().containsKey("m1"));
+ assertFalse(registry.getCounters().containsKey("c1"));
+
+ //Metrics from 'bar' bundle should be present
+ assertTrue(registry.getMeters().containsKey("m2"));
+ }
+
+ private Bundle cb(String name){
+ MockBundle bundle = new MockBundle(context.bundleContext());
+ bundle.setSymbolicName(name);
+ return bundle;
+ }
+
+
+}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
"commits@sling.apache.org" <co...@sling.apache.org>.