W3C Integration #782

dhaval24 · 2018-12-07T03:45:45Z

This PR is super set of all previous PRs which completes the integration of W3C for both incoming and outbound calls.

Non Goals of this PR: Backward Compatibility with AI proprietary correlation protocol.

Enable W3C:

Incoming Side -

J2EE Apps add the following to the <TelemetryModules> tag inside ApplicationInsights.xml

<Add type="com.microsoft.applicationinsights.web.extensibility.modules.WebRequestTrackingTelemetryModul>
   <Param name = "  W3CEnabled" value ="true"/>
</Add>

Springboot apps add the following property:
azure.application-insights.web.w3c=true

Outgoing Side -

Add the following to AI-Agent.xml

<Instrumentation>
        <BuiltIn enabled="true">
            <HTTP enabled="true" W3C="true"/>
        </BuiltIn>
    </Instrumentation>

Remember : you need both inbound and outbound settings to be the same for correlation to work properly.

…d address partial PR comments

…boundRequest

…ests, propogate traceflags

dhaval24 · 2018-12-07T03:47:05Z

Results of W3C Test harness on sample service

...va/com/microsoft/applicationinsights/agent/internal/config/XmlAgentConfigurationBuilder.java

...m/microsoft/applicationinsights/autoconfigure/ApplicationInsightsWebModuleConfiguration.java

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

...com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelationTests.java

lmolkova

we need to report id and parent id in the |traceid.spanid. format. otherwise, there will be no correlation with .NET and python and go

…te tests

dhaval24 · 2018-12-08T02:25:40Z

Test harness results

@reyang test_tracestate_member_count_limit fails because you are using 32 as an argument in LinkedHashMap. That is not something that limits the size of the map. But instead it is the initial size of hashmap.

https://docs.oracle.com/javase/8/docs/api/java/util/LinkedHashMap.html#LinkedHashMap-int-

dhaval24 · 2018-12-08T02:26:09Z

@lmolkova could you please approve the changes. I believe I addressed all your concerns.

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

lmolkova · 2018-12-08T02:55:33Z

...ft/applicationinsights/web/extensibility/modules/WebRequestTrackingTelemetryModuleTests.java

@@ -286,6 +286,10 @@ public void testCrossComponentCorrelationHeadersAreCapturedWhenW3CTurnedOn() {

    }

