Skip to content

Commit

Permalink
W3C Final implementation (#785)
Browse files Browse the repository at this point in the history
* W3C : Tracecontext classes  (#775)

* W3C Tracecontext

* adding comments

* addressing PR comments

* Implement W3C tracestate

* address review feedbacks

* address review comments

* W3C Integration (#782)

* Initial integration of W3C protocol for incoming request

* refactoring header name

* updating the verbosity level when correlation fails

* Fix bugs in correlation found via tests, implement more unit tests and address partial PR comments

* improve createOutboundTracestate()

* create tracestate when header not available

* fix test

* enable outbout w3c

* Refactering code to use Helper methods with TraceContext classes

*  fix tracestate integration, fix outbound tracestate injection, fix tests, propogate traceflags

* add property to turn on W3C in springboot starter, remove debug logs

* adopt internal storage of id's to legacy AI format for backport, update tests

* address PR comments

* Fix an incorrect assert

* refactor resolveCorrelation() method to be more readable and debuggable

* rename method names, create outbound traceparent for http if there is no incoming request too

* fixing a bug in w3c config for agent

* fix the dependency type name, fix target to be host+port | target

* update changelog and readme

* Minor changes, add log trace to check W3C enabled on agent

* adding support for backport with AI-Legacy-Correlation-Headers

* adding tests for backward compatibility

* address PR comments

* add few tests, comments

* add backport switch turn off test

* reset after tests, to fix the build

* adding tracing to understand when W3C is turned on for debugging

* fix format string

* fix a bug in creating correct target for dependencies
  • Loading branch information
dhaval24 committed Dec 15, 2018
1 parent cd5ccbc commit 6418b31
Show file tree
Hide file tree
Showing 23 changed files with 2,152 additions and 73 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# CHANGELOG

# Version 2.3.0
- [#716](https://github.com/Microsoft/ApplicationInsights-Java/issues/716) Introduced W3C Distributed tracing protocol.

# Version 2.2.1
- Fixed [#767](https://github.com/Microsoft/ApplicationInsights-Java/issues/767). Updated gRPC dependencies which inlcudes latest netty version.
- Fixed [#751](https://github.com/Microsoft/ApplicationInsights-Java/issues/751). Added support for absolute paths for log file output.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public void setConfiguration(AgentConfiguration agentConfiguration) {

if (agentConfiguration.getBuiltInConfiguration().isHttpEnabled()) {
InternalLogger.INSTANCE.trace("Adding built-in HTTP instrumentation");
new HttpClassDataProvider(classesToInstrument).add();
HttpClassDataProvider httpClassDataProvider= new HttpClassDataProvider(classesToInstrument);
httpClassDataProvider.setIsW3CEnabled(agentConfiguration.getBuiltInConfiguration().isW3cEnabled());
httpClassDataProvider.setIsW3CBackportEnabled(agentConfiguration.getBuiltInConfiguration().isW3CBackportEnabled());
httpClassDataProvider.add();
}

if (agentConfiguration.getBuiltInConfiguration().isRedisEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,20 @@ public final class HttpClientMethodVisitor extends AbstractHttpMethodVisitor {

private final static String FINISH_DETECT_METHOD_NAME = "httpMethodFinished";
private final static String FINISH_METHOD_RETURN_SIGNATURE = "(Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IJ)V";
private final boolean isW3CEnabled;
private final boolean isW3CBackportEnabled;

public HttpClientMethodVisitor(int access,
String desc,
String owner,
String methodName,
MethodVisitor methodVisitor,
ClassToMethodTransformationData additionalData) {
ClassToMethodTransformationData additionalData,
boolean isW3CEnabled,
boolean isW3CBackportEnabled) {
super(access, desc, owner, methodName, methodVisitor, additionalData);
this.isW3CEnabled = isW3CEnabled;
this.isW3CBackportEnabled = isW3CBackportEnabled;
}

private int deltaInNS;
Expand All @@ -50,43 +56,103 @@ public HttpClientMethodVisitor(int access,
private int childIdLocal;
private int correlationContextLocal;
private int appCorrelationId;
private int tracestate;
private int traceparent;

@Override
public void onMethodEnter() {
mv.visitMethodInsn(INVOKESTATIC, "java/lang/System", "nanoTime", "()J", false);
deltaInNS = this.newLocal(Type.LONG_TYPE);
mv.visitVarInsn(LSTORE, deltaInNS);

// generate child ID
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "generateChildDependencyId", "()Ljava/lang/String;", false);
childIdLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, childIdLocal);

// retrieve correlation context
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "retrieveCorrelationContext", "()Ljava/lang/String;", false);
correlationContextLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, correlationContextLocal);

// retrieve request context
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "retrieveApplicationCorrelationId", "()Ljava/lang/String;", false);
appCorrelationId = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, appCorrelationId);

// inject headers
mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Request-Id");
mv.visitVarInsn(ALOAD, childIdLocal);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);
// This byte code instrumentation is responsible for injecting legacy AI correlation headers which include
// Request-Id and Request-Context and Correlation-Context. By default this headers are propagated if W3C
// is turned off. Please refer to generateChildDependencyId(), retrieveCorrelationContext(),
// retrieveApplicationCorrelationId() from TelemetryCorrelationUtils class for details on how these headers
// are created.
if (!isW3CEnabled) {
// generate child ID
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "generateChildDependencyId", "()Ljava/lang/String;", false);
childIdLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, childIdLocal);

// retrieve correlation context
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "retrieveCorrelationContext", "()Ljava/lang/String;", false);
correlationContextLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, correlationContextLocal);

