From 123ffd736c0a435f24f8d9c647e5d384de8cab4a Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 26 Jun 2014 16:11:56 +0100 Subject: [PATCH] Exclude @ManagedResources from Endpoint MBeans If an Endpoint is already @ManagedResource then it doesn't need an additional (probably wrong) MBEan registration based on the invoke() method. --- .../endpoint/jmx/EndpointMBeanExporter.java | 14 +++ .../endpoint/jmx/ShutdownEndpointMBean.java | 2 + ...ointMBeanExportAutoConfigurationTests.java | 115 +++++++++++++++--- 3 files changed, 117 insertions(+), 14 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java index 16bd234cc06..06aa707f56c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java @@ -38,9 +38,11 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationListener; import org.springframework.context.SmartLifecycle; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.jmx.export.MBeanExportException; import org.springframework.jmx.export.MBeanExporter; import org.springframework.jmx.export.annotation.AnnotationJmxAttributeSource; +import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.jmx.export.assembler.MetadataMBeanInfoAssembler; import org.springframework.jmx.export.naming.MetadataNamingStrategy; import org.springframework.jmx.export.naming.SelfNaming; @@ -144,6 +146,18 @@ public class EndpointMBeanExporter extends MBeanExporter implements SmartLifecyc } protected void registerEndpoint(String beanName, Endpoint endpoint) { + @SuppressWarnings("rawtypes") + Class type = endpoint.getClass(); + if (AnnotationUtils.findAnnotation(type, ManagedResource.class) != null) { + // Already managed + return; + } + if (type.isMemberClass() + && AnnotationUtils.findAnnotation(type.getEnclosingClass(), + ManagedResource.class) != null) { + // Nested class with @ManagedResource in parent + return; + } try { registerBeanNameOrInstance(getEndpointMBean(beanName, endpoint), beanName); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java index 0a48775ad77..836d28a8b9c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java @@ -19,12 +19,14 @@ package org.springframework.boot.actuate.endpoint.jmx; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.ShutdownEndpoint; import org.springframework.jmx.export.annotation.ManagedOperation; +import org.springframework.jmx.export.annotation.ManagedResource; /** * Special endpoint wrapper for {@link ShutdownEndpoint}. * * @author Christian Dupuis */ +@ManagedResource public class ShutdownEndpointMBean extends EndpointMBean { public ShutdownEndpointMBean(String beanName, Endpoint endpoint) { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfigurationTests.java index 29f1ed6fa9a..8db2073cbb3 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfigurationTests.java @@ -25,18 +25,26 @@ import javax.management.ReflectionException; import org.junit.After; import org.junit.Test; import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.boot.actuate.endpoint.AbstractEndpoint; +import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.jmx.EndpointMBeanExporter; +import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.autoconfigure.jmx.JmxAutoConfiguration; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.EnableMBeanExport; import org.springframework.jmx.export.MBeanExporter; +import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.jmx.support.ObjectNameManager; import org.springframework.mock.env.MockEnvironment; +import org.springframework.stereotype.Component; import org.springframework.util.ObjectUtils; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; /** @@ -55,13 +63,51 @@ public class EndpointMBeanExportAutoConfigurationTests { } @Test - public void testEndpointMBeanExporterIsInstalled() { + public void testEndpointMBeanExporterIsInstalled() throws Exception { this.context = new AnnotationConfigApplicationContext(); this.context.register(TestConfiguration.class, JmxAutoConfiguration.class, EndpointAutoConfiguration.class, - EndpointMBeanExportAutoConfiguration.class); + EndpointMBeanExportAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); this.context.refresh(); assertNotNull(this.context.getBean(EndpointMBeanExporter.class)); + MBeanExporter mbeanExporter = this.context.getBean(EndpointMBeanExporter.class); + + assertFalse(mbeanExporter.getServer() + .queryNames(getObjectName("*", "*,*", this.context), null).isEmpty()); + } + + @Test + public void testEndpointMBeanExporterIsNotInstalledIfManagedResource() + throws Exception { + this.context = new AnnotationConfigApplicationContext(); + this.context.register(TestConfiguration.class, JmxAutoConfiguration.class, + ManagedEndpoint.class, EndpointMBeanExportAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + assertNotNull(this.context.getBean(EndpointMBeanExporter.class)); + + MBeanExporter mbeanExporter = this.context.getBean(EndpointMBeanExporter.class); + + assertTrue(mbeanExporter.getServer() + .queryNames(getObjectName("*", "*,*", this.context), null).isEmpty()); + } + + @Test + public void testEndpointMBeanExporterIsNotInstalledIfNestedInManagedResource() + throws Exception { + this.context = new AnnotationConfigApplicationContext(); + this.context.register(TestConfiguration.class, JmxAutoConfiguration.class, + NestedInManagedEndpoint.class, + EndpointMBeanExportAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + assertNotNull(this.context.getBean(EndpointMBeanExporter.class)); + + MBeanExporter mbeanExporter = this.context.getBean(EndpointMBeanExporter.class); + + assertTrue(mbeanExporter.getServer() + .queryNames(getObjectName("*", "*,*", this.context), null).isEmpty()); } @Test(expected = NoSuchBeanDefinitionException.class) @@ -125,21 +171,23 @@ public class EndpointMBeanExportAutoConfigurationTests { private ObjectName getObjectName(String domain, String beanKey, ApplicationContext applicationContext) throws MalformedObjectNameException { + String name = "%s:type=Endpoint,name=%s"; if (applicationContext.getParent() != null) { - return ObjectNameManager - .getInstance(String.format( - "%s:type=Endpoint,name=%s,context=%s,identity=%s", domain, - beanKey, - ObjectUtils.getIdentityHexString(applicationContext), - ObjectUtils.getIdentityHexString(applicationContext - .getBean(beanKey)))); + name = name + ",context=%s"; + } + if (applicationContext.getEnvironment().getProperty("endpoints.jmx.unique_names", + Boolean.class, false)) { + name = name + + ",identity=" + + ObjectUtils.getIdentityHexString(applicationContext + .getBean(beanKey)); + } + if (applicationContext.getParent() != null) { + return ObjectNameManager.getInstance(String.format(name, domain, beanKey, + ObjectUtils.getIdentityHexString(applicationContext))); } else { - return ObjectNameManager - .getInstance(String.format("%s:type=Endpoint,name=%s,identity=%s", - domain, beanKey, ObjectUtils - .getIdentityHexString(applicationContext - .getBean(beanKey)))); + return ObjectNameManager.getInstance(String.format(name, domain, beanKey)); } } @@ -148,4 +196,43 @@ public class EndpointMBeanExportAutoConfigurationTests { public static class TestConfiguration { } + + @Component + @ManagedResource + protected static class ManagedEndpoint extends AbstractEndpoint { + + public ManagedEndpoint() { + super("managed", true, true); + } + + @Override + public Boolean invoke() { + return true; + } + + } + + @Configuration + @ManagedResource + protected static class NestedInManagedEndpoint { + + @Bean + public Endpoint nested() { + return new Nested(); + } + + class Nested extends AbstractEndpoint { + + public Nested() { + super("managed", true, true); + } + + @Override + public Boolean invoke() { + return true; + } + } + + } + }