WIP TraceContext propagation handler #693

codefromthecrypt · 2018-04-20T09:29:02Z

I paired with @tylerbenson so thanks for the help.

Very early so don't read too much into how the code is shaped until comments suggest otherwise

See also openzipkin/zipkin4net#200 cc @bgrainger

codefromthecrypt · 2018-04-21T00:45:48Z

I will move this to openzipkin-contrib repo at some point. It is too early to put this into brave and I also have less confidence the spec until there are others implementing it.

codefromthecrypt · 2018-05-03T13:54:39Z

https://github.com/w3c/distributed-tracing is a decent place to consider

codefromthecrypt · 2018-09-10T13:57:03Z

fyi this is on pause still as there's a discussion that significantly affects portability pending. We don't have the resources to thrash the impl so waiting on a decision to finalize with ideally a quorum of diverse input.

w3c/trace-context#167

codefromthecrypt · 2019-03-05T23:48:00Z

@SergeyKanzhelev do you mind a quick mention of the status of the w3c tracecontext. I see it is working draft on the github repo, but then also see some pull requests for review. How stable are things? are there any public endpoints available to interop with?

codefromthecrypt · 2019-10-14T00:45:59Z

rebased

codefromthecrypt · 2020-02-10T17:34:33Z

some things that changed since this was done:

tracestate encoding is more complex as characters such as @ are now allowed. I've noticed some using a more expensive regex approach to split on account of this.
oddly 00 is the final version in the delimited header
there are a lot of MUST in the spec for things that were opposite before. For example, you have to handle mixed case, which will be silly in messaging for the reasons we already know about.
odd things like traceparent field for span id renamed to parent id, which conflicts conceptually with B3 with no mention about B3 of course. super strange behavior as using simply "id" could have been done if the word span was problematic. anyway like all the above regressions, this is water under the bridge.

codefromthecrypt · 2020-02-10T18:35:36Z

here's the published version, several days ago. we should watch out to see which errata are accepted (eg if version 00 is errata or intentional) https://www.w3.org/TR/trace-context-1/

iotw recommend if someone wants to pick this up, to wait a few days for errata

codefromthecrypt · 2020-03-11T09:20:17Z

looks like people settled on version double-zero.

codefromthecrypt · 2020-03-11T09:59:23Z

I rebased this and renamed the thing to "w3c" as I can't imagine us wanting to add another module for correlation-context later. Also, I wrote in the README the implementation rationale, which is made to survive multiple systems.

later, I'll scan for spec drift and rally for merge.

codefromthecrypt · 2020-03-12T06:49:10Z

backfilled traceparent tests as that format didn't change much. next is to manage the drift on tracestate which is a lot more permissive than last time.

codefromthecrypt · 2020-03-13T06:03:49Z

I will redo the traceparent stuff following this as it is faster and has better logging #1114

codefromthecrypt · 2020-03-16T13:00:49Z

rewrote the traceparent parser based on recent work committed for b3 single (thanks to @anuraaga for the epic review on that).

This fully supports the traceparent spec now, including edge cases like forward versions. next step is to redo the tracestate parser.

propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java

propagation/w3c/RATIONALE.md

codefromthecrypt · 2020-03-18T06:38:48Z

started on tracestate key generation. it took a bit to understand the validation rules. hats off to @anuraaga for the tip to parse with a boolean array as it is faster than the usual approach netty/netty#9896

codefromthecrypt · 2020-03-18T07:21:13Z

tracestate key parsing numbers from latest commit:

Benchmark                                                     Mode  Cnt  Score   Error   Units
TracestateFormatBenchmarks.validateKey_brave_longest_basic   thrpt   15  5.168 ± 0.061  ops/us
TracestateFormatBenchmarks.validateKey_brave_longest_tenant  thrpt   15  4.056 ± 0.018  ops/us
TracestateFormatBenchmarks.validateKey_regex_longest_basic   thrpt   15  0.401 ± 0.013  ops/us
TracestateFormatBenchmarks.validateKey_regex_longest_tenant  thrpt   15  0.414 ± 0.011  ops/us

codefromthecrypt · 2020-03-18T10:27:03Z

ok tracestatekey algo done.. need to go back and cite the sections of the spec and add logging for the various failure cases.

