INT-3186 Fix Metrics For Direct MessageHandler
When a simple MessageHandler is referenced from a `<service-activator/>`, it is wrapped in an anonymous AbstractReplyProducingMessageHandler to facilitate proper handling of the `<request-handler-advice-chain/>` and so that normal 'produced no reply` messages are logged. Previously, this caused 2 MBeans to be exposed, one for tha actual handler; one for the wrapper. This added more stack frames between the dispatcher and the handler (because 2 sets of statistics were captured). The root cause was that the MBean exporter was not aware of the relationship between these objects. https://jira.springsource.org/browse/INT-3186 Add code to the IntegrationMBeanExporter to suppress adding metrics to the wrapper. This involves detecting that it is a wrapper and not exposing it as an MBean and, when matching handlers with endpoints, checking that the wrapped handler is the actual handler for the endpoint. Update test to reflect the reduced number of stack frames between the dispatcher and handler. Also add tests to verify the detection of multiple references to the same ARPMH. INT-3186: `IntegrationMBeanExporter` optimization
This commit is contained in:
committed by
Artem Bilan
parent
cc434db959
commit
eaa28a9dbf
@@ -73,7 +73,7 @@ public class ServiceActivatorFactoryBean extends AbstractStandardMessageHandlerF
|
||||
}
|
||||
/*
|
||||
* Return a reply-producing message handler so that we still get 'produced no reply' messages
|
||||
* and the super class will inject the advice chain to advise the handler if needed.
|
||||
* and the super class will inject the advice chain to advise the handler method if needed.
|
||||
*/
|
||||
handler = new AbstractReplyProducingMessageHandler() {
|
||||
|
||||
|
||||
@@ -22,19 +22,19 @@
|
||||
<service-activator id="replyingHandlerWithStandardMethodTestService"
|
||||
input-channel="replyingHandlerWithStandardMethodTestInputChannel"
|
||||
method="handleMessage">
|
||||
<beans:bean
|
||||
<beans:bean id="innerReplyingHandler"
|
||||
class="org.springframework.integration.handler.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
<service-activator id="replyingHandlerWithOtherMethodTestService"
|
||||
input-channel="replyingHandlerWithOtherMethodTestInputChannel"
|
||||
method="foo">
|
||||
<beans:bean
|
||||
<beans:bean id="innerReplyingHandlerFoo"
|
||||
class="org.springframework.integration.handler.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
<service-activator id="handlerTestService" input-channel="handlerTestInputChannel">
|
||||
<beans:bean
|
||||
<beans:bean id="innerHandler"
|
||||
class="org.springframework.integration.handler.ServiceActivatorDefaultFrameworkMethodTests$TestMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<beans:beans xmlns="http://www.springframework.org/schema/integration"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xmlns:beans="http://www.springframework.org/schema/beans"
|
||||
xsi:schemaLocation="http://www.springframework.org/schema/beans
|
||||
http://www.springframework.org/schema/beans/spring-beans.xsd
|
||||
http://www.springframework.org/schema/integration
|
||||
http://www.springframework.org/schema/integration/spring-integration.xsd">
|
||||
|
||||
<service-activator id="optimizedRefReplyingHandlerTestService1"
|
||||
input-channel="optimizedRefReplyingHandlerTestInputChannel" ref="testReplyingMessageHandler"/>
|
||||
|
||||
<service-activator id="optimizedRefReplyingHandlerTestService2"
|
||||
input-channel="optimizedRefReplyingHandlerTestInputChannel" ref="testReplyingMessageHandler"/>
|
||||
|
||||
<beans:bean id="testReplyingMessageHandler"
|
||||
class="org.springframework.integration.handler.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
|
||||
</beans:beans>
|
||||
@@ -18,11 +18,16 @@ package org.springframework.integration.handler;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertSame;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import org.hamcrest.Matchers;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
|
||||
import org.springframework.beans.factory.BeanCreationException;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.context.support.ClassPathXmlApplicationContext;
|
||||
import org.springframework.integration.Message;
|
||||
import org.springframework.integration.MessageChannel;
|
||||
import org.springframework.integration.channel.QueueChannel;
|
||||
@@ -149,6 +154,22 @@ public class ServiceActivatorDefaultFrameworkMethodTests {
|
||||
assertEquals("processorTestInputChannel,processorTestService", reply.getHeaders().get("history").toString());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailOnDoubleReference() {
|
||||
try {
|
||||
new ClassPathXmlApplicationContext(this.getClass().getSimpleName() + "-fail-context.xml",
|
||||
this.getClass());
|
||||
fail("Expected exception due to 2 endpoints referencing the same bean");
|
||||
}
|
||||
catch (Exception e) {
|
||||
assertThat(e, Matchers.instanceOf(BeanCreationException.class));
|
||||
assertThat(e.getCause(), Matchers.instanceOf(BeanCreationException.class));
|
||||
assertThat(e.getCause().getCause(), Matchers.instanceOf(IllegalArgumentException.class));
|
||||
assertThat(e.getCause().getCause().getMessage(),
|
||||
Matchers.containsString("An AbstractReplyProducingMessageHandler may only be referenced once"));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@SuppressWarnings("unused")
|
||||
private static class TestReplyingMessageHandler extends AbstractReplyProducingMessageHandler {
|
||||
|
||||
@@ -27,6 +27,7 @@ import java.util.concurrent.ExecutorService;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicLong;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.concurrent.locks.ReentrantLock;
|
||||
|
||||
import javax.management.DynamicMBean;
|
||||
@@ -80,6 +81,8 @@ import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.PatternMatchUtils;
|
||||
import org.springframework.util.ReflectionUtils;
|
||||
import org.springframework.util.ReflectionUtils.FieldCallback;
|
||||
import org.springframework.util.ReflectionUtils.FieldFilter;
|
||||
|
||||
/**
|
||||
* <p>
|
||||
@@ -106,6 +109,7 @@ import org.springframework.util.ReflectionUtils;
|
||||
* @author Helena Edelson
|
||||
* @author Oleg Zhurakousky
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
*/
|
||||
@ManagedResource
|
||||
public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostProcessor, BeanFactoryAware,
|
||||
@@ -217,7 +221,7 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP
|
||||
@Override
|
||||
public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
|
||||
super.setBeanFactory(beanFactory);
|
||||
Assert.isTrue(beanFactory instanceof ListableBeanFactory, "A ListableBeanFactory is required.");
|
||||
Assert.isInstanceOf(ListableBeanFactory.class, beanFactory, "A ListableBeanFactory is required.");
|
||||
this.beanFactory = (ListableBeanFactory) beanFactory;
|
||||
}
|
||||
|
||||
@@ -246,6 +250,12 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP
|
||||
}
|
||||
|
||||
if (bean instanceof MessageHandler) {
|
||||
if (this.handlerInAnonymousWrapper(bean) != null) {
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("Skipping " + beanName + " because it wraps another handler");
|
||||
}
|
||||
return bean;
|
||||
}
|
||||
SimpleMessageHandlerMetrics monitor = new SimpleMessageHandlerMetrics((MessageHandler) bean);
|
||||
Object advised = applyHandlerInterceptor(bean, monitor, beanClassLoader);
|
||||
handlers.add(monitor);
|
||||
@@ -282,6 +292,33 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP
|
||||
|
||||
}
|
||||
|
||||
private MessageHandler handlerInAnonymousWrapper(final Object bean) {
|
||||
if (bean != null && bean.getClass().isAnonymousClass()) {
|
||||
final AtomicReference<MessageHandler> wrapped = new AtomicReference<MessageHandler>();
|
||||
ReflectionUtils.doWithFields(bean.getClass(), new FieldCallback() {
|
||||
|
||||
@Override
|
||||
public void doWith(Field field) throws IllegalArgumentException, IllegalAccessException {
|
||||
field.setAccessible(true);
|
||||
Object handler = field.get(bean);
|
||||
if (handler instanceof MessageHandler) {
|
||||
wrapped.set((MessageHandler) handler);
|
||||
}
|
||||
}
|
||||
}, new FieldFilter() {
|
||||
|
||||
@Override
|
||||
public boolean matches(Field field) {
|
||||
return wrapped.get() == null && field.getName().startsWith("val$");
|
||||
}
|
||||
});
|
||||
return wrapped.get();
|
||||
}
|
||||
else {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Copy of private method in super class. Needed so we can avoid using the bean factory to extract the bean again,
|
||||
* and risk it being a proxy (which it almost certainly is by now).
|
||||
@@ -987,20 +1024,22 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP
|
||||
String source = "endpoint";
|
||||
Object endpoint = null;
|
||||
|
||||
MessageHandler messageHandler = monitor.getMessageHandler();
|
||||
|
||||
for (String beanName : names) {
|
||||
endpoint = beanFactory.getBean(beanName);
|
||||
Object field = null;
|
||||
try {
|
||||
field = extractTarget(getField(endpoint, "handler"));
|
||||
Object field = extractTarget(getField(endpoint, "handler"));
|
||||
if (field == messageHandler ||
|
||||
this.extractTarget(this.handlerInAnonymousWrapper(field)) == messageHandler) {
|
||||
name = beanName;
|
||||
endpointName = beanName;
|
||||
break;
|
||||
}
|
||||
}
|
||||
catch (Exception e) {
|
||||
logger.trace("Could not get handler from bean = " + beanName);
|
||||
}
|
||||
if (field == monitor.getMessageHandler()) {
|
||||
name = beanName;
|
||||
endpointName = beanName;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (name != null && endpoint != null && name.startsWith("_org.springframework.integration")) {
|
||||
name = getInternalComponentName(name);
|
||||
@@ -1044,11 +1083,11 @@ public class IntegrationMBeanExporter extends MBeanExporter implements BeanPostP
|
||||
}
|
||||
|
||||
if (name == null) {
|
||||
if (monitor.getMessageHandler() instanceof NamedComponent) {
|
||||
name = ((NamedComponent) monitor.getMessageHandler()).getComponentName();
|
||||
if (messageHandler instanceof NamedComponent) {
|
||||
name = ((NamedComponent) messageHandler).getComponentName();
|
||||
}
|
||||
if (name == null) {
|
||||
name = monitor.getMessageHandler().toString();
|
||||
name = messageHandler.toString();
|
||||
}
|
||||
source = "handler";
|
||||
}
|
||||
|
||||
@@ -27,19 +27,19 @@
|
||||
<service-activator id="replyingHandlerWithStandardMethodTestService"
|
||||
input-channel="replyingHandlerWithStandardMethodTestInputChannel"
|
||||
method="handleMessage">
|
||||
<beans:bean
|
||||
<beans:bean id="innerReplyingHandler"
|
||||
class="org.springframework.integration.jmx.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
<service-activator id="replyingHandlerWithOtherMethodTestService"
|
||||
input-channel="replyingHandlerWithOtherMethodTestInputChannel"
|
||||
method="foo">
|
||||
<beans:bean
|
||||
<beans:bean id="innerReplyingHandlerFoo"
|
||||
class="org.springframework.integration.jmx.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
<service-activator id="handlerTestService" input-channel="handlerTestInputChannel">
|
||||
<beans:bean
|
||||
<beans:bean id="innerHandler"
|
||||
class="org.springframework.integration.jmx.ServiceActivatorDefaultFrameworkMethodTests$TestMessageHandler"/>
|
||||
</service-activator>
|
||||
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
<?xml version="1.0" encoding="UTF-8"?>
|
||||
<beans:beans xmlns="http://www.springframework.org/schema/integration"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xmlns:beans="http://www.springframework.org/schema/beans"
|
||||
xmlns:context="http://www.springframework.org/schema/context"
|
||||
xmlns:int-jmx="http://www.springframework.org/schema/integration/jmx"
|
||||
xsi:schemaLocation="http://www.springframework.org/schema/integration http://www.springframework.org/schema/integration/spring-integration.xsd
|
||||
http://www.springframework.org/schema/integration/jmx http://www.springframework.org/schema/integration/jmx/spring-integration-jmx.xsd
|
||||
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
|
||||
http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context.xsd">
|
||||
|
||||
<context:mbean-server/>
|
||||
<int-jmx:mbean-export default-domain="test2"/>
|
||||
|
||||
<service-activator id="optimizedRefReplyingHandlerTestService1"
|
||||
input-channel="optimizedRefReplyingHandlerTestInputChannel" ref="testReplyingMessageHandler"/>
|
||||
|
||||
<service-activator id="optimizedRefReplyingHandlerTestService2"
|
||||
input-channel="optimizedRefReplyingHandlerTestInputChannel" ref="testReplyingMessageHandler"/>
|
||||
|
||||
<beans:bean id="testReplyingMessageHandler"
|
||||
class="org.springframework.integration.jmx.ServiceActivatorDefaultFrameworkMethodTests$TestReplyingMessageHandler"/>
|
||||
|
||||
</beans:beans>
|
||||
@@ -18,11 +18,16 @@ package org.springframework.integration.jmx;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertSame;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import org.hamcrest.Matchers;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
|
||||
import org.springframework.beans.factory.BeanCreationException;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.context.support.ClassPathXmlApplicationContext;
|
||||
import org.springframework.integration.Message;
|
||||
import org.springframework.integration.MessageChannel;
|
||||
import org.springframework.integration.channel.QueueChannel;
|
||||
@@ -151,6 +156,23 @@ public class ServiceActivatorDefaultFrameworkMethodTests {
|
||||
assertEquals("processorTestInputChannel,processorTestService", reply.getHeaders().get("history").toString());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailOnDoubleReference() {
|
||||
try {
|
||||
new ClassPathXmlApplicationContext(this.getClass().getSimpleName() + "-fail-context.xml",
|
||||
this.getClass());
|
||||
fail("Expected exception due to 2 endpoints referencing the same bean");
|
||||
}
|
||||
catch (Exception e) {
|
||||
assertThat(e, Matchers.instanceOf(BeanCreationException.class));
|
||||
assertThat(e.getCause(), Matchers.instanceOf(BeanCreationException.class));
|
||||
assertThat(e.getCause().getCause(), Matchers.instanceOf(IllegalArgumentException.class));
|
||||
assertThat(e.getCause().getCause().getMessage(),
|
||||
Matchers.containsString("An AbstractReplyProducingMessageHandler may only be referenced once"));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private interface Foo {
|
||||
|
||||
public String foo(String in);
|
||||
@@ -181,7 +203,7 @@ public class ServiceActivatorDefaultFrameworkMethodTests {
|
||||
public void handleMessage(Message<?> requestMessage) {
|
||||
Exception e = new RuntimeException();
|
||||
StackTraceElement[] st = e.getStackTrace();
|
||||
assertEquals("doDispatch", st[28].getMethodName());
|
||||
assertEquals("doDispatch", st[16].getMethodName());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user