Merge pull request #1280 from emeroad/#1276_type_null_checking

#1276 Tighten up type checking of Interceptor
pinpoint-apm · Dec 1, 2015 · 5ee7367 · 5ee7367
2 parents 1c33a6f + 7c01e33
commit 5ee7367
Show file tree

Hide file tree

Showing 14 changed files with 49 additions and 25 deletions.
diff --git a/...arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/AddOpInterceptor.java b/...arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/AddOpInterceptor.java
@@ -37,9 +37,11 @@ public void before(Object target, Object[] args) {
         if (isDebug) {
             logger.beforeInterceptor(target, args);
         }
-
-        String serviceCode = ((ServiceCodeAccessor)target)._$PINPOINT$_getServiceCode();
-        ((ServiceCodeAccessor)args[1])._$PINPOINT$_setServiceCode(serviceCode);
+        final Object serviceCodeAccessor = args[1];
+        if (target instanceof ServiceCodeAccessor && serviceCodeAccessor instanceof ServiceCodeAccessor) {
+            String serviceCode = ((ServiceCodeAccessor) target)._$PINPOINT$_getServiceCode();
+            ((ServiceCodeAccessor) serviceCodeAccessor)._$PINPOINT$_setServiceCode(serviceCode);
+        }
     }
 
     @Override

diff --git a/...ava/com/navercorp/pinpoint/plugin/arcus/interceptor/CacheManagerConstructInterceptor.java b/...ava/com/navercorp/pinpoint/plugin/arcus/interceptor/CacheManagerConstructInterceptor.java
@@ -41,7 +41,8 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         if (isDebug) {
             logger.afterInterceptor(target, args, result, throwable);
         }
-
-        ((ServiceCodeAccessor)target)._$PINPOINT$_setServiceCode((String)args[1]);
+        if (target instanceof ServiceCodeAccessor) {
+            ((ServiceCodeAccessor) target)._$PINPOINT$_setServiceCode((String) args[1]);
+        }
     }
 }
