AWS Propagation support

[codefromthecrypt]

Starting with trace ID propagation and ad-hoc reporting:

• extracting trace identifiers costs more because we have to parse within a string.
  • I did get a 12% performance improvement by avoiding use of String.split
• injecting trace identifiers costs less because we only write one string.

Below is reporting within a brave tracer in lambda, with a custom zipkin reporter:

Next step is cleaning up stuff, including readable demo code, and code move to zipkin-aws repo.

Benchmark                          Mode  Cnt   Score   Error   Units
PropagationBenchmarks.extract_aws  thrpt   15   4.307 ± 0.122  ops/us
PropagationBenchmarks.extract_b3   thrpt   15  10.617 ± 0.924  ops/us
PropagationBenchmarks.inject_aws   thrpt   15  13.092 ± 0.794  ops/us
PropagationBenchmarks.inject_b3    thrpt   15  11.132 ± 0.944  ops/us

End to end (overhead) benchmarks (currently propagation only, not conversion)

Benchmark                                            Mode  Cnt    Score   Error  Units
EndToEndBenchmarks.server_get                        avgt   15  149.558 ± 4.030  us/op
EndToEndBenchmarks.traced128Server_get               avgt   15  171.492 ± 8.606  us/op
EndToEndBenchmarks.traced128Server_get_resumeTrace   avgt   15  179.316 ± 5.602  us/op
EndToEndBenchmarks.tracedServer_get                  avgt   15  171.560 ± 4.287  us/op
EndToEndBenchmarks.tracedServer_get_resumeTrace      avgt   15  178.494 ± 7.080  us/op
EndToEndBenchmarks.tracedxrayServer_get              avgt   15  172.102 ± 6.651  us/op
EndToEndBenchmarks.tracedxrayServer_get_resumeTrace  avgt   15  178.151 ± 3.580  us/op
EndToEndBenchmarks.unsampledServer_get               avgt   15  158.549 ± 2.813  us/op

[codefromthecrypt]

Added extract impl, tests and benchmarks. Parsing within a header value is more expensive than separate headers. I got it as quick as I could within a reasonable amount of time.

[codefromthecrypt]

Note: there's a behavior difference in X-Ray vs B3, which is that you can send only a trace ID (not a span ID). This implied a separate commit, which I can pull out separately later. This commit makes it possible to get just the trace ID (which is specified by ELB for log correlation purposes, and has no span ID as ELB doesn't write to X-Ray).

[jcchavezs]

Since the actual precedence is TraceContext and then TraceIdContext, what about reflecting this in the signature?

[codefromthecrypt]

good idea. done

[cemo]

@adriancole this is the best thing, I have ever seen for a while. do you have any idea about when this support will be ready for testing?

[jcchavezs]

Awesome work @adriancole

[codefromthecrypt]

added end-to-end overhead benchmarks, which show the overhead related to this isn't statistically significant in the big picture.

@cemo glad you like! trying to get a converting reporter working today. After that, will need to step back and make sure tech debt isn't added, but won't be long. watch this thread

[codefromthecrypt]

so far so good. Propagation works (creating single-host spans). The trace ID prefix of 59d5a836 is legit (1507174454 in epoch seconds, which is right)

Here's a diff for the https://github.com/openzipkin/brave-webmvc-example changed to use this so far

index 3e0967a..0ccec3b 100755
--- a/servlet3/pom.xml
+++ b/servlet3/pom.xml
@@ -13,7 +13,7 @@
 
   <properties>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-    <brave.version>4.8.1</brave.version>
+    <brave.version>4.8.2-SNAPSHOT</brave.version>
     <zipkin-reporter2.version>2.0.2</zipkin-reporter2.version>
     <spring.version>4.3.8.RELEASE</spring.version>
     <log4j.version>2.8.2</log4j.version>
