Handle 128 bits trace Ids #90
Conversation
|
cc @adriancole |
| var secondRecord = new Record(new SpanState(1, 0, 1, SpanFlags.None), TimeUtils.UtcNow, Annotations.ClientRecv()); | ||
| var traceId = 1; | ||
| var traceIdHigh = 2; | ||
| var firstRecord = new Record(new SpanState(traceId, traceIdHigh, 0, 1, SpanFlags.None), TimeUtils.UtcNow, Annotations.ClientRecv()); |
There was a problem hiding this comment.
nit I usually try to order traceIdHigh first, as it aligns with the hex encoding of traceIdHigh+traceId
| public class T_SpanState | ||
| { | ||
| private const long traceId = 1; | ||
| private const long traceIdHigh = 0; |
There was a problem hiding this comment.
same nit on ordering, won't repeat again. feel free to go your way and I understand lexicographically this looks right
| @@ -78,7 +79,7 @@ public void TraceCreatesCorrectRecordWithTimeSpecified() | |||
| [TestCase(SpanFlags.SamplingKnown | SpanFlags.Sampled)] | |||
| public void TraceSamplingForced(SpanFlags initialFlags) | |||
| { | |||
| var spanState = new SpanState(1, 0, 1, initialFlags); | |||
| var spanState = new SpanState(1, 2, 0, 1, initialFlags); | |||
There was a problem hiding this comment.
does csharp support constructor overloading? if so default traceIdHigh to zero
| [Test] | ||
| public void Supports128BitsTraceId() | ||
| { | ||
| var traceId = "00FF0000000000001000000000002111"; |
There was a problem hiding this comment.
make a test that makes it obvious that the 00FF.. part is traceIdHigh? Ex by using shorter numbers like traceIdHigh=1, traceId=2 (as the hex is equiv to decimal and easy to see)
| { | ||
| TraceId = traceId; | ||
| TraceIdHigh = traceIdHigh; |
There was a problem hiding this comment.
default to zero if possible
| hashCode = (hashCode * 397) ^ SpanId.GetHashCode(); | ||
| return hashCode; | ||
| } | ||
| } | ||
|
|
||
| public override string ToString() | ||
| { | ||
| return string.Format("{0}.{1}<:{2}", TraceId, SpanId, ParentSpanId.HasValue ? ParentSpanId.Value.ToString(CultureInfo.InvariantCulture) : "_"); | ||
| return string.Format("{0}{1}.{2}<:{3}", TraceIdHigh == 0 ? "" : TraceIdHigh.ToString(), TraceId, SpanId, ParentSpanId.HasValue ? ParentSpanId.Value.ToString(CultureInfo.InvariantCulture) : "_"); |
| /// <summary> | ||
| /// Extracts traceId. Detects if present and then decode the last 16 bytes | ||
| /// </summary> | ||
| private static long ExtractTraceId(string encodedTraceId) |
fedj
force-pushed
the
128bits
branch
5 times, most recently
from
1c12086 to
99884d1
Compare
|
@adriancole Done, I think it's easier to read with your comments. I also introduced a constant to see the absence of traceIdHigh |
There was a problem hiding this comment.
I note an inconsistency between SpanState's Equals and GetHashCode that we can fix in another PR.
https://github.com/criteo/zipkin4net/issues/91
Fixes #86