aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dmclaugh...@apache.org
Subject aurora git commit: Expose ResponseCode stats on Thrift server calls
Date Fri, 23 Dec 2016 10:08:41 GMT
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 <mehrdad@nurolahzade.com>
Authored: Fri Dec 23 02:08:29 2016 -0800
Committer: David McLaughlin <dmclaughlin@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());
   }
 }


Mime
View raw message