diff --git a/servlet3/src/main/java/brave/webmvc/TracingConfiguration.java b/servlet3/src/main/java/brave/webmvc/TracingConfiguration.java
index dd1f7e4..465501b 100644
--- a/servlet3/src/main/java/brave/webmvc/TracingConfiguration.java
+++ b/servlet3/src/main/java/brave/webmvc/TracingConfiguration.java
@@ -3,6 +3,7 @@ package brave.webmvc;
 import brave.Tracing;
 import brave.context.log4j2.ThreadContextCurrentTraceContext;
 import brave.http.HttpTracing;
+import brave.propagation.XRayPropagation;
 import brave.spring.web.TracingClientHttpRequestInterceptor;
 import brave.spring.webmvc.TracingHandlerInterceptor;
 import java.util.ArrayList;
@@ -44,6 +45,8 @@ public class TracingConfiguration extends WebMvcConfigurerAdapter {
   @Bean Tracing tracing() {
     return Tracing.newBuilder()
         .localServiceName("brave-webmvc-example")
+        .traceId128Bit(true)
+        .propagationFactory(new XRayPropagation.Factory())
         .currentTraceContext(ThreadContextCurrentTraceContext.create()) // puts trace IDs into logs
         .spanReporter(spanReporter()).build();
   }

[codefromthecrypt]

next step was a custom UDP reporter which converts zipkin spans. This should probably live in zipkin-aws when finished. Here's an example from the demo app:

[cemo]

🕺

[codefromthecrypt]

last experiments before moving this code to zipkin-aws repo:

• incoming B3 -> outgoing AWS ID
• incoming AWS -> outgoing B3
• reporting within lambda (to understand things)

in zipkin-aws repo, I think we'll end up with a converter, collector and a reporter component similar to what we have for stackdriver. the zipkin-aws repo contains stock zipkin components which will work on anything like brave, sleuth, wingtips etc.

Trace ID propagation is beyond X-Ray (ex multiple AWS services use the same format), and the implementation is brave-specific (brave's propagation api is currently only for brave), so I think the AWSPropagation class might stay here even if in a different module

[codefromthecrypt]

Below is reporting within a brave tracer in lambda, with a custom zipkin reporter:

Next step is cleaning up stuff, including readable demo code, and code move to zipkin-aws repo.

[devinsba]

Thats awesome. Great work Adrian!! Hopefully this can be a fallback for us when people decide we shouldn't be hosting zipkin ourselves... SaaS all the things..

…

On Fri, Oct 6, 2017 at 6:47 AM, Adrian Cole ***@***.***> wrote: Below is reporting within a brave tracer in lambda, with a custom zipkin reporter: [image: screen shot 2017-10-06 at 6 42 22 pm] <https://user-images.githubusercontent.com/64215/31274713-bd32d706-aac6-11e7-898f-256db3e64c8c.png> Next step is cleaning up stuff, including readable demo code, and code move to zipkin-aws repo. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#510 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIiosyo_k5_HL7zQWqEStvSerrHlvuyks5spgVUgaJpZM4PtGGt> .

[codefromthecrypt]

Renamed to Amazon and made a utility to extract env. Checking, but I think amazon is the right name as the header format is also used by non-xray products.

[codefromthecrypt]

This code is ready to go. The other part will be in zipkin-aws repo (the XRayUDPSender). Once that's released you will be able to make a brave app. I'm not releasing this as there's currently various problems across travis and maven central. Will try tomorrow.

[codefromthecrypt]

here's a place the reporter will go openzipkin/zipkin-aws#56

[codefromthecrypt]

release won't happen until #514 is in, as that supports the extra fields needed in AWS (plus removes a yet-unreleased api for a better one)

[codefromthecrypt]

OK the implementation no longer drops "extra" fields (besides the Self references which should be dropped). Later implementations can expose means to access these, but in the mean time we can release.

Will merge and release tomorrow morning unless I hear screams.

[jcchavezs]

Lgtm