RootIds generated by TelemetryCorrelationUtils can be < 32 characters

[raedur]

Expected behavior

Traceparent.validate doesn't throw an exception on generated traceIds

Actual behavior

Traceparent.validate sometimes throws an exception on generated traceIds if they are not 32 characters long

Traceparent.validate requires traceIds to be 32 characters long, however those created by TelemetryCorrelationUtils are not guaranteed to be 32 characters long, the current code is as follows:

private static String generateRootId() {
    UUID guid = UUID.randomUUID();
    long least = guid.getLeastSignificantBits();
    long most = guid.getMostSignificantBits();
    return Long.toHexString(most) + Long.toHexString(least);
}

This generates 32 char ids most of the time, (e.g. for UUIDs of "bb1141ac-f798-4a6c-1234-76774462e31f") but if the generated UUID contains multiple 0s in a row (e.g. bb1141ac-f798-4a6c-0000-76774462e31f) it omits the 0s in the generated id (i.e. it generates bb1141acf7984a6c76774462e31f) which is only 28 characters long which subsequently throws the exception in Traceparent.validate

Maybe the generateRootId should be:

private static String generateRootId() {
    return UUID.randomUUID().toString().replace("-", "");
}

[dhaval24]

@raedur thanks for opening the issue. I am curious when do you see this behavior. Have you turned on W3C for both agent and sdk?

[raedur]

This is not using the javaagent, just the sdk. I'll have to double check if W3C is on, but the function should probably be fixed anyway, for now in my code I have created an interceptor that generates the root Id instead of letting application insights do it.

[vlev]

We hit the same bug with 2.4.0 version.

Also, with AI agent instrumenting Apache Http Client, this bug prevents some of the outgoing http requests from executing:

AI: TRACE 26-07-2019 12:23:39.272+0000, 2171(hystrix-CaseHub-31): Stack trace generated is java.lang.IllegalArgumentException: invalid traceId
	at com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Traceparent.validate(Traceparent.java:86)
	at com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Traceparent.<init>(Traceparent.java:39)
	at com.microsoft.applicationinsights.web.internal.correlation.tracecontext.Traceparent.<init>(Traceparent.java:52)
	at com.microsoft.applicationinsights.web.internal.correlation.TraceContextCorrelation.generateChildDependencyTraceparent(TraceContextCorrelation.java:477)
	at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:83)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:108)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:56)
	at feign.httpclient.ApacheHttpClient.execute(ApacheHttpClient.java:85)
	at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:108)
	at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:78)
	at feign.hystrix.HystrixInvocationHandler$1.run(HystrixInvocationHandler.java:106)

In this particular case, traceId was 31 symbols long.

ThreadContext.getRequestTelemetryContext()
                .getHttpRequestTelemetry().getContext().getOperation().getId();

[vlev]

This is pretty significant issue for the users of AI agent.
I could provide a fix proposed by @raedur. Will it be possible to create dedicated release 2.4.1?

[trask]

Hi @vlev, yes, a PR would be great. As far as releasing dedicated 2.4.1 for this, it looks like this exception is caught and logged by TraceContextCorrelation.java, so it should not prevent the outgoing http request from executing, can you confirm what you are seeing?

[vlev]

Hi @trask,
Here is the place where the call fails:

RxError: null
java.lang.NullPointerException: null
	at com.microsoft.applicationinsights.web.internal.correlation.TraceContextCorrelation.createChildIdFromTraceparentString(TraceContextCorrelation.java:498)
	at org.apache.http.impl.client.InternalHttpClient.doExecute(InternalHttpClient.java)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:83)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:108)
	at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:56)
	at feign.httpclient.ApacheHttpClient.execute(ApacheHttpClient.java:85)

It seems, that createChildIdFromTraceparentString() won't work with null value.

    public static String createChildIdFromTraceparentString(String traceparent) {
        assert traceparent != null;

        String[] traceparentArr = traceparent.split("-");
        assert traceparentArr.length == 4;

        return "|" + traceparentArr[1] + "." + traceparentArr[2] + ".";

This method is called thru cryptic asm instrumentation:

 // 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);

[littleaj]

Fixed in version 2.4.1, releasing this week.

[trask]

Thanks @vlev !