Benchmark                                                     Mode  Cnt  Score   Error   Units
TracestateFormatBenchmarks.validateKey_brave_longest_basic   thrpt   15  5.265 ± 0.035  ops/us
TracestateFormatBenchmarks.validateKey_brave_longest_tenant  thrpt   15  5.127 ± 0.021  ops/us
TracestateFormatBenchmarks.validateKey_regex_longest_basic   thrpt   15  0.375 ± 0.033  ops/us
TracestateFormatBenchmarks.validateKey_regex_longest_tenant  thrpt   15  0.425 ± 0.009  ops/us

codefromthecrypt · 2020-05-05T01:16:22Z

started reworking this over latest master. next step: take out the unnecessary @ logic

codefromthecrypt · 2020-05-05T01:17:39Z

propagation/w3c/src/test/java/brave/propagation/w3c/TraceContextPropagationTest.java

+  @Test public void extracted_toString() {
+    request.put("traceparent", validTraceparent);
+    request.put("tracestate", "b3=" + validB3Single + "," + otherState);
+
+    assertThat(extractor.extract(request)).hasToString(
+        "Extracted{"
+            + "traceContext=" + sampledContext + ", "
+            + "samplingFlags=SAMPLED_REMOTE, "
+            + "extra=[tracestate: " + otherState + "]"
+            + "}");
+  }


thanks to @anuraaga for all the advice, this is much more intuitive now, and especially useful for partially deconstructed formats like this.

This adds `EntrySplitter` to help handle complex formats such as W3C trace-context efficiently. For example, this has flexible whitespace rules to comply with standards like 'tracestate' which require retaining leading whitespace. See #693

codefromthecrypt · 2020-05-07T14:05:47Z

Annoyingly, trace-context defines some strange whitespace rules. Ex

"tracestate: key=          value"

"          value" is actually the value per the spec.

w3c/trace-context#409

I don't expect them to fix this so I wrote a parser that can deal with it.

once this is in I can finish up #1193

This adds `EntrySplitter` to help handle complex formats such as W3C trace-context efficiently. For example, this has flexible whitespace rules to comply with standards like 'tracestate' which require retaining leading whitespace. See #693

codefromthecrypt · 2020-05-08T07:48:03Z

@anuraaga @samukce I think Brave now has what's needed to do this with the least cruft. As such, once 5.12 is out I'll cut a new openzipkin-contrib repository named brave-w3c with this as "initial commit" referencing this issue in order to retain history. After that, I'll close this PR.

In that repo, we can carry forward the rest of it decoupled from Brave releases, allow people to experiment while details in w3c are still fluid.

codefromthecrypt · 2020-05-11T02:15:49Z

@anuraaga @samukce as people are often confused by HTTP encoding rules, I summarized the relationship with our APIs here #1205

Fixes openzipkin/brave#693

codefromthecrypt · 2020-05-25T02:09:43Z

https://github.com/openzipkin-contrib/brave-propagation-w3c will carry this on

codefromthecrypt force-pushed the trace-context branch from 4180431 to 706e068 Compare June 14, 2018 04:10

codefromthecrypt force-pushed the trace-context branch 3 times, most recently from e7647d7 to 0566e90 Compare August 14, 2018 02:49

codefromthecrypt mentioned this pull request Sep 10, 2018

Jaeger integration spring-cloud/spring-cloud-sleuth#628

Closed

alonbl mentioned this pull request Jun 26, 2019

[Feature Request] Store misc information within TraceContext which are not propagated #929

Closed

codefromthecrypt mentioned this pull request Aug 13, 2019

Resurrects duplicate handlers and explains all the bugs we can make #961

Merged

codefromthecrypt mentioned this pull request Oct 14, 2019

Update docs to mention brave and slf4j earlier spring-cloud/spring-cloud-sleuth#1466

Closed

codefromthecrypt force-pushed the trace-context branch from 0580d73 to 8616978 Compare October 14, 2019 00:45

codefromthecrypt mentioned this pull request Jan 31, 2020

Suggest W3C Trace Context compatibility spring-cloud/spring-cloud-sleuth#1540

Closed

codefromthecrypt force-pushed the trace-context branch from 8616978 to 94aca21 Compare March 11, 2020 09:58

codefromthecrypt force-pushed the trace-context branch from b7ea224 to d77b9ae Compare March 12, 2020 07:01

codefromthecrypt force-pushed the trace-context branch from 09b995c to c45f5f3 Compare March 16, 2020 12:58

codefromthecrypt commented Mar 16, 2020

View reviewed changes

propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java Outdated Show resolved Hide resolved

codefromthecrypt commented Mar 16, 2020

View reviewed changes

