Make the agent not log stacktraces if introspected method not found

CJSCommonPlatform · Nov 23, 2016 · f5babb4 · f5babb4
1 parent 4bdb5e8
commit f5babb4
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 63 deletions.
diff --git a/...wildfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/jms/WildflyJmsAgentHelper.java b/...wildfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/jms/WildflyJmsAgentHelper.java
@@ -8,6 +8,8 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Optional;
+import java.util.function.BiConsumer;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -17,27 +19,23 @@ public class WildflyJmsAgentHelper {
     public static final String INTROSPECTION_ERROR = "Introspection error";
 
     public void onEntry(Object interceptor, Object interceptorContext) {
-        if (firstInterceptorInChain(interceptor)) {
-
-            try {
-                final Object coreMessage = coreMessageFrom(interceptorContext);
-                final TimerContext timerContext = timerContextOf(coreMessage);
-                final Object messageId = messageIdOf(coreMessage);
-                timerContext.startTimer(messageId);
-            } catch (ReflectiveOperationException e) {
-                LOGGER.error(INTROSPECTION_ERROR, e);
-            }
-        }
+        collectMetrics(interceptor, interceptorContext, TimerContext::startTimer);
     }
 
     public void onExit(Object interceptor, Object interceptorContext) {
+        collectMetrics(interceptor, interceptorContext, TimerContext::stopTimer);
+    }
+
+    private void collectMetrics(final Object interceptor, final Object interceptorContext, final BiConsumer<TimerContext, Object> timerContextOperation) {
         if (firstInterceptorInChain(interceptor)) {
             try {
-                final Object coreMessage = coreMessageFrom(interceptorContext);
-                final Object messageId = messageIdOf(coreMessage);
-                final TimerContext timerContext = timerContextOf(coreMessage);
-                timerContext.stopTimer(messageId);
-            } catch (ReflectiveOperationException e) {
+                final Optional<Object> coreMessage = coreMessageFrom(interceptorContext);
+                if (coreMessage.isPresent()) {
+                    final Object messageId = messageIdOf(coreMessage.get()).get();
+                    final TimerContext timerContext = timerContextOf(coreMessage.get());
+                    timerContextOperation.accept(timerContext, messageId);
+                }
+            } catch (Exception e) {
                 LOGGER.error(INTROSPECTION_ERROR, e);
             }
         }
@@ -47,18 +45,18 @@ private boolean firstInterceptorInChain(final Object interceptor) {
         return interceptor.getClass().getName().contains("ViewDescription");
     }
 
-    private Object messageIdOf(final Object coreMessage) throws ReflectiveOperationException {
+    private Optional<Object> messageIdOf(final Object coreMessage) throws ReflectiveOperationException {
         return invokeMethod(coreMessage, "getMessageID");
     }
 
     private TimerContext timerContextOf(final Object coreMessage) throws ReflectiveOperationException {
-        final Object address = invokeMethod(coreMessage, "getAddress");
-        final Object action = invokeMethod(coreMessage, "getStringProperty", "CPPNAME");
+        final Object address = invokeMethod(coreMessage, "getAddress").get();
+        final Object action = invokeMethod(coreMessage, "getStringProperty", "CPPNAME").get();
         return WildflyJmsMetricsTimerContextFactory.timerContextOf(address, action);
     }
 
-    private Object coreMessageFrom(final Object interceptorContext) throws ReflectiveOperationException {
-        final Object parameters = invokeMethod(interceptorContext, "getParameters");
+    private Optional<Object> coreMessageFrom(final Object interceptorContext) throws ReflectiveOperationException {
+        final Object parameters = invokeMethod(interceptorContext, "getParameters").get();
         Object amqMessage = ((Object[]) parameters)[0];
         return invokeMethod(amqMessage, "getCoreMessage");
     }

diff --git a/...ldfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestAgentHelper.java b/...ldfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestAgentHelper.java
@@ -4,6 +4,8 @@
 import static uk.gov.justice.metrics.agent.wildfly.rest.WildflyRestMetricsTimerContextFactory.timerContextOf;
 import static uk.gov.justice.metrics.agent.wildfly.util.ReflectionUtil.invokeMethod;
 
+import java.util.Optional;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -32,7 +34,8 @@ public void onExit(final Object serverExchange) {
     }
 
 
-    private String requestPathFrom(final Object serverExchange) throws ReflectiveOperationException {
-        return (String) invokeMethod(serverExchange, "getRequestPath");
+    private Optional<String> requestPathFrom(final Object serverExchange) throws ReflectiveOperationException {
+        final Optional<Object> requestPath = invokeMethod(serverExchange, "getRequestPath");
+        return requestPath.isPresent() ? Optional.of((String) requestPath.get()) : Optional.empty();
     }
 }
diff --git a/...java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestMetricsTimerContextFactory.java b/...java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestMetricsTimerContextFactory.java
@@ -6,6 +6,8 @@
 import uk.gov.justice.metrics.agent.artemis.agent.common.TimerContext;
 import uk.gov.justice.metrics.agent.artemis.agent.common.BaseTimeContextFactory;
 
+import java.util.Optional;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -18,9 +20,13 @@ public class WildflyRestMetricsTimerContextFactory {
     private static final EmptyTimerContext EMPTY_TIMER_CONTEXT = new EmptyTimerContext();
 
 
-    public static TimerContext timerContextOf(final String requestURL) {
-        LOGGER.trace("Fetching timer context for rest request: {}", requestURL);
-        return requestURL != null ? BASE_TIME_CONTEXT_FACTORY.timerContextOf(timerContextNameFrom(requestURL)) : EMPTY_TIMER_CONTEXT;
+    public static TimerContext timerContextOf(final Optional<String> requestURL) {
+        if (requestURL.isPresent()) {
+            LOGGER.trace("Fetching timer context for rest request: {}", requestURL.get());
+            return BASE_TIME_CONTEXT_FACTORY.timerContextOf(timerContextNameFrom(requestURL.get()));
+        } else {
+            return EMPTY_TIMER_CONTEXT;
+        }
     }
 
     static String timerContextNameFrom(final String requestURL) {

diff --git a/...agent-wildfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/util/ReflectionUtil.java b/...agent-wildfly/src/main/java/uk/gov/justice/metrics/agent/wildfly/util/ReflectionUtil.java
@@ -1,27 +1,30 @@
 package uk.gov.justice.metrics.agent.wildfly.util;
 
 
+import static java.util.Optional.empty;
+
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Optional;
 
 public class ReflectionUtil {
-    public static Object invokeMethod(final Object obj, final String methodName, final Object... param) throws ReflectiveOperationException {
+    public static Optional<Object> invokeMethod(final Object obj, final String methodName, final Object... param) throws ReflectiveOperationException {
         Class<?> clazz = obj.getClass();
         for (int i = 0; i < 3; i++) {
             try {
                 final Method method = param.length == 0 ? clazz.getDeclaredMethod(methodName) : clazz.getDeclaredMethod(methodName, param[0].getClass());
-                return method.invoke(obj, param);
+                return Optional.ofNullable(method.invoke(obj, param));
             } catch (IllegalAccessException | InvocationTargetException e) {
                 throw e;
             } catch (NoSuchMethodException e) {
                 final Class<?> superclass = clazz.getSuperclass();
                 if (superclass != null) {
                     clazz = superclass;
                 } else {
-                    throw e;
+                    return empty();
                 }
             }
         }
-        return null;
+        return empty();
     }
 }
diff --git a/...fly/src/test/java/uk/gov/justice/metrics/agent/wildfly/jms/WildflyJmsAgentHelperTest.java b/...fly/src/test/java/uk/gov/justice/metrics/agent/wildfly/jms/WildflyJmsAgentHelperTest.java
@@ -58,14 +58,22 @@ public void shouldCollectMetricsForMessages() throws Exception {
     }
 
     @Test
-    public void shouldNotCollectMetricsIfNotIntialInterceptor() {
+    public void shouldNotCollectMetricsIfActiveMQMessageNotInContext() {
+        agentHelper.onEntry(INITIAL_INTERCEPTOR, new InterceptorContext(new Object()));
+        agentHelper.onExit(INITIAL_INTERCEPTOR, new InterceptorContext(new Object()));
+        assertThat(TimerRegistry.timerOf("wildfly.jms.queue.abc-contextname.actionname").getCount(), is(0L));
+    }
+
+    @Test
+    public void shouldNotCollectMetricsIfNotInitialInterceptor() {
         final Message message = new ActiveMQTextMessage(new TestClientMessage(127, "jms.queue.abc", "contextname.actionname"), null);
         agentHelper.onEntry(new Object(), new InterceptorContext(message));
         agentHelper.onExit(new Object(), new InterceptorContext(message));
         assertThat(TimerRegistry.timerOf("wildfly.jms.queue.abc-contextname.actionname").getCount(), is(0L));
 
     }
 
+
     @Test
     public void shouldLogErrorInCaseExcpectedMethodNotFoundInPassedObject() {
         final TestAppender appender = new TestAppender();

diff --git a/...y/src/test/java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestAgentHelperTest.java b/...y/src/test/java/uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestAgentHelperTest.java
@@ -48,37 +48,6 @@ public void shouldCollectMetricsForRestRequests() throws Exception {
 
     }
 
-    @Test
-    public void shouldLogErrorIfExpectedMethodNotFoundInPassedObjectOnEntry() {
-        final TestAppender appender = new TestAppender();
-        final Logger logger = Logger.getRootLogger();
-        logger.addAppender(appender);
-
-        agentHelper.onEntry(new Object());
-
-        logger.removeAppender(appender);
-        final List<LoggingEvent> log = appender.messages();
-        final LoggingEvent logEntry = log.get(0);
-        assertThat(logEntry.getLevel(), is(Level.ERROR));
-        assertThat((String) logEntry.getMessage(), containsString("Introspection error"));
-    }
-
-    @Test
-    public void shouldLogErrorIfExpectedMethodNotFoundInPassedObjectOnExit() {
-        final TestAppender appender = new TestAppender();
-        final Logger logger = Logger.getRootLogger();
-        logger.addAppender(appender);
-
-        agentHelper.onExit(new Object());
-
-        logger.removeAppender(appender);
-        final List<LoggingEvent> log = appender.messages();
-        final LoggingEvent logEntry = log.get(0);
-        assertThat(logEntry.getLevel(), is(Level.ERROR));
-        assertThat((String) logEntry.getMessage(), containsString("Introspection error"));
-    }
-
-
     @Test
     public void shouldDoNothingWhenRequestUrlNull() {
         HttpServerExchange exchange = new HttpServerExchange(null);

diff --git a/.../uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestMetricsTimerContextFactoryTest.java b/.../uk/gov/justice/metrics/agent/wildfly/rest/WildflyRestMetricsTimerContextFactoryTest.java
@@ -8,6 +8,8 @@
 import uk.gov.justice.metrics.agent.artemis.agent.common.TimerContext;
 import uk.gov.justice.metrics.agent.artemis.agent.common.TimerRegistry;
 
+import java.util.Optional;
+
 import org.junit.Test;
 
 public class WildflyRestMetricsTimerContextFactoryTest {
@@ -49,7 +51,7 @@ public void shouldReplaceIdsWithPlaceHolders() throws Exception {
     @Test
     public void shouldCreateTimerContextForRestRequest() {
 
-        final TimerContext timerContext = timerContextOf("/example-query-view/query/view/rest/cakeshop/cakes");
+        final TimerContext timerContext = timerContextOf(Optional.of("/example-query-view/query/view/rest/cakeshop/cakes"));
         timerContext.startTimer("1");
         timerContext.stopTimer("1");
         timerContext.startTimer("2");