You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ch...@apache.org on 2016/11/17 05:08:27 UTC

svn commit: r1770119 - in /sling/trunk/bundles/commons/metrics/src: main/java/org/apache/sling/commons/metrics/internal/ test/java/org/apache/sling/commons/metrics/internal/

Author: chetanm
Date: Thu Nov 17 05:08:27 2016
New Revision: 1770119

URL: http://svn.apache.org/viewvc?rev=1770119&view=rev
Log:
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.

Added:
    sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java   (with props)
    sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java   (with props)
    sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java   (with props)
    sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java   (with props)
Modified:
    sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java

Added: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java?rev=1770119&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java (added)
+++ sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java Thu Nov 17 05:08:27 2016
@@ -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;
+    }
+}

Propchange: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java?rev=1770119&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java (added)
+++ sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java Thu Nov 17 05:08:27 2016
@@ -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);
+        }
+    }
+}

Propchange: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactory.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java?rev=1770119&r1=1770118&r2=1770119&view=diff
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java (original)
+++ sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/MetricsServiceImpl.java Thu Nov 17 05:08:27 2016
@@ -50,9 +50,10 @@ import org.osgi.framework.ServiceRegistr
 
 @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 implemen
     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 implemen
         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 implemen
     }
 
     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();

Added: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java?rev=1770119&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java (added)
+++ sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java Thu Nov 17 05:08:27 2016
@@ -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

Propchange: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java?rev=1770119&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java (added)
+++ sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java Thu Nov 17 05:08:27 2016
@@ -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

Propchange: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricsServiceFactoryTest.java
------------------------------------------------------------------------------
    svn:eol-style = native