// retrieve request context
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "retrieveApplicationCorrelationId", "()Ljava/lang/String;", false);
appCorrelationId = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, appCorrelationId);

// inject headers
// 2 because the 1 is the method being instrumented
mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Request-Id");
mv.visitVarInsn(ALOAD, childIdLocal);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Correlation-Context");
mv.visitVarInsn(ALOAD, correlationContextLocal);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Request-Context");
mv.visitVarInsn(ALOAD, appCorrelationId);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);
} else {
// If W3C is enabled, we propagate Traceparent, Tracecontext headers
// to enable correlation. Please refer to generateChildDependencyTraceparent(), generateChildDependencyTraceparent()
// from TraceContextCorrelation class on how to generate this headers.

// generate child Traceparent
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation",
"generateChildDependencyTraceparent", "()Ljava/lang/String;", false);
traceparent = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, traceparent);

mv.visitVarInsn(ALOAD, traceparent);
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation",
"createChildIdFromTraceparentString", "(Ljava/lang/String;)Ljava/lang/String;", false);
childIdLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, childIdLocal);

// retrieve tracestate
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation",
"retriveTracestate", "()Ljava/lang/String;", false);
tracestate = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, tracestate);

// inject headers
// load 2nd variable because the 1st is the method being instrumented
mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("traceparent");
mv.visitVarInsn(ALOAD, traceparent);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

if (isW3CBackportEnabled) {
mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Request-Id");
mv.visitVarInsn(ALOAD, childIdLocal);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);
}

mv.visitVarInsn(ALOAD, tracestate);
Label nullLabel = new Label();
mv.visitJumpInsn(IFNULL, nullLabel);

mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("tracestate");
mv.visitVarInsn(ALOAD, tracestate);

mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

// skip adding tracestate
mv.visitLabel(nullLabel);
}

mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Correlation-Context");
mv.visitVarInsn(ALOAD, correlationContextLocal);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

mv.visitVarInsn(ALOAD, 2);
mv.visitLdcInsn("Request-Context");
mv.visitVarInsn(ALOAD, appCorrelationId);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "addHeader", "(Ljava/lang/String;Ljava/lang/String;)V", true);

mv.visitVarInsn(ALOAD, 2);
mv.visitMethodInsn(INVOKEINTERFACE, "org/apache/http/HttpRequest", "getRequestLine", "()Lorg/apache/http/RequestLine;", true);
Expand Down Expand Up @@ -153,11 +219,20 @@ protected void byteCodeForMethodExit(int opcode) {
int headerValueLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, headerValueLocal);

