Skip to content

Commit

Permalink
Merge pull request #254 from ryanrupp/objectNameMapConfig
Browse files Browse the repository at this point in the history
Allow the JMX ObjectNameMapper to be specified as a property (com.netfli...
  • Loading branch information
dmuino committed May 27, 2014
2 parents 2f71d3c + 473b9ca commit 3128289
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
package com.netflix.servo;

import com.netflix.servo.jmx.JmxMonitorRegistry;
import com.netflix.servo.jmx.ObjectNameMapper;
import com.netflix.servo.monitor.Monitor;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -29,13 +31,21 @@
* specified registry class must have a constructor with no arguments. If the
* property is not specified or the class cannot be loaded an instance of
* {@link com.netflix.servo.jmx.JmxMonitorRegistry} will be used.
* <p>
* If the default {@link com.netflix.servo.jmx.JmxMonitorRegistry} is used, the property
* {@code com.netflix.servo.DefaultMonitorRegistry.jmxMapperClass} can optionally be
* specified to control how monitors are mapped to JMX {@link javax.management.ObjectName}.
* This property specifies the {@link com.netflix.servo.jmx.ObjectNameMapper}
* implementation class to use. The implementation must have a constructor with
* no arguments.
*/
public final class DefaultMonitorRegistry implements MonitorRegistry {

private static final Logger LOG = LoggerFactory.getLogger(DefaultMonitorRegistry.class);
private static final String CLASS_NAME = DefaultMonitorRegistry.class.getCanonicalName();
private static final String REGISTRY_CLASS_PROP = CLASS_NAME + ".registryClass";
private static final String REGISTRY_NAME_PROP = CLASS_NAME + ".registryName";
private static final String REGISTRY_JMX_NAME_PROP = CLASS_NAME + ".jmxMapperClass";
private static final MonitorRegistry INSTANCE = new DefaultMonitorRegistry();
private static final String DEFAULT_REGISTRY_NAME = "com.netflix.servo";

Expand Down Expand Up @@ -77,8 +87,36 @@ public static MonitorRegistry getInstance() {
}
registry = r;
} else {
registry = new JmxMonitorRegistry(registryName);
registry = new JmxMonitorRegistry(registryName,
getObjectNameMapper(props));
}
}

/**
* Gets the {@link ObjectNameMapper} to use by looking at the
* {@code com.netflix.servo.DefaultMonitorRegistry.jmxMapperClass}
* property. If not specified, then {@link ObjectNameMapper#DEFAULT}
* is used.
* @param props the properties
* @return the mapper to use
*/
private static ObjectNameMapper getObjectNameMapper(Properties props) {
ObjectNameMapper mapper = ObjectNameMapper.DEFAULT;
final String jmxNameMapperClass = props.getProperty(REGISTRY_JMX_NAME_PROP);
if (jmxNameMapperClass != null) {
try {
Class<?> mapperClazz = Class.forName(jmxNameMapperClass);
mapper = (ObjectNameMapper) mapperClazz.newInstance();
} catch (Throwable t) {
LOG.error(
"failed to create the JMX ObjectNameMapper instance of class "
+ jmxNameMapperClass
+ ", using the default naming scheme",
t);
}
}

return mapper;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@

import com.google.common.collect.Sets;
import com.netflix.servo.jmx.JmxMonitorRegistry;
import com.netflix.servo.jmx.ObjectNameMapper;
import com.netflix.servo.monitor.BasicCounter;
import com.netflix.servo.monitor.Monitor;
import com.netflix.servo.monitor.MonitorConfig;

import org.testng.annotations.Test;

import java.lang.management.ManagementFactory;
import java.util.Properties;
import java.util.Set;

import javax.management.ObjectName;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;

Expand Down Expand Up @@ -136,4 +142,43 @@ public void testGetRegisteredMonitors() throws Exception {
assertTrue(monitors.contains(m1));
assertTrue(monitors.contains(m2));
}*/

@Test
public void testCustomJmxObjectMapper() {
Properties props = new Properties();
props.put("com.netflix.servo.DefaultMonitorRegistry.jmxMapperClass",
"com.netflix.servo.DefaultMonitorRegistryTest$ChangeDomainMapper");
DefaultMonitorRegistry registry = new DefaultMonitorRegistry(props);
BasicCounter counter = new BasicCounter(
new MonitorConfig.Builder("testCustomJmxObjectMapper").build());
registry.register(counter);
ObjectName expectedName =
new ChangeDomainMapper().createObjectName("com.netflix.servo", counter);
assertEquals(expectedName.getDomain(), "com.netflix.servo.Renamed");
assertTrue(ManagementFactory.getPlatformMBeanServer().isRegistered(expectedName));
}

@Test
public void testInvalidMapperDefaults() {
Properties props = new Properties();
props.put("com.netflix.servo.DefaultMonitorRegistry.jmxMapperClass",
"com.my.invalid.class");
DefaultMonitorRegistry registry = new DefaultMonitorRegistry(props);
BasicCounter counter = new BasicCounter(
new MonitorConfig.Builder("testInvalidMapperDefaults").build());
registry.register(counter);
ObjectName expectedName =
ObjectNameMapper.DEFAULT.createObjectName("com.netflix.servo", counter);
assertEquals(expectedName.getDomain(), "com.netflix.servo");
assertTrue(ManagementFactory.getPlatformMBeanServer().isRegistered(expectedName));
}

public static class ChangeDomainMapper implements ObjectNameMapper {

@Override
public ObjectName createObjectName(String domain, Monitor<?> monitor) {
return ObjectNameMapper.DEFAULT.createObjectName(domain + ".Renamed", monitor);
}

}
}

0 comments on commit 3128289

Please sign in to comment.