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:42 UTC
svn commit: r1770120 - 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:41 2016
New Revision: 1770120
URL: http://svn.apache.org/viewvc?rev=1770120&view=rev
Log:
SLING-5443 - Review the naming conventions used by JMXReporter to register the Metric MBeans
Specify type for JMX ObjectNames created for Metrics
Added:
sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/JmxUtil.java (with props)
sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java (with props)
Modified:
sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java
sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java
sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricServiceTest.java
Modified: 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=1770120&r1=1770119&r2=1770120&view=diff
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java (original)
+++ sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapper.java Thu Nov 17 05:08:41 2016
@@ -19,10 +19,12 @@
package org.apache.sling.commons.metrics.internal;
+import java.util.Hashtable;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import javax.management.MalformedObjectNameException;
import javax.management.ObjectName;
import com.codahale.metrics.DefaultObjectNameFactory;
@@ -35,6 +37,7 @@ 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";
+ static final String JMX_TYPE_METRICS = "Metrics";
private final Logger log = LoggerFactory.getLogger(getClass());
private final ConcurrentMap<String, Bundle> metricToBundleMapping = new ConcurrentHashMap<>();
private final MetricRegistry registry;
@@ -58,11 +61,20 @@ class BundleMetricsMapper implements Obj
@Override
public ObjectName createName(String type, String domain, String name) {
- String mappedDomainName = safeDomainName(getDomainName(name));
+ String mappedDomainName = JmxUtil.safeDomainName(getDomainName(name));
if (mappedDomainName == null) {
mappedDomainName = domain;
}
- return defaultFactory.createName(type, mappedDomainName, name);
+
+ Hashtable<String, String> table = new Hashtable<String, String>();
+ table.put("type", JMX_TYPE_METRICS);
+ table.put("name", JmxUtil.quoteValueIfRequired(name));
+ try {
+ return new ObjectName(mappedDomainName, table);
+ } catch (MalformedObjectNameException e) {
+ log.warn("Unable to register {} {}", type, name, e);
+ throw new RuntimeException(e);
+ }
}
private String getDomainName(String name) {
@@ -84,19 +96,4 @@ class BundleMetricsMapper implements Obj
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;
- }
}
Added: sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/JmxUtil.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/JmxUtil.java?rev=1770120&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/JmxUtil.java (added)
+++ sling/trunk/bundles/commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/JmxUtil.java Thu Nov 17 05:08:41 2016
@@ -0,0 +1,89 @@
+/*
+ * 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.MalformedObjectNameException;
+import javax.management.ObjectName;
+
+/**
+ * Utility methods related to JMX
+ */
+final class JmxUtil {
+
+ /**
+ * Checks if the passed value string can be used as is as part of
+ * JMX {@link javax.management.ObjectName} If it cannot be used then
+ * it would return a quoted string which is then safe to be used
+ * as part of ObjectName.
+ *
+ * <p>This is meant to avoid unnecessary quoting of value</p>
+ *
+ * @param unquotedValue to quote if required
+ * @return passed value or quoted value if required
+ */
+ public static String quoteValueIfRequired(String unquotedValue) {
+ String result;
+ String quotedValue = ObjectName.quote(unquotedValue);
+
+ //Check if some chars are escaped or not. In that case
+ //length of quoted string (excluding quotes) would differ
+ if (quotedValue.substring(1, quotedValue.length() - 1).equals(unquotedValue)) {
+ ObjectName on = null;
+ try {
+ //Quoting logic in ObjectName does not escape ',', '='
+ //etc. So try now by constructing ObjectName. If that
+ //passes then value can be used as safely
+
+ //Also we cannot just rely on ObjectName as it treats
+ //*, ? as pattern chars and which should ideally be escaped
+ on = new ObjectName("dummy", "dummy", unquotedValue);
+ } catch (MalformedObjectNameException ignore) {
+ //ignore
+ }
+
+ if (on != null){
+ result = unquotedValue;
+ } else {
+ result = quotedValue;
+ }
+ } else {
+ //Some escaping done. So do quote
+ result = quotedValue;
+ }
+ return result;
+ }
+
+
+ public 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/JmxUtil.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: 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=1770120&r1=1770119&r2=1770120&view=diff
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java (original)
+++ sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/BundleMetricsMapperTest.java Thu Nov 17 05:08:41 2016
@@ -66,12 +66,5 @@ public class BundleMetricsMapperTest {
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
Added: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java?rev=1770120&view=auto
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java (added)
+++ sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java Thu Nov 17 05:08:41 2016
@@ -0,0 +1,51 @@
+/*
+ * 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 junit.framework.TestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class JmxUtilTest {
+
+ @Test
+ public void quotation() throws Exception{
+ assertEquals("text", JmxUtil.quoteValueIfRequired("text"));
+ TestCase.assertEquals("", JmxUtil.quoteValueIfRequired(""));
+ assertTrue(JmxUtil.quoteValueIfRequired("text*with?chars").startsWith("\""));
+ }
+
+ @Test
+ public void quoteAndComma() throws Exception{
+ assertTrue(JmxUtil.quoteValueIfRequired("text,withComma").startsWith("\""));
+ assertTrue(JmxUtil.quoteValueIfRequired("text=withEqual").startsWith("\""));
+ }
+
+ @Test
+ public void safeDomainName() throws Exception{
+ assertEquals("com.foo", JmxUtil.safeDomainName("com.foo"));
+ assertEquals("com_foo", JmxUtil.safeDomainName("com:foo"));
+ assertEquals("com_foo", JmxUtil.safeDomainName("com?foo"));
+ assertEquals("com_foo", JmxUtil.safeDomainName("com*foo"));
+ }
+
+}
\ No newline at end of file
Propchange: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/JmxUtilTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricServiceTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricServiceTest.java?rev=1770120&r1=1770119&r2=1770120&view=diff
==============================================================================
--- sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricServiceTest.java (original)
+++ sling/trunk/bundles/commons/metrics/src/test/java/org/apache/sling/commons/metrics/internal/MetricServiceTest.java Thu Nov 17 05:08:41 2016
@@ -41,6 +41,7 @@ import org.junit.After;
import org.junit.Rule;
import org.junit.Test;
+import static org.apache.sling.commons.metrics.internal.BundleMetricsMapper.JMX_TYPE_METRICS;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
@@ -134,7 +135,7 @@ public class MetricServiceTest {
Meter meter = service.meter("test");
assertNotNull(meter);
QueryExp q = Query.isInstanceOf(Query.value(JmxReporter.JmxMeterMBean.class.getName()));
- Set<ObjectName> names = server.queryNames(new ObjectName("org.apache.sling:name=*"), q);
+ Set<ObjectName> names = server.queryNames(new ObjectName("org.apache.sling:name=*,type="+ JMX_TYPE_METRICS), q);
assertThat(names, is(not(empty())));
MockOsgi.deactivate(service, context.bundleContext());