Repository: aurora Updated Branches: refs/heads/master 436a2adb8 -> e2ca371c9
Expose ResponseCode stats on Thrift server calls Bugs closed: AURORA-1848 Reviewed at https://reviews.apache.org/r/55003/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/e2ca371c Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/e2ca371c Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/e2ca371c Branch: refs/heads/master Commit: e2ca371c93fc0dd9262c0d0e1a1a9847476445b2 Parents: 436a2ad Author: Mehrdad Nurolahzade <[email protected]> Authored: Fri Dec 23 02:08:29 2016 -0800 Committer: David McLaughlin <[email protected]> Committed: Fri Dec 23 02:08:29 2016 -0800 ---------------------------------------------------------------------- .../thrift/aop/LoggingInterceptor.java | 29 ++++++++++++++++++-- .../thrift/aop/LoggingInterceptorTest.java | 17 ++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/e2ca371c/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java index fc89c81..7621fac 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java @@ -15,17 +15,23 @@ package org.apache.aurora.scheduler.thrift.aop; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import com.google.common.base.Function; import com.google.common.base.Throwables; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.apache.aurora.common.stats.Stats; import org.apache.aurora.gen.ExecutorConfig; import org.apache.aurora.gen.JobConfiguration; import org.apache.aurora.gen.JobUpdateRequest; +import org.apache.aurora.gen.Response; import org.apache.aurora.gen.ResponseCode; import org.apache.aurora.scheduler.storage.Storage; import org.apache.aurora.scheduler.thrift.Responses; @@ -62,6 +68,15 @@ class LoggingInterceptor implements MethodInterceptor { } ); + private final LoadingCache<ResponseCode, AtomicLong> responseCodeCounters = + CacheBuilder.newBuilder() + .build(new CacheLoader<ResponseCode, AtomicLong>() { + @Override + public AtomicLong load(ResponseCode code) throws Exception { + return Stats.exportLong("scheduler_thrift_response_" + code.name()); + } + }); + @Override public Object invoke(MethodInvocation invocation) throws Throwable { List<String> argStrings = Lists.newArrayList(); @@ -76,17 +91,25 @@ class LoggingInterceptor implements MethodInterceptor { String methodName = invocation.getMethod().getName(); String messageArgs = String.join(", ", argStrings); LOG.info("{}({})", methodName, messageArgs); + + Response response = null; try { - return invocation.proceed(); + // casting is safe, interception happens on methods that return Response or its subclasses + response = (Response) invocation.proceed(); } catch (Storage.TransientStorageException e) { LOG.warn("Uncaught transient exception while handling {}({})", methodName, messageArgs, e); - return Responses.addMessage(Responses.empty(), ResponseCode.ERROR_TRANSIENT, e); + response = Responses.addMessage(Responses.empty(), ResponseCode.ERROR_TRANSIENT, e); } catch (RuntimeException e) { // We need shiro's exceptions to bubble up to the Shiro servlet filter so we intentionally // do not swallow them here. Throwables.throwIfInstanceOf(e, ShiroException.class); LOG.warn("Uncaught exception while handling {}({})", methodName, messageArgs, e); - return Responses.addMessage(Responses.empty(), ResponseCode.ERROR, e); + response = Responses.addMessage(Responses.empty(), ResponseCode.ERROR, e); + } finally { + if (response != null) { + responseCodeCounters.getUnchecked(response.getResponseCode()).incrementAndGet(); + } } + return response; } } http://git-wip-us.apache.org/repos/asf/aurora/blob/e2ca371c/src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java index 8dff558..b61b865 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptorTest.java @@ -18,6 +18,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.aopalliance.intercept.MethodInvocation; +import org.apache.aurora.common.stats.Stats; import org.apache.aurora.common.testing.easymock.EasyMockTest; import org.apache.aurora.gen.JobConfiguration; import org.apache.aurora.gen.Response; @@ -29,6 +30,7 @@ import org.junit.Test; import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -70,9 +72,11 @@ public class LoggingInterceptorTest extends EasyMockTest { control.replay(); + assertNull(Stats.getVariable("scheduler_thrift_response_ERROR_TRANSIENT")); assertEquals( ResponseCode.ERROR_TRANSIENT, ((Response) loggingInterceptor.invoke(methodInvocation)).getResponseCode()); + assertEquals(1L, Stats.getVariable("scheduler_thrift_response_ERROR_TRANSIENT").read()); } @Test @@ -84,14 +88,17 @@ public class LoggingInterceptorTest extends EasyMockTest { control.replay(); + assertNull(Stats.getVariable("scheduler_thrift_response_ERROR")); assertEquals( ResponseCode.ERROR, ((Response) loggingInterceptor.invoke(methodInvocation)).getResponseCode()); + assertEquals(1L, Stats.getVariable("scheduler_thrift_response_ERROR").read()); } @Test public void testInvokePrintsArgs() throws Throwable { Response response = new Response(); + response.setResponseCode(ResponseCode.OK); expect(methodInvocation.getMethod()) .andReturn(InterceptedClass.class.getDeclaredMethod("respond", Object.class, Object.class)); @@ -110,13 +117,17 @@ public class LoggingInterceptorTest extends EasyMockTest { control.replay(); + assertNull(Stats.getVariable("scheduler_thrift_response_OK")); assertSame(response, loggingInterceptor.invoke(methodInvocation)); assertTrue(calledAtLeastOnce.get()); + assertEquals(1L, Stats.getVariable("scheduler_thrift_response_OK").read()); } @Test public void testInvokePrintsBlankedJobConfiguration() throws Throwable { Response response = new Response(); + response.setResponseCode(ResponseCode.INVALID_REQUEST); + expect(methodInvocation.getMethod()) .andReturn(InterceptedClass.class.getDeclaredMethod("respond", JobConfiguration.class)); expect(methodInvocation.getArguments()) @@ -125,12 +136,16 @@ public class LoggingInterceptorTest extends EasyMockTest { control.replay(); + assertNull(Stats.getVariable("scheduler_thrift_response_INVALID_REQUEST")); assertSame(response, loggingInterceptor.invoke(methodInvocation)); + assertEquals(1L, Stats.getVariable("scheduler_thrift_response_INVALID_REQUEST").read()); } @Test public void testInvokePrintsJobConfiguration() throws Throwable { Response response = new Response(); + response.setResponseCode(ResponseCode.WARNING); + expect(methodInvocation.getMethod()) .andReturn(InterceptedClass.class.getDeclaredMethod("respond", JobConfiguration.class)); expect(methodInvocation.getArguments()).andReturn(new Object[] {new JobConfiguration()}); @@ -138,6 +153,8 @@ public class LoggingInterceptorTest extends EasyMockTest { control.replay(); + assertNull(Stats.getVariable("scheduler_thrift_response_WARNING")); assertSame(response, loggingInterceptor.invoke(methodInvocation)); + assertEquals(1L, Stats.getVariable("scheduler_thrift_response_WARNING").read()); } }
