INSPECTIT-2407: Capture exception in remote sensor and report as erro…

…r in span
ivansenic · May 19, 2017 · 6d93b62 · 6d93b62
1 parent cc2e869
commit 6d93b62
Show file tree

Hide file tree

Showing 50 changed files with 864 additions and 452 deletions.
diff --git a/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/config/IPropertyAccessor.java b/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/config/IPropertyAccessor.java
@@ -30,12 +30,16 @@ public interface IPropertyAccessor {
 	 *            The method parameters (can be <code>null</code>).
 	 * @param returnValue
 	 *            The return value of the method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 * @return The {@link String} representation of the field or parameter followed by the path.
 	 * @throws PropertyAccessException
 	 *             This exception is thrown whenever something unexpectedly happens while accessing
 	 *             a property.
 	 */
-	String getPropertyContent(PropertyPathStart propertyPathStart, Object clazz, Object[] parameters, Object returnValue) throws PropertyAccessException;
+	String getPropertyContent(PropertyPathStart propertyPathStart, Object clazz, Object[] parameters, Object returnValue, boolean exception) throws PropertyAccessException;
 
 	/**
 	 * Converts the list of property accessors {@link PropertyPathStart} into a list of
@@ -49,8 +53,12 @@ public interface IPropertyAccessor {
 	 *            The parameters.
 	 * @param returnValue
 	 *            The return value of the method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 * @return The list of {@link ParameterContentData}.
 	 */
-	List<ParameterContentData> getParameterContentData(List<PropertyPathStart> propertyAccessorList, Object clazz, Object[] parameters, Object returnValue);
+	List<ParameterContentData> getParameterContentData(List<PropertyPathStart> propertyAccessorList, Object clazz, Object[] parameters, Object returnValue, boolean exception);
 
 }
