Clarifies the ABNF on tracestate keys

[codefromthecrypt]

I'm not sure if this interpretation is correct, as I'm reverse engineering it from the ABNF. I'm reverse engineering as I expect that folks reading the spec will treat ABNF authoritative vs clerical errors interpreting it.

The main thing is that the change to allow tenant format confused whether or not digits can prefix the basic format. Other changes help better explain the impact and assumptions around tenant format, for example that the intent is to reduce (but not prevent) collisions.

https:/w3c/trace-context/pull/322/files#r393638103

I've allowed edits from maintainers. You may make changes to this and merge it, or close it out with derivative progress. However, I'm not likely to do a lot of revisions. The intent was to force a discussion to fix the underlying issue and I hope you do that one way or another, even if this text is not used at all.

[codefromthecrypt]

I'll make another update as I missed that the "vendor name" in tenant format is effectively a shorter version of the basic format.

[Oberon00]

Why such complex restrictions anyway? Why not just add @ to keychar and use basic-key = 0*256 ( keychar )?

BTW 256 (as oppposed to 255) as maximum length is odd, since you need 9 bit to represent 256.

[codefromthecrypt]

I agree on both points @Oberon00, it is faster to parse also, if there can be multiple '@'. I had to think a while about why 256, but didn't mention it here.

This change so far is to reverse engineer text on the existing ABNF. To me simple is better, if '@' was just an allowed character, a lot of special casing and explanations go away.

I don't know how many sites are using tenant syntax anyway. I suspect most tracing sites can get away with shared knowledge about the format used or a non-standard convention of the same.

I would have expected at least a few diverse requests from different companies using such a syntax before it became standard. I can't see evidence at least in open source of it used at all, though I'm sure there's at least one commercial product doing tenant@vendor syntax. I do agree if doing such a syntax, one should be consistent, but I also think it was way too early to define special use case practices.

[danielkhan]

@SergeyKanzhelev @discostu105 PTAL

[discostu105]

We're using the @ symbol in Dynatrace, but I actually see not really a problem with @Oberon00 's suggestion to just allow the '@' character. 👍

[codefromthecrypt]

If the overspecification of @ could be considered an errata, that would be great. I scoured github and there are very few implementations of the header format yet, apart from what folks are doing that are involved in the spec here directly.

basically we can remove the dual text around the practice as it isn't relevant to pretend this is a common practice when there aren't even a lot of impls. this simplifies things quite a bit as especially when @ is an allowed character, the shortcut approach is invalidated.

I'd prefer when finishing the impl for zipkin's brave client to not have to handle this, and pointing to a spec change that says we don't is the way out.

[codefromthecrypt]

PS while OC and OT (and @felixbarny elastic) support pushing tracestate, I only found 2 projects using tracestate on github

@CodingFabian's gang are using this to stash context as described in order to survive a hop they don't own
instana/nodejs@ef6fdf3

@SergeyKanzhelev's gang are stashing an app ID in it
https:/microsoft/ApplicationInsights-Java/blob/d1f387f2fc7478aad4ba276ee33d6f3045ca6c4c/web/src/main/java/com/microsoft/applicationinsights/web/internal/correlation/TraceContextCorrelation.java

Besides Dynatrace who've already spoke up about using tracestate, not sure quorum, but it would be nice for some resolution of folks using this header on the topic of @

[SergeyKanzhelev]

It looks like a correct interpretation and an improvement for the spec readability.

[SergeyKanzhelev]

Any objections to mark it as "non-substantive" as it doesn't change the format?

[yurishkuro]

Why not just add @ to keychar and use basic-key = 0*256 ( keychar )?

wouldn't this (a) allow repeated @ in the key, and therefore (b) break the clause "Otherwise, a valid tenant ID in one system could be mistaken for the vendor name of another, leading to misinterpretation"?

[codefromthecrypt]

@yurishkuro on your point, yes. that's what I meant by overspecification. We'd need to delete the whole block on tenant stuff

[felixbarny]

AFAIK, our Elastic agents don’t support adding tracestate, we just forward the headers.

[basti1302]

We (Instana aka "Fabian's gang") do not give @ any special meaning nor do we plan to. So +1 from me on removing the special handling of it in the spec.

[codefromthecrypt]

As far as I can tell, the consensus here is to simplify the spec and just allow the @ character. Similar to asterisks (*), there is no count restriction. I'll update the PR and thanks for the feedback

[codefromthecrypt]

the only other thing I did was clarify the total character count for trace keys in text as someone unfamiliar with ABNF might guess it is 255 not 256

[codefromthecrypt]

thanks for the help with the digit thing @dyladan @justinfoote I was going to raise that today because my unit tests broke :) appreciate you saving me the effort.