+    private String formatedID(String id) {


lmolkova · 2018-12-08T02:55:45Z

...com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelationTests.java

@@ -248,4 +248,9 @@ public static String getRequestSourceValue(String appId) {

        return String.format("cid-v1:%s", appId);
    }
+
+    private String formatedID(String id) {


dhaval24 · 2018-12-10T04:40:22Z

@lmolkova I refactored resolveCorrelation() method to be more readable and debugable. The test harness results still hold. Though, I am trying to test E2E with a .NET app and I am finding hard time to enable W3C even after following all the docs. https://docs.microsoft.com/en-us/azure/application-insights/application-insights-correlation?toc=/azure/azure-monitor/toc.json

...ain/java/com/microsoft/applicationinsights/agent/internal/agent/HttpClientMethodVisitor.java

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

lmolkova · 2018-12-10T19:11:46Z

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

+ *
+ * @author Dhaval Doshi
+ */
+public class TraceContextCorrelation {


I wonder, why it is in web packge? it's needed for outgoing requests as well that have nothing to do with web.

In general, Java SDK has tried to blur the distinction within core and web sdk and make the life simpler for users. If you look at ApplicationInsights-SpringBoot-Starter, it usually pulls down both core and web jars, and then smartly detect if the app is MVC type than only enable web modules. But classes are generally available on classpath. So this is okay I think. Also correlation has a primary relationship with requests in general and how they propogate in distributed systems.

However, I think Tracecontext classes can be well placed in core and implementation in web. However, I am aware that this classes will eventually go out of sdk and be replaced with a library so I won't worry about this atm.

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

… no incoming request too

dhaval24 · 2018-12-11T23:32:09Z

@lmolkova @littleaj with above all possible issues are fixed. I can see correlation working completely with W3C. Can you both sign off so I can merge this PR today?

dhaval24 · 2018-12-11T23:55:07Z

https://dev.azure.com/mseng/AppInsights/_build/results?buildId=7964109 (passed CI)

* 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

dhaval24 added 12 commits November 29, 2018 11:43

Initial integration of W3C protocol for incoming request

3afbcea

refactoring header name

abc4fc8

updating the verbosity level when correlation fails

1eae023

Fix bugs in correlation found via tests, implement more unit tests an…

d2f780a

…d address partial PR comments

improve createOutboundTracestate()

5610875

create tracestate when header not available

95b902c

fix test

1ef1ba3

enable outbout w3c

19e172f

resolve merge conflict

8480736

Refactering code to use Helper methods with TraceContext classes

2a83046

Merge branch 'W3CIntegration/IncomingRequest' into W3CIntegration/Out…

49a48d2

…boundRequest

fix tracestate integration, fix outbound tracestate injection, fix t…

3d1407c

…ests, propogate traceflags

dhaval24 self-assigned this Dec 7, 2018

dhaval24 requested review from reyang, littleaj and lmolkova December 7, 2018 03:45

dhaval24 added the Enhancement label Dec 7, 2018

lmolkova reviewed Dec 7, 2018

View reviewed changes

...va/com/microsoft/applicationinsights/agent/internal/config/XmlAgentConfigurationBuilder.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...m/microsoft/applicationinsights/autoconfigure/ApplicationInsightsWebModuleConfiguration.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelationTests.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelationTests.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 7, 2018

View reviewed changes

...com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelationTests.java Outdated Show resolved Hide resolved

lmolkova suggested changes Dec 7, 2018

View reviewed changes

dhaval24 added 2 commits December 7, 2018 10:06

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

5df07a8

adopt internal storage of id's to legacy AI format for backport, upda…

fe9b26e

…te tests

lmolkova reviewed Dec 8, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 8, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 8, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 8, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 8, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 8, 2018

View reviewed changes

address PR comments

06963b2

lmolkova approved these changes Dec 8, 2018

View reviewed changes

dhaval24 added 2 commits December 9, 2018 18:36

Fix an incorrect assert

3a8cb63

refactor resolveCorrelation() method to be more readable and debuggable

363f172

lmolkova reviewed Dec 10, 2018

View reviewed changes

...ain/java/com/microsoft/applicationinsights/agent/internal/agent/HttpClientMethodVisitor.java Show resolved Hide resolved

lmolkova reviewed Dec 10, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

lmolkova reviewed Dec 10, 2018

View reviewed changes

...java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java Outdated Show resolved Hide resolved

dhaval24 added 3 commits December 10, 2018 11:48

rename method names, create outbound traceparent for http if there is…

f9a2ea0

… no incoming request too

fixing a bug in w3c config for agent

c319aad

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

43f9686

dhaval24 merged commit b04fd64 into W3CImplementation Dec 13, 2018

This was referenced Dec 13, 2018

W3C Final implementation #785

Merged

Initial integration of W3C protocol for incoming request #776

Closed

trask deleted the W3CIntegration/OutboundRequest branch September 21, 2020 00:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

W3C Integration #782

W3C Integration #782

dhaval24 commented Dec 7, 2018 •

edited

Loading

dhaval24 commented Dec 7, 2018

lmolkova left a comment

dhaval24 commented Dec 8, 2018

dhaval24 commented Dec 8, 2018

lmolkova Dec 8, 2018

lmolkova Dec 8, 2018

dhaval24 commented Dec 10, 2018

lmolkova Dec 10, 2018

dhaval24 Dec 10, 2018

dhaval24 commented Dec 11, 2018

dhaval24 commented Dec 11, 2018

		@@ -286,6 +286,10 @@ public void testCrossComponentCorrelationHeadersAreCapturedWhenW3CTurnedOn() {

		}

		private String formatedID(String id) {

W3C Integration #782

W3C Integration #782

Conversation

dhaval24 commented Dec 7, 2018 • edited Loading

dhaval24 commented Dec 7, 2018

lmolkova left a comment

Choose a reason for hiding this comment

dhaval24 commented Dec 8, 2018

dhaval24 commented Dec 8, 2018

lmolkova Dec 8, 2018

Choose a reason for hiding this comment

lmolkova Dec 8, 2018

Choose a reason for hiding this comment

dhaval24 commented Dec 10, 2018

lmolkova Dec 10, 2018

Choose a reason for hiding this comment

dhaval24 Dec 10, 2018

Choose a reason for hiding this comment

dhaval24 commented Dec 11, 2018

dhaval24 commented Dec 11, 2018

dhaval24 commented Dec 7, 2018 •

edited

Loading