Add comments and explanation on strategies used

Author: gpolaert

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/AWSClientHelper.java

@@ -9,16 +9,25 @@
 import java.util.Arrays;
 import java.util.List;
 
-
+/**
+ * Patch the AWS  Client during the building steps.
+ * This opentracing integration is compatible with the latest release of the AWS SDK
+ */
 public class AWSClientHelper extends DDAgentTracingHelper<AwsClientBuilder> {
 
 
 	public AWSClientHelper(Rule rule) {
 		super(rule);
 	}
 
-
-	@Override
+	/**
+	 * Strategy: we add a tracing handler to the client when it has just been built. We intercept the return value of
+	 * the com.amazonaws.client.builder.AwsClientBuilder.build() method and add the handler.
+	 *
+	 * @param client The fresh AWS client instance
+	 * @return The same instance, but patched
+	 * @throws Exception
+	 */
 	protected AwsClientBuilder doPatch(AwsClientBuilder client) throws Exception {
 
 		RequestHandler2 handler = new TracingRequestHandler(tracer);

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/ApacheHTTPClientHelper.java

@@ -4,24 +4,32 @@
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.jboss.byteman.rule.Rule;
 
-
+/**
+ * Patch the Apache HTTP Client during the building steps
+ */
 public class ApacheHTTPClientHelper extends DDAgentTracingHelper<HttpClientBuilder> {
 
 	public ApacheHTTPClientHelper(Rule rule) {
 		super(rule);
 	}
 
 
+	@Override
 	public HttpClientBuilder patch(HttpClientBuilder builder) {
 		return super.patch(builder);
 	}
 
 
-	@Override
+	/**
+	 * Strategy: We replace the legacy builder by a new instance providing by the opentracing contribution when
+	 * the builder is instantiate. @see org.apache.http.impl.client.HttpClientBuilder.create() method
+	 *
+	 * @param builder The legacy builder instance
+	 * @return A tracing builder instance (new reference)
+	 * @throws Exception
+	 */
 	protected HttpClientBuilder doPatch(HttpClientBuilder builder) throws Exception {
-
 		return new TracingHttpClientBuilder();
-
 	}
 
 }

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/CassandraHelper.java

@@ -6,31 +6,44 @@
 
 import java.lang.reflect.Constructor;
 
-
+/**
+ * Patch each new sessions created when trying to connect to a Cassandra cluster.
+ */
 public class CassandraHelper extends DDAgentTracingHelper<Session> {
 
 
 	protected CassandraHelper(Rule rule) {
 		super(rule);
 	}
 
-
+	@Override
 	public Session patch(Session session) {
 		return super.patch(session);
 	}
 
-	@Override
+
+	/**
+	 * Strategy: each time we build a connection to a Cassandra cluster, the com.datastax.driver.core.Cluster$Manager.newSession()
+	 * method is called. The opentracing contribution is a simple wrapper, so we just have to wrap the new session.
+	 *
+	 * @param session The fresh session to patch
+	 * @return A new tracing session
+	 * @throws Exception
+	 */
 	protected Session doPatch(Session session) throws Exception {
 
 
+		if ("io.opentracing.contrib.cassandra.TracingSession".equals(session.getClass().getCanonicalName())) {
+			return session;
+		}
+
 		Class<?> clazz = Class.forName("io.opentracing.contrib.cassandra.TracingSession");
 		Constructor<?> constructor = clazz.getDeclaredConstructor(Session.class, Tracer.class);
 		constructor.setAccessible(true);
 		Session newSession = (Session) constructor.newInstance(session, tracer);
 
 		return newSession;
 
-
 	}
 
 

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/DDAgentTracingHelper.java

@@ -8,23 +8,46 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-
+/**
+ * This class provides helpfull stuff in order to easy patch object using Byteman rules
+ *
+ * @param <ObjectType> The type of the object to patch
+ */
 public abstract class DDAgentTracingHelper<ObjectType> extends OpenTracingHelper {
 
-	protected static Tracer tracer;
 	private static final Logger LOGGER = Logger.getLogger(DDAgentTracingHelper.class.getCanonicalName());
 
-	public DDAgentTracingHelper(Rule rule) {
+	/**
+	 * The current instance of the tracer. If something goes wrong during the resolution,
+	 * we provides a NoopTracer.
+	 */
+	protected final Tracer tracer;
+
+	DDAgentTracingHelper(Rule rule) {
 		super(rule);
+		Tracer tracerResolved;
 		try {
-			tracer = getTracer() != null ? getTracer() : NoopTracerFactory.create();
+			tracerResolved = getTracer();
+			tracerResolved = tracerResolved == null ? NoopTracerFactory.create() : tracerResolved;
 		} catch (Exception e) {
-			tracer = NoopTracerFactory.create();
-			warning("Failed to retrieve the tracer, using a NoopTracer: " + e.getMessage());
+			tracerResolved = NoopTracerFactory.create();
+			warning("Failed to retrieve the tracer, using a NoopTracer instead: " + e.getMessage());
 			logStackTrace(e.getMessage(), e);
 		}
+		tracer = tracerResolved;
 	}
 
+
+	/**
+	 * This method takes an object and applies some mutation in order to add tracing capabilities.
+	 * This method should never return any Exception in order to not stop the app traced.
+	 * <p>
+	 * This method should be defined as final, but something Byteman need to define this one with the explicit
+	 * type (i.e. without using generic), so this is why we don't use final here.
+	 *
+	 * @param args The object to patch, the type is defined by the subclass instantiation
+	 * @return The object patched
+	 */
 	public ObjectType patch(ObjectType args) {
 
 		info("Try to patch " + args.getClass().getName());
@@ -38,19 +61,42 @@ public ObjectType patch(ObjectType args) {
 			patched = args;
 		}
 		return patched;
-
 	}
 
-	abstract protected ObjectType doPatch(ObjectType input) throws Exception;
+	/**
+	 * The current implementation of the patch
+	 *
+	 * @param obj the object to patch
+	 * @return the object patched
+	 * @throws Exception The exceptions are managed directly to the patch method
+	 */
+	abstract protected ObjectType doPatch(ObjectType obj) throws Exception;
+
 
+	/**
+	 * Simple wrapper to emit a warning
+	 *
+	 * @param message the message to log as a warning
+	 */
 	protected void warning(String message) {
 		log(Level.WARNING, message);
 	}
 
+	/**
+	 * Simple wrapper to emit an info
+	 *
+	 * @param message the message to log as an info
+	 */
 	protected void info(String message) {
 		log(Level.INFO, message);
 	}
 
+	/**
+	 * Simple wrapper to emit the corresponding stacktrace. To not warn the user, we log the stack as a debug info.
+	 * By default, the stack traces are noit shown in the log.
+	 *
+	 * @param message the stacktrace to log as a debug
+	 */
 	protected void logStackTrace(String message, Throwable th) {
 		LOGGER.log(Level.FINE, message, th);
 	}

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/ElasticsearchHelper.java

@@ -10,7 +10,11 @@
 
 import java.lang.reflect.Method;
 
-
+/**
+ * Instrument all Elasticsearch queries.
+ * We have not found a way to inject the opentracing contribution, so this helper is the instrumentation.
+ * FIXME find a better to way to inject the OT contrib
+ */
 public class ElasticsearchHelper extends DDAgentTracingHelper<ActionListener> {
 
 	public ElasticsearchHelper(Rule rule) {
@@ -19,26 +23,38 @@ public ElasticsearchHelper(Rule rule) {
 
 	private Object request;
 
+	/**
+	 * This method is used to register/save some object that will be used for the instrumentation.
+	 * Currently, we need to keep a reference of the request called
+	 *
+	 * @param request The request used for the query
+	 */
 	public void registerArgs(Object request) {
 		this.request = request;
 	}
 
-
+	@Override
 	public ActionListener patch(ActionListener listener) {
 		return super.patch(listener);
 	}
 
-
-	@Override
-	protected  ActionListener doPatch(ActionListener listener) throws Exception {
-
-		if (listener instanceof  TracingResponseListener) {
+	/**
+	 * Strategy: When a query is executed, if start the instrumentation and a new Span.
+	 * We override the default FutureAction by using the one provided in the opentracing contribution.
+	 *
+	 * @param listener default listener
+	 * @return The tracing listener, the default listener is wrapped in this one.
+	 * @throws Exception
+	 */
+	protected ActionListener doPatch(ActionListener listener) throws Exception {
+
+		if (listener instanceof TracingResponseListener) {
 			return listener;
 		}
 
 		Tracer.SpanBuilder spanBuilder = tracer.buildSpan(request.getClass().getSimpleName()).ignoreActiveSpan().withTag(Tags.SPAN_KIND.getKey(), "client");
 		ActiveSpan parentSpan = tracer.activeSpan();
-		if(parentSpan != null) {
+		if (parentSpan != null) {
 			spanBuilder.asChildOf(parentSpan.context());
 		}
 

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/JettyServletHelper.java

@@ -8,15 +8,21 @@
 import java.util.EnumSet;
 
 /**
- * Created by gpolaert on 6/15/17.
+ * Patch the Jetty Servlet during the init steps
  */
 public class JettyServletHelper extends DDAgentTracingHelper<ServletContextHandler> {
 
 	public JettyServletHelper(Rule rule) {
 		super(rule);
 	}
 
-	@Override
+	/**
+	 * Strategy: Use the contextHandler provided to add a new Tracing filter
+	 *
+	 * @param contextHandler The current contextHandler
+	 * @return The same current contextHandler but patched
+	 * @throws Exception
+	 */
 	protected ServletContextHandler doPatch(ServletContextHandler contextHandler) throws Exception {
 
 		String[] patterns = {"/*"};

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/MongoHelper.java

@@ -4,16 +4,24 @@
 import io.opentracing.contrib.mongo.TracingCommandListener;
 import org.jboss.byteman.rule.Rule;
 
-
+/**
+ * Patch the Mongo builder before constructing the final client
+ */
 public class MongoHelper extends DDAgentTracingHelper<MongoClientOptions.Builder> {
 
 
 	public MongoHelper(Rule rule) {
 		super(rule);
 	}
 
-
-	@Override
+	/**
+	 * Strategy: Just before com.mongodb.MongoClientOptions$Builder.build() method is called, we add a new command listener
+	 * in charge of the tracing.
+	 *
+	 * @param builder The builder instance
+	 * @return The same builder instance with a new tracing command listener that will be use for the client construction
+	 * @throws Exception
+	 */
 	protected MongoClientOptions.Builder doPatch(MongoClientOptions.Builder builder) throws Exception {
 
 

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/OkHttpHelper.java

@@ -8,8 +8,9 @@
 
 import static io.opentracing.contrib.okhttp3.OkHttpClientSpanDecorator.STANDARD_TAGS;
 
+
 /**
- * Created by gpolaert on 6/15/17.
+ * Patch the OkHttp Client during the building steps.
  */
 public class OkHttpHelper extends DDAgentTracingHelper<OkHttpClient.Builder> {
 
@@ -18,7 +19,14 @@ public OkHttpHelper(Rule rule) {
 		super(rule);
 	}
 
-	@Override
+	/**
+	 * Strategy: Just before the okhttp3.OkHttpClient$Builder.build() method called, we add a new interceptor for the tracing
+	 * part.
+	 *
+	 * @param builder The builder instance to patch
+	 * @return The same builder instance with a new tracing interceptor
+	 * @throws Exception
+	 */
 	protected OkHttpClient.Builder doPatch(OkHttpClient.Builder builder) throws Exception {
 		TracingInterceptor interceptor = new TracingInterceptor(tracer, Collections.singletonList(STANDARD_TAGS));
 		builder.addInterceptor(interceptor);

dd-java-agent/src/main/java/io/opentracing/contrib/agent/helper/TomcatServletHelper.java

@@ -8,15 +8,21 @@
 import java.util.EnumSet;
 
 /**
- * Created by gpolaert on 6/15/17.
+ * Patch the Jetty Servlet during the init steps
  */
 public class TomcatServletHelper extends DDAgentTracingHelper<ApplicationContext> {
 
 	public TomcatServletHelper(Rule rule) {
 		super(rule);
 	}
 
-	@Override
+	/**
+	 * Strategy: Use the contextHandler provided to add a new Tracing filter
+	 *
+	 * @param contextHandler The current contextHandler
+	 * @return The same current contextHandler but patched
+	 * @throws Exception
+	 */
 	protected ApplicationContext doPatch(ApplicationContext contextHandler) throws Exception {
 
 		String[] patterns = {"/*"};