diff --git a/...tit.agent.java/src/main/java/rocks/inspectit/agent/java/config/impl/PropertyAccessor.java b/...tit.agent.java/src/main/java/rocks/inspectit/agent/java/config/impl/PropertyAccessor.java
@@ -40,6 +40,11 @@ public class PropertyAccessor implements IPropertyAccessor {
 	 */
 	private static final String NULL_VALUE = "null";
 
+	/**
+	 * Static null value for return value capturing in case of method exited with exception.
+	 */
+	private static final String NA = "n/a";
+
 	/**
 	 * An array containing the names of all methods that might be called by the PropertyAccessor.
 	 * Names should not include the brackets.
@@ -50,7 +55,7 @@ public class PropertyAccessor implements IPropertyAccessor {
 	 * {@inheritDoc}
 	 */
 	@Override
-	public String getPropertyContent(PropertyPathStart propertyPathStart, Object clazz, Object[] parameters, Object returnValue) throws PropertyAccessException {
+	public String getPropertyContent(PropertyPathStart propertyPathStart, Object clazz, Object[] parameters, Object returnValue, boolean exception) throws PropertyAccessException {
 		if (null == propertyPathStart) {
 			throw new PropertyAccessException("Property path start cannot be null!");
 		}
@@ -77,9 +82,12 @@ public String getPropertyContent(PropertyPathStart propertyPathStart, Object cla
 			return getPropertyContent(propertyPathStart.getPathToContinue(), parameters[propertyPathStart.getSignaturePosition()]);
 		case RETURN:
 			// we will not throw an exception here as the return value of a method can sometimes be
-			// null. If we throw an exception, this will lead to the removal of the path and thus no
+			// null or we had exceptional method exit.
+			// If we throw an exception, this will lead to the removal of the path and thus no
 			// return value of this property accessor will be captured afterwards.
-			if (null == returnValue) {
+			if (exception) {
+				return NA;
+			} else if (null == returnValue) {
 				return NULL_VALUE;
 			} else {
 				return getPropertyContent(propertyPathStart.getPathToContinue(), returnValue);
@@ -239,11 +247,11 @@ private String getPropertyContent(PropertyPath propertyPath, Object object) thro
 	 * {@inheritDoc}
 	 */
 	@Override
-	public List<ParameterContentData> getParameterContentData(List<PropertyPathStart> propertyAccessorList, Object clazz, Object[] parameters, Object returnValue) {
+	public List<ParameterContentData> getParameterContentData(List<PropertyPathStart> propertyAccessorList, Object clazz, Object[] parameters, Object returnValue, boolean exception) {
 		List<ParameterContentData> parameterContentData = new ArrayList<ParameterContentData>();
 		for (PropertyPathStart start : propertyAccessorList) {
 			try {
-				String content = this.getPropertyContent(start, clazz, parameters, returnValue);
+				String content = this.getPropertyContent(start, clazz, parameters, returnValue, exception);
 				ParameterContentData paramContentData = new ParameterContentData();
 				paramContentData.setContent(content);
 				paramContentData.setContentType(start.getContentType());

diff --git a/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/IHookDispatcher.java b/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/IHookDispatcher.java
@@ -32,9 +32,13 @@ public interface IHookDispatcher {
 	 * @param parameters
 	 *            The parameters of the method.
 	 * @param returnValue
-	 *            The return value of the method.
+	 *            The return value of the method or exception thrown by method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 */
-	void dispatchFirstMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue);
+	void dispatchFirstMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue, boolean exception);
 
 	/**
 	 * Dispatches the second 'after' method statement.
@@ -46,9 +50,13 @@ public interface IHookDispatcher {
 	 * @param parameters
 	 *            The parameters of the method.
 	 * @param returnValue
-	 *            The return value of the method.
+	 *            The return value of the method or exception thrown by method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 */
-	void dispatchSecondMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue);
+	void dispatchSecondMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue, boolean exception);
 
 	/**
 	 * Dispatches the 'addCatch' statement of a method.

diff --git a/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/IMethodHook.java b/inspectit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/IMethodHook.java
@@ -32,7 +32,7 @@ public interface IMethodHook extends IHook {
 	 * after body calls. It is important that a hook, implementing this method, just adds time or
 	 * memory critical settings. Everything else, including computing or adding values to the value
 	 * storage has to be added to the
-	 * {@link #secondAfterBody(ICoreService, int, String, Object, Object[], Object, RegisteredSensorConfig)}
+	 * {@link #secondAfterBody(ICoreService, int, String, Object, Object[], Object, boolean, RegisteredSensorConfig)}
 	 * implementation.
 	 *
 	 * @param methodId
@@ -44,12 +44,16 @@ public interface IMethodHook extends IHook {
 	 * @param parameters
 	 *            The parameters of the method call.
 	 * @param result
-	 *            The return value
+	 *            The return value of the method or exception thrown by method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 * @param rsc
 	 *            The {@link RegisteredSensorConfig} object which holds all the information of the
 	 *            executed method.
 	 */
-	void firstAfterBody(long methodId, long sensorTypeId, Object object, Object[] parameters, Object result, RegisteredSensorConfig rsc);
+	void firstAfterBody(long methodId, long sensorTypeId, Object object, Object[] parameters, Object result, boolean exception, RegisteredSensorConfig rsc);
 
 	/**
 	 * This method will be called before the original method will return. It is the second of two
@@ -66,10 +70,14 @@ public interface IMethodHook extends IHook {
 	 * @param parameters
 	 *            The parameters of the method call.
 	 * @param result
-	 *            The return value
+	 *            The return value of the method or exception thrown by method.
+	 * @param exception
+	 *            If method exited as result of exception. If <code>true</code> then the returnValue
+	 *            parameter will be the exception and not the return value of the method execution
+	 *            as such does not exist.
 	 * @param rsc
 	 *            The {@link RegisteredSensorConfig} object which holds all the information of the
 	 *            executed method.
 	 */
-	void secondAfterBody(ICoreService coreService, long methodId, long sensorTypeId, Object object, Object[] parameters, Object result, RegisteredSensorConfig rsc);
+	void secondAfterBody(ICoreService coreService, long methodId, long sensorTypeId, Object object, Object[] parameters, Object result, boolean exception, RegisteredSensorConfig rsc); // NOCHK:8-params
 }
diff --git a/...ctit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/impl/HookDispatcher.java b/...ctit.agent.java/src/main/java/rocks/inspectit/agent/java/hooking/impl/HookDispatcher.java
@@ -159,7 +159,7 @@ public void dispatchMethodBeforeBody(long id, Object object, Object[] parameters
 	 * {@inheritDoc}
 	 */
 	@Override
-	public void dispatchFirstMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue) {
+	public void dispatchFirstMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue, boolean exception) {
 		if (!executionMarker.isActive()) {
 			try {
 				executionMarker.active();
@@ -171,7 +171,7 @@ public void dispatchFirstMethodAfterBody(long id, Object object, Object[] parame
 					// normal execution (sensor with highest priority first)
 					for (IMethodSensor methodSensor : rsc.getMethodSensors()) {
 						IMethodHook methodHook = (IMethodHook) methodSensor.getHook();
-						methodHook.firstAfterBody(id, methodSensor.getSensorTypeConfig().getId(), object, parameters, returnValue, rsc);
+						methodHook.firstAfterBody(id, methodSensor.getSensorTypeConfig().getId(), object, parameters, returnValue, exception, rsc);
 					}
 				} catch (Throwable throwable) { // NOPMD
 					log.error("An error happened in the Hook Dispatcher! (after body)", throwable);
@@ -186,7 +186,7 @@ public void dispatchFirstMethodAfterBody(long id, Object object, Object[] parame
 	 * {@inheritDoc}
 	 */
 	@Override
-	public void dispatchSecondMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue) {
+	public void dispatchSecondMethodAfterBody(long id, Object object, Object[] parameters, Object returnValue, boolean exception) {
 		if (!executionMarker.isActive()) {
 			try {
 				executionMarker.active();
@@ -207,15 +207,15 @@ public void dispatchSecondMethodAfterBody(long id, Object object, Object[] param
 							// the invocation sequence sensor needs the original core service!
 							long sensorId = methodSensor.getSensorTypeConfig().getId();
 							if (invocCoreService == methodHook) { // NOPMD
-								methodHook.secondAfterBody(coreService, id, sensorId, object, parameters, returnValue, rsc);
+								methodHook.secondAfterBody(coreService, id, sensorId, object, parameters, returnValue, exception, rsc);
 							} else {
-								methodHook.secondAfterBody(invocCoreService, id, sensorId, object, parameters, returnValue, rsc);
+								methodHook.secondAfterBody(invocCoreService, id, sensorId, object, parameters, returnValue, exception, rsc);
 							}
 						}
 					} else {
 						for (IMethodSensor methodSensor : rsc.getMethodSensors()) {
 							IMethodHook methodHook = (IMethodHook) methodSensor.getHook();
-							methodHook.secondAfterBody(coreService, id, methodSensor.getSensorTypeConfig().getId(), object, parameters, returnValue, rsc);
+							methodHook.secondAfterBody(coreService, id, methodSensor.getSensorTypeConfig().getId(), object, parameters, returnValue, exception, rsc);
 						}
 					}
 
@@ -232,7 +232,7 @@ public void dispatchSecondMethodAfterBody(long id, Object object, Object[] param
 
 						// The sensor type ID is not important here, thus we are passing a -1. It is
 						// already stored in the data object
-						invocationHook.secondAfterBody(coreService, id, -1, object, parameters, returnValue, rsc);
+						invocationHook.secondAfterBody(coreService, id, -1, object, parameters, returnValue, exception, rsc);
 					}
 				} catch (Throwable throwable) { // NOPMD
 					log.error("An error happened in the Hook Dispatcher! (second after body)", throwable);

diff --git a/...a/src/main/java/rocks/inspectit/agent/java/instrumentation/asm/IInstrumenterConstant.java b/...a/src/main/java/rocks/inspectit/agent/java/instrumentation/asm/IInstrumenterConstant.java
@@ -48,17 +48,23 @@ public interface IInstrumenterConstant {
 	String DISPATCH_METHOD_BEFORE_BODY_DESCRIPTOR = Type.getMethodDescriptor(Type.VOID_TYPE, Type.LONG_TYPE, Type.getType(Object.class), Type.getType(Object[].class));
 
 	/**
-	 * {@link IHookDispatcher#dispatchFirstMethodAfterBody(long, Object, Object[], Object)} and
-	 * {@link IHookDispatcher#dispatchSecondMethodAfterBody(long, Object, Object[], Object)}
+	 * {@link IHookDispatcher#dispatchFirstMethodAfterBody(long, Object, Object[], Object, boolean)} and
+	 * {@link IHookDispatcher#dispatchSecondMethodAfterBody(long, Object, Object[], Object, boolean)}
 	 * descriptor.
 	 */
-	String DISPATCH_METHOD_AFTER_BODY_DESCRIPTOR = Type.getMethodDescriptor(Type.VOID_TYPE, Type.LONG_TYPE, Type.getType(Object.class), Type.getType(Object[].class), Type.getType(Object.class));
+	String DISPATCH_METHOD_AFTER_BODY_DESCRIPTOR = Type.getMethodDescriptor(Type.VOID_TYPE, Type.LONG_TYPE, Type.getType(Object.class), Type.getType(Object[].class), Type.getType(Object.class),
+			Type.BOOLEAN_TYPE);
 
 	/**
 	 * {@link IHookDispatcher#dispatchBeforeCatch(long, Object)} descriptor.
 	 */
 	String DISPATCH_BEFORE_CATCH_DESCRIPTOR = Type.getMethodDescriptor(Type.VOID_TYPE, Type.LONG_TYPE, Type.getType(Object.class));;
 
+	/**
+	 * {@link IHookDispatcher#dispatchOnThrowInBody(long, Object, Object[], Object) descriptor.
+	 */
+	String DISPATCH_ON_THROW_BODY_DESCRIPTOR = Type.getMethodDescriptor(Type.VOID_TYPE, Type.LONG_TYPE, Type.getType(Object.class), Type.getType(Object[].class), Type.getType(Object.class));
+
 	/**
 	 * {@link IHookDispatcher#dispatchConstructorBeforeBody(long, Object[])} descriptor.
 	 */

diff --git a/...java/src/main/java/rocks/inspectit/agent/java/instrumentation/asm/MethodInstrumenter.java b/...java/src/main/java/rocks/inspectit/agent/java/instrumentation/asm/MethodInstrumenter.java
@@ -84,9 +84,10 @@ protected void onMethodExit(int opcode) {
 			dup();
 		}
 
+		// add two false booleans to denote no exception in the call
 		// generate code for calling first and second
-		generateAfterBodyCall("dispatchFirstMethodAfterBody");
-		generateAfterBodyCall("dispatchSecondMethodAfterBody");
+		generateAfterBodyCall("dispatchFirstMethodAfterBody", false);
+		generateAfterBodyCall("dispatchSecondMethodAfterBody", false);
 	}
 
 	/**
@@ -113,11 +114,12 @@ public void visitMaxs(int maxStack, int maxLocals) {
 		visitLabel(finallyHandler);
 
 		// generate code for calling first and second
-		// push nulls as we don't have a result
-		pushNull();
-		pushNull();
-		generateAfterBodyCall("dispatchFirstMethodAfterBody");
-		generateAfterBodyCall("dispatchSecondMethodAfterBody");
+		// push exception as we don't have a result
+		dup();
+		dup();
+		// add true booleans to denote exception in the call
+		generateAfterBodyCall("dispatchFirstMethodAfterBody", true);
+		generateAfterBodyCall("dispatchSecondMethodAfterBody", true);
 
 		mv.visitInsn(ATHROW);
 
@@ -155,24 +157,58 @@ private void generateBeforeBodyCall() {
 	 *
 	 * @param method
 	 *            method to be called can be only
-	 *            {@link IHookDispatcher#dispatchFirstMethodAfterBody(long, Object, Object[], Object)}
+	 *            {@link IHookDispatcher#dispatchFirstMethodAfterBody(long, Object, Object[], Object, boolean)}
 	 *            or
-	 *            {@link IHookDispatcher#dispatchSecondMethodAfterBody(long, Object, Object[], Object)}
+	 *            {@link IHookDispatcher#dispatchSecondMethodAfterBody(long, Object, Object[], Object, boolean)}
+	 * @param exception
+	 *            Value of the exception argument pass to the dispatcher.
 	 */
-	private void generateAfterBodyCall(String method) {
+	private void generateAfterBodyCall(String method, boolean exception) {
+		// prepare first everything up to exception boolean
+		prepareAfterBodyCall();
+
+		// then push on stack info about exception
+		push(exception);
+
+		// execute after body
+		mv.visitMethodInsn(Opcodes.INVOKEINTERFACE, IInstrumenterConstant.IHOOK_DISPATCHER_INTERNAL_NAME, method, IInstrumenterConstant.DISPATCH_METHOD_AFTER_BODY_DESCRIPTOR, true);
+	}
+
+	/**
+	 * Generates call for
+	 * {@link IHookDispatcher#dispatchOnThrowInBody(long, Object, Object[], Object)}. This method
+	 * expects exception object on stack that can be consumed.
+	 */
+	private void generateThrowInBodyCall() {
+		// we can use same code for the after body call since method signature is same (without
+		// exception)
+		prepareAfterBodyCall();
+
+		// execute dispatchOnThrowInBody
+		mv.visitMethodInsn(Opcodes.INVOKEINTERFACE, IInstrumenterConstant.IHOOK_DISPATCHER_INTERNAL_NAME, "dispatchOnThrowInBody", IInstrumenterConstant.DISPATCH_ON_THROW_BODY_DESCRIPTOR, true);
+	}
+
+	/**
+	 * Prepares the afterBody or throwInBody calls by loading dispatcher, methodId, object and
+	 * parameters to the stack. This method excepts result on stack (result will remain first on
+	 * stack).
+	 */
+	private void prepareAfterBodyCall() {
 		// prepare for calls
-		// we expect result on stack so we must swap as result is last argument in the call
+		// we expect following stack: result (short r)
 		loadHookDispatcher();
+		// r-d
 		swap();
+		// d-r
 
-		// first push method id
+		// push method id
 		push(methodId);
 		// can not just swap because method id is long, thus a bit of gymnastic
-		// r-l-l2
+		// d-r-l-l
 		dup2X1();
-		// l-l2-r-l-l2
+		// d-l-l-r-l-l2
 		pop2();
-		// l-l2-r :)
+		// d-l-l-r :)
 
 		// then this object or null if's static
 		if (isStatic) {
@@ -185,19 +221,6 @@ private void generateAfterBodyCall(String method) {
 		// then parameters
 		loadArgArray();
 		swap();
-
-		// execute after body
-		mv.visitMethodInsn(Opcodes.INVOKEINTERFACE, IInstrumenterConstant.IHOOK_DISPATCHER_INTERNAL_NAME, method, IInstrumenterConstant.DISPATCH_METHOD_AFTER_BODY_DESCRIPTOR, true);
-	}
-
-	/**
-	 * Generates call for
-	 * {@link IHookDispatcher#dispatchOnThrowInBody(long, Object, Object[], Object)}. This method
-	 * expects exception object on stack that can be consumed.
-	 */
-	private void generateThrowInBodyCall() {
-		// we can use same code for the after body call since method signature is same
-		generateAfterBodyCall("dispatchOnThrowInBody");
 	}
 
 	/**