propagation/w3c/RATIONALE.md Show resolved Hide resolved

codefromthecrypt commented May 5, 2020

View reviewed changes

codefromthecrypt mentioned this pull request May 7, 2020

Adds EntrySplitter for use in w3c specs and secondary-sampling #1193

Merged

codefromthecrypt force-pushed the trace-context branch from dd20f10 to 733a586 Compare May 8, 2020 07:45

Adrian Cole and others added 13 commits May 12, 2020 19:43

Adds preconditions to extract to another PR

aa12655

WIP: tracecontext

3f3596b

notes and dead code removal

6b253e5

reverts fancy threadlocal which actually hurts more than helps

c6ef509

adds classloader test

0962de6

Starts adding tracestate validation

4363d2d

adds benchmarks to show why not regex

9ac2ffd

better perf and better tests

8d0e73b

validates all the rules

8295d04

polish

d409cf3

Trace context sync master and test fix (#1151)

f5182b3

polishing up

6537377

Onto latest brave

beb3451

codefromthecrypt force-pushed the trace-context branch from 733a586 to beb3451 Compare May 12, 2020 11:51

codefromthecrypt pushed a commit to openzipkin-contrib/brave-propagation-w3c that referenced this pull request May 25, 2020

Imports code from Brave Work-In-Progress PR

4e82d7a

Fixes openzipkin/brave#693

codefromthecrypt mentioned this pull request May 25, 2020

Imports code from Brave Work-In-Progress PR openzipkin-contrib/brave-propagation-w3c#1

Merged

codefromthecrypt closed this in openzipkin-contrib/brave-propagation-w3c#1 May 25, 2020

codefromthecrypt pushed a commit to openzipkin-contrib/brave-propagation-w3c that referenced this pull request May 25, 2020

Imports code from Brave Work-In-Progress PR (#1)

ca6abe1

Fixes openzipkin/brave#693

codefromthecrypt deleted the trace-context branch May 25, 2020 02:09

suman-aella mentioned this pull request Jul 29, 2020

W3C tracecontext propagation spring-cloud/spring-cloud-sleuth#1700

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP TraceContext propagation handler #693

WIP TraceContext propagation handler #693

codefromthecrypt commented Apr 20, 2018

codefromthecrypt commented Apr 21, 2018

codefromthecrypt commented May 3, 2018

codefromthecrypt commented Sep 10, 2018

codefromthecrypt commented Mar 5, 2019

codefromthecrypt commented Oct 14, 2019

codefromthecrypt commented Feb 10, 2020 •

edited

Loading

codefromthecrypt commented Feb 10, 2020

codefromthecrypt commented Mar 11, 2020 •

edited

Loading

codefromthecrypt commented Mar 11, 2020

codefromthecrypt commented Mar 12, 2020

codefromthecrypt commented Mar 13, 2020

codefromthecrypt commented Mar 16, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented May 5, 2020

codefromthecrypt May 5, 2020

codefromthecrypt commented May 7, 2020 •

edited

Loading

codefromthecrypt commented May 8, 2020

codefromthecrypt commented May 11, 2020

codefromthecrypt commented May 25, 2020

WIP TraceContext propagation handler #693

WIP TraceContext propagation handler #693

Conversation

codefromthecrypt commented Apr 20, 2018

codefromthecrypt commented Apr 21, 2018

codefromthecrypt commented May 3, 2018

codefromthecrypt commented Sep 10, 2018

codefromthecrypt commented Mar 5, 2019

codefromthecrypt commented Oct 14, 2019

codefromthecrypt commented Feb 10, 2020 • edited Loading

codefromthecrypt commented Feb 10, 2020

codefromthecrypt commented Mar 11, 2020 • edited Loading

codefromthecrypt commented Mar 11, 2020

codefromthecrypt commented Mar 12, 2020

codefromthecrypt commented Mar 13, 2020

codefromthecrypt commented Mar 16, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented Mar 18, 2020

codefromthecrypt commented May 5, 2020

codefromthecrypt May 5, 2020

Choose a reason for hiding this comment

codefromthecrypt commented May 7, 2020 • edited Loading

codefromthecrypt commented May 8, 2020

codefromthecrypt commented May 11, 2020

codefromthecrypt commented May 25, 2020

codefromthecrypt commented Feb 10, 2020 •

edited

Loading

codefromthecrypt commented Mar 11, 2020 •

edited

Loading

codefromthecrypt commented May 7, 2020 •

edited

Loading