You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by dm...@apache.org on 2016/12/23 10:08:41 UTC
aurora git commit: Expose ResponseCode stats on Thrift server calls
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 <me...@nurolahzade.com>
Authored: Fri Dec 23 02:08:29 2016 -0800
Committer: David McLaughlin <dm...@twitter.com>
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());
}
}