//generate target
mv.visitVarInsn(ALOAD, headerValueLocal);
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "generateChildDependencyTarget", "(Ljava/lang/String;)Ljava/lang/String;", false);
int targetLocal = this.newLocal(Type.getType(Object.class));
mv.visitVarInsn(ASTORE, targetLocal);
if (!isW3CEnabled) {
//generate target
mv.visitVarInsn(ALOAD, headerValueLocal);
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TelemetryCorrelationUtils", "generateChildDependencyTarget", "(Ljava/lang/String;)Ljava/lang/String;", false);
mv.visitVarInsn(ASTORE, targetLocal);
} else {
//generate target
mv.visitVarInsn(ALOAD, headerValueLocal);
mv.visitMethodInsn(INVOKESTATIC, "com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation",
"generateChildDependencyTarget", "(Ljava/lang/String;)Ljava/lang/String;", false);
mv.visitVarInsn(ASTORE, targetLocal);
}


mv.visitFieldInsn(Opcodes.GETSTATIC, internalName, "INSTANCE", "L" + internalName + ";");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ public final class HttpClassDataProvider {

private final static String REST_TEMPLATE_METTHOD = "doExecute";

public static void setIsW3CEnabled(boolean isW3CEnabled) {
HttpClassDataProvider.isW3CEnabled = isW3CEnabled;
}

private static boolean isW3CEnabled;

private static boolean isW3CBackportEnabled;

public static void setIsW3CBackportEnabled(boolean isW3CBackportEnabled) {
HttpClassDataProvider.isW3CBackportEnabled = isW3CBackportEnabled;
}

private final Map<String, ClassInstrumentationData> classesToInstrument;

public HttpClassDataProvider(Map<String, ClassInstrumentationData> classesToInstrument) {
Expand All @@ -80,7 +92,7 @@ public MethodVisitor create(MethodInstrumentationDecision decision,
String methodName,
MethodVisitor methodVisitor,
ClassToMethodTransformationData additionalData) {
return new HttpClientMethodVisitor(access, desc, className, methodName, methodVisitor, additionalData);
return new HttpClientMethodVisitor(access, desc, className, methodName, methodVisitor, additionalData, isW3CEnabled, isW3CBackportEnabled);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
public class AgentBuiltInConfiguration {
private final boolean enabled;
private final boolean httpEnabled;
private final boolean w3cEnabled;
private final boolean isW3CBackportEnabled;
private final boolean jdbcEnabled;
private final boolean hibernateEnabled;
private final boolean jedisEnabled;
Expand All @@ -43,6 +45,8 @@ public class AgentBuiltInConfiguration {
public AgentBuiltInConfiguration(boolean enabled,
List<ClassInstrumentationData> simpleBuiltInClasses,
boolean httpEnabled,
boolean w3cEnabled,
boolean isW3CBackportEnabled,
boolean jdbcEnabled,
boolean hibernateEnabled,
boolean jedisEnabled,
Expand All @@ -53,6 +57,8 @@ public AgentBuiltInConfiguration(boolean enabled,
this.simpleBuiltInClasses = simpleBuiltInClasses;
this.enabled = enabled;
this.httpEnabled = httpEnabled;
this.w3cEnabled = w3cEnabled;
this.isW3CBackportEnabled = isW3CBackportEnabled;
this.jdbcEnabled = jdbcEnabled;
this.hibernateEnabled = hibernateEnabled;
this.jmxEnabled = jmxEnabled;
Expand Down Expand Up @@ -97,6 +103,14 @@ public boolean isJmxEnabled() {
return jmxEnabled;
}

public boolean isW3cEnabled() {
return w3cEnabled;
}

public boolean isW3CBackportEnabled() {
return isW3CBackportEnabled;
}

public DataOfConfigurationForException getDataOfConfigurationForException() {
return dataOfConfigurationForException;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.microsoft.applicationinsights.agent.internal.agent.ClassInstrumentationData;

import com.microsoft.applicationinsights.internal.logger.InternalLogger;
import java.util.List;

/**
Expand All @@ -35,6 +36,8 @@ public class AgentBuiltInConfigurationBuilder {
private boolean hibernateEnabled = false;
private boolean jedisEnabled = false;
private boolean jmxEnabled = false;
private boolean w3cEnabled = false;
private boolean isW3CBackportEnabled = true;
private long jedisThresholdInMS = 10000L;
private Long maxSqlQueryLimitInMS = 10000L;
private DataOfConfigurationForException dataOfConfigurationForException = new DataOfConfigurationForException();
Expand All @@ -45,9 +48,14 @@ public AgentBuiltInConfiguration create() {
this.dataOfConfigurationForException.setEnabled(false);
}

InternalLogger.INSTANCE.trace(String.format("Outbound W3C tracing is enabled : %s", w3cEnabled));
InternalLogger.INSTANCE.trace(String.format("Outbound W3C backport mode is enabled : %s", isW3CBackportEnabled));

return new AgentBuiltInConfiguration(enabled,
simpleBuiltInClasses,
httpEnabled && enabled,
w3cEnabled && enabled,
isW3CBackportEnabled && enabled,
jdbcEnabled && enabled,
hibernateEnabled && enabled,
jedisEnabled && enabled,
Expand All @@ -62,8 +70,11 @@ public AgentBuiltInConfigurationBuilder setEnabled(boolean enabled) {
return this;
}

public AgentBuiltInConfigurationBuilder setHttpEnabled(boolean httpEnabled) {
public AgentBuiltInConfigurationBuilder setHttpEnabled(boolean httpEnabled, boolean w3cEnabled,
boolean isW3CBackportEnabled) {
this.httpEnabled = httpEnabled;
this.w3cEnabled = w3cEnabled;
this.isW3CBackportEnabled = isW3CBackportEnabled;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ final class XmlAgentConfigurationBuilder implements AgentConfigurationBuilder {
@VisibleForTesting final static String SDK_LOGGER_NUMBER_OF_TOTAL_SIZE_IN_MB = "NumberOfTotalSizeInMB";

private final static long JEDIS_ARGS_THRESHOLD_IN_MS = 10000L;
private final static String W3C_ENABLED = "W3C";
private final static String W3C_BACKCOMPAT_PARAMETER = "enableW3CBackCompat";

private final static String EXCLUDED_PREFIXES_TAG = "ExcludedPrefixes";
private final static String FORBIDDEN_PREFIX_TAG = "Prefix";
Expand Down Expand Up @@ -223,7 +225,10 @@ private void setBuiltInInstrumentation(AgentConfigurationDefaultImpl agentConfig
new ConfigRuntimeExceptionDataBuilder().setRuntimeExceptionData(builtInElement, builtInConfigurationBuilder);

nodes = builtInElement.getElementsByTagName(HTTP_TAG);
builtInConfigurationBuilder.setHttpEnabled(XmlParserUtils.getEnabled(XmlParserUtils.getFirst(nodes), HTTP_TAG));
Element httpElement = XmlParserUtils.getFirst(nodes);
boolean isW3CEnabled = XmlParserUtils.w3cEnabled(httpElement, W3C_ENABLED, false);
boolean isW3CBackportEnabled =XmlParserUtils.w3cEnabled(httpElement, W3C_BACKCOMPAT_PARAMETER, true);
builtInConfigurationBuilder.setHttpEnabled(XmlParserUtils.getEnabled(element, HTTP_TAG),isW3CEnabled, isW3CBackportEnabled);

nodes = builtInElement.getElementsByTagName(JDBC_TAG);
builtInConfigurationBuilder.setJdbcEnabled(XmlParserUtils.getEnabled(XmlParserUtils.getFirst(nodes), JDBC_TAG));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import com.microsoft.applicationinsights.agent.internal.common.StringUtils;
import com.microsoft.applicationinsights.internal.logger.InternalLogger;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
Expand Down Expand Up @@ -59,6 +60,28 @@ public static boolean getEnabled(Element element, String attributeName) {
return getEnabled(element, attributeName, true);
}

/**
* Method to get the attribute value for W3C
* @param element
* @param attributeName
* @return boolean
*/
static boolean w3cEnabled(Element element, String attributeName, boolean defaultValue) {
try {
String strValue = element.getAttribute(attributeName);
if (!StringUtils.isNullOrEmpty(strValue)) {
boolean value = Boolean.valueOf(strValue);
return value;
}
return defaultValue;

} catch (Exception e) {
InternalLogger.INSTANCE.error("cannot parse the correlation format, will default"
+ "to AI proprietory correlation", ExceptionUtils.getStackTrace(e));
}
return defaultValue;
}

public static boolean getEnabled(Element element, String elementName, boolean defaultValue) {
if (element == null) {
return true;
Expand Down
Loading

0 comments on commit 6418b31

Please sign in to comment.