diff --git a/...va/com/navercorp/pinpoint/plugin/arcus/interceptor/FrontCacheGetFutureGetInterceptor.java b/...va/com/navercorp/pinpoint/plugin/arcus/interceptor/FrontCacheGetFutureGetInterceptor.java
@@ -70,9 +70,11 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         try {
             final SpanEventRecorder recorder = trace.currentSpanEventRecorder();
             recorder.recordApi(methodDescriptor);
-            String cacheName = ((CacheNameAccessor)target)._$PINPOINT$_getCacheName();
-            if (cacheName != null) {
-                recorder.recordDestinationId(cacheName);
+            if (target instanceof CacheNameAccessor) {
+                final String cacheName = ((CacheNameAccessor) target)._$PINPOINT$_getCacheName();
+                if (cacheName != null) {
+                    recorder.recordDestinationId(cacheName);
+                }
             }
 
             recorder.recordServiceType(ArcusConstants.ARCUS_EHCACHE_FUTURE_GET);

diff --git a/...lugin/httpclient4/interceptor/DefaultClientExchangeHandlerImplStartMethodInterceptor.java b/...lugin/httpclient4/interceptor/DefaultClientExchangeHandlerImplStartMethodInterceptor.java
@@ -152,6 +152,7 @@ public void before(Object target, Object[] args) {
                 // set asynchronous trace
                 final AsyncTraceId asyncTraceId = trace.getAsyncTraceId();
                 recorder.recordNextAsyncId(asyncTraceId.getAsyncId());
+                // check type isAsynchronousInvocation()
                 ((AsyncTraceIdAccessor)((ResultFutureGetter)target)._$PINPOINT$_getResultFuture())._$PINPOINT$_setAsyncTraceId(asyncTraceId);
                 if (isDebug) {
                     logger.debug("Set asyncTraceId metadata {}", asyncTraceId);

diff --git a/...t4/interceptor/HttpRequestExecutorDoSendRequestAndDoReceiveResponseMethodInterceptor.java b/...t4/interceptor/HttpRequestExecutorDoSendRequestAndDoReceiveResponseMethodInterceptor.java
@@ -62,6 +62,7 @@ public void before(Object target, Object[] args) {
 
         InterceptorScopeInvocation invocation = interceptorScope.getCurrentInvocation();
         if(invocation != null && invocation.getAttachment() != null) {
+            // TODO type check
             HttpCallContext callContext = (HttpCallContext) invocation.getAttachment();
             if(methodDescriptor.getMethodName().equals("doSendRequest")) {
                 callContext.setWriteBeginTime(System.currentTimeMillis());
@@ -85,6 +86,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
 
         InterceptorScopeInvocation invocation = interceptorScope.getCurrentInvocation();
         if(invocation != null && invocation.getAttachment() != null) {
+            // TODO type check
             HttpCallContext callContext = (HttpCallContext) invocation.getAttachment();
             if(methodDescriptor.getMethodName().equals("doSendRequest")) {
                 callContext.setWriteEndTime(System.currentTimeMillis());

diff --git a/...rc/main/java/com/navercorp/pinpoint/plugin/jetty/interceptor/ServerHandleInterceptor.java b/...rc/main/java/com/navercorp/pinpoint/plugin/jetty/interceptor/ServerHandleInterceptor.java
@@ -19,7 +19,11 @@ public ServerHandleInterceptor(TraceContext traceContext, MethodDescriptor descr
 
     @Override
     protected Request getRequest(Object[] args) {
-        final HttpChannel<?> channel = (HttpChannel<?>) args[0];
+        final Object httpChannelObject = args[0];
+        if (!(httpChannelObject instanceof HttpChannel)) {
+           return null;
+        }
+        final HttpChannel<?> channel = (HttpChannel<?>) httpChannelObject;
         final Request request = channel.getRequest();
         return request;
     }

diff --git a/.../src/main/java/com/navercorp/pinpoint/plugin/json_lib/interceptor/ParsingInterceptor.java b/.../src/main/java/com/navercorp/pinpoint/plugin/json_lib/interceptor/ParsingInterceptor.java
@@ -69,8 +69,7 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             recorder.recordServiceType(JsonLibConstants.SERVICE_TYPE);
             recorder.recordApi(descriptor);
             recorder.recordException(throwable);
-
-            if (args.length > 0 && args[0] instanceof String) {
+            if (args != null && args.length > 0 && args[0] instanceof String) {
                 recorder.recordAttribute(JsonLibConstants.JSON_LIB_ANNOTATION_KEY_JSON_LENGTH, ((String) args[0]).length());
             }
         } finally {

diff --git a/.../plugin/okhttp/interceptor/RequestBuilderBuildMethodBackwardCompatibilityInterceptor.java b/.../plugin/okhttp/interceptor/RequestBuilderBuildMethodBackwardCompatibilityInterceptor.java
@@ -60,6 +60,9 @@ public void before(Object target, Object[] args) {
         }
 
         try {
+            if (!(target instanceof Request.Builder)) {
+                return;
+            }
             final Request.Builder builder = ((Request.Builder) target);
             if (!trace.canSampled()) {
                 if (isDebug) {

diff --git a/...om/navercorp/pinpoint/plugin/okhttp/interceptor/RequestBuilderBuildMethodInterceptor.java b/...om/navercorp/pinpoint/plugin/okhttp/interceptor/RequestBuilderBuildMethodInterceptor.java
@@ -58,6 +58,9 @@ public void before(Object target, Object[] args) {
         }
 
         try {
+            if(!(target instanceof Request.Builder)) {
+                return;
+            }
             final Request.Builder builder = ((Request.Builder) target);
             if (!trace.canSampled()) {
                 if (isDebug) {

diff --git a/...rp/pinpoint/plugin/jdbc/postgresql/interceptor/PostgreSQLConnectionCreateInterceptor.java b/...rp/pinpoint/plugin/jdbc/postgresql/interceptor/PostgreSQLConnectionCreateInterceptor.java
@@ -56,9 +56,6 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             return;
         }
 
-        for(Object o:args) {
-            logger.info("test: "+o.toString());
-        }
 
         Properties properties = getProperties(args[3]);
 

diff --git a/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor1.java b/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor1.java
@@ -39,8 +39,11 @@ public PostgreSqlPreparedStatementCreateInterceptor1(TraceContext context, Metho
 
     @Override
     public void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args)  {
-        DatabaseInfo databaseInfo = (target instanceof DatabaseInfoAccessor) ? ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo() : null;
-
+        DatabaseInfo databaseInfo = null;
+        if (target instanceof DatabaseInfoAccessor) {
+            databaseInfo = ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo();
+        }
+
         if (databaseInfo == null) {
             databaseInfo = UnKnownDatabaseInfo.INSTANCE;
         }
@@ -56,7 +59,7 @@ protected void prepareAfterTrace(Object target, Object[] args, Object result, Th
         if (success) {
             if (target instanceof DatabaseInfoAccessor) {
                 // set databaseInfo to PreparedStatement only when preparedStatement is generated successfully.
-                DatabaseInfo databaseInfo = ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo();
+                final DatabaseInfo databaseInfo = ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo();
                 if (databaseInfo != null) {
                     if (result instanceof DatabaseInfoAccessor) {
                         ((DatabaseInfoAccessor)result)._$PINPOINT$_setDatabaseInfo(databaseInfo);
@@ -67,7 +70,7 @@ protected void prepareAfterTrace(Object target, Object[] args, Object result, Th
                 // 1. Don't check traceContext. preparedStatement can be created in other thread.
                 // 2. While sampling is active, the thread which creates preparedStatement could not be a sampling target. So record sql anyway. 
                 String sql = (String) args[0];
-                ParsingResult parsingResult = traceContext.parseSql(sql);
+                final ParsingResult parsingResult = traceContext.parseSql(sql);
                 if (parsingResult != null) {
                     ((ParsingResultAccessor)result)._$PINPOINT$_setParsingResult(parsingResult);
                 } else {

diff --git a/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor2.java b/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor2.java
@@ -42,8 +42,11 @@ public PostgreSqlPreparedStatementCreateInterceptor2(TraceContext context, Metho
 
     @Override
     public void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args)  {
-        DatabaseInfo databaseInfo = (target instanceof DatabaseInfoAccessor) ? ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo() : null;
-
+        DatabaseInfo databaseInfo = null;
+        if (target instanceof DatabaseInfoAccessor) {
+            databaseInfo = ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo();
+        }
+
         if (databaseInfo == null) {
             databaseInfo = UnKnownDatabaseInfo.INSTANCE;
         }

diff --git a/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor3.java b/...int/plugin/jdbc/postgresql/interceptor/PostgreSqlPreparedStatementCreateInterceptor3.java
@@ -39,8 +39,11 @@ public PostgreSqlPreparedStatementCreateInterceptor3(TraceContext context, Metho
 
     @Override
     public void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args)  {
-        DatabaseInfo databaseInfo = (target instanceof DatabaseInfoAccessor) ? ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo() : null;
-
+        DatabaseInfo databaseInfo = null;
+        if (target instanceof DatabaseInfoAccessor) {
+            databaseInfo = ((DatabaseInfoAccessor)target)._$PINPOINT$_getDatabaseInfo();
+        }
+
         if (databaseInfo == null) {
             databaseInfo = UnKnownDatabaseInfo.INSTANCE;
         }

diff --git a/...java/com/navercorp/pinpoint/plugin/tomcat/interceptor/ConnectorInitializeInterceptor.java b/...java/com/navercorp/pinpoint/plugin/tomcat/interceptor/ConnectorInitializeInterceptor.java
@@ -47,8 +47,9 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         if (isDebug) {
             logger.afterInterceptor(target, args, result, throwable);
         }
-
-        Connector connector = (Connector)target;
-        this.traceContext.getServerMetaDataHolder().addConnector(connector.getProtocol(), connector.getPort());
+        if (target instanceof Connector) {
+            final Connector connector = (Connector) target;
+            this.traceContext.getServerMetaDataHolder().addConnector(connector.getProtocol(), connector.getPort());
+        }
     }
 }
-Original file line number
+Diff line change
@@ Expand Up @@
                 return;
             }
-            for(Object o:args) {
-                logger.info("test: "+o.toString());
-            }
             Properties properties = getProperties(args[3]);
@@ Expand Down @@