Trace.CreateFromId ignores TraceIdHigh?

[jpotisch]

I apologize if I am just being dumb here but we were working on implementing Zipkin on our distributed platform and are finding that the 128-bit trace ID from a java app is being passed correctly to our .net app via the X-B3-TraceId header, but Zipkin4Net seems to only use the lower 64 bits as its own trace ID. This of course breaks our trace into two parts.

I looked at the source and I see that TraceId is used but TraceIdHigh is not:

private Trace(ITraceContext spanState)
        {
            CurrentSpan = spanState;
            CorrelationId = NumberUtils.LongToGuid(CurrentSpan.TraceId);
        }

I'm not familiar enough with Zipkin to attempt a PR on this but if I'm right (a big if!), that would explain what we observe, e.g. Zipkin4Net extracts 128-bit X-B3-TraceId value 0123456789ABCDEF but discards the first 64 bits and writes its own traces to 89ABCDEF.

If we're doing something wrong, of course please close this ticket and sorry for wasting your time, but any guidance on how to do it right would be much appreciated.

Thanks.

[jpotisch]

I played around with this and found that if I modify Trace.cs as shown below, the incoming 128-bit X-B3-TraceId header gets correctly mapped to the trace CorrelationId:

BEFORE

// line 61
CorrelationId = NumberUtils.LongToGuid(traceId);

// line 77
CorrelationId = NumberUtils.LongToGuid(CurrentSpan.TraceId);

AFTER

//line 61
CorrelationId = Guid.Parse(CurrentSpan.TraceIdHigh.ToString("X") + CurrentSpan.TraceId.ToString("X"));

//line 77
CorrelationId = Guid.Parse(CurrentSpan.TraceIdHigh.ToString("X") + CurrentSpan.TraceId.ToString("X"));

Note that this is just very simple code to show the issue. I'm not suggesting this is the actual way to make the change! But it does seem to confirm my belief that the library does not correctly propagate 128-bit traces today.

[jpotisch]

@adriancole I am very sorry to bother you. I saw #230 so I know you're trying to solve the responsiveness issue on this project, and I know you don't owe me a thing. I am just hoping you might be able to at least point me in the right direction on this issue? If I've found a real bug and am able to come up with a fix and tests, I'm happy to submit a PR, but I need at least a little guidance at this extremely early stage of my understanding of the zipkin4net internals.

We are using Zipkin on our distributed platform (React client, two Java and one .NET backend services) and we are so close to having a single trace represent a request's entire lifecycle, but the .NET app's losing the high half of the incoming trace ID is keeping that goal tantalizingly out of reach, so I am highly motivated to volunteer to help come up with a fix.

So again, I apologize, and again, I'm happy to contribute a fix if this is in fact a bug and someone can give me a map to get started. (And again, if there is no bug and we're just doing something wrong, sorry!)

[codefromthecrypt]

hi. I haven't been active on this project because I can barely compile csharp. I was pulled to another issue due to it being cross posted and sadly it declined from there. in other words I wasnt intentionally ignoring your issue.. I haven't been strictly attentive to the repo. personally I help on many many repos in or outside our org when asked and usually the help is received without drama. in other words I am happy to advise just i cant code effectively for one reason i am near clueless in csharp and two my internet is busted so I can only thumb advise till fixed. the fact that you offer help is awesome as that basically makes progress possible. in other words thank you for the offer.

all that said I will read the technical stuff and try to get back to you either busting thumbs or after my caffeine level drops to where I can starbucks again ;)

Meanwhile, and not as a punt, but personally I am probably not the only person who can answer this question. we have a team and sometimes people always ask me directly. that is fine but fragile. for future reference we have a gitter channel openzipkin/zipkin where others who similarly dont actively watch this repo or know csharp, but do know zipkin, live.

anyway I will reply in a bit. thanks

On Thu, Feb 7, 2019, 1:05 PM Joel Potischman <notifications@github.com wrote:

@adriancole https://github.com/adriancole I am very sorry to bother you. I saw #230 https://github.com/openzipkin/zipkin4net/issues/230 so I know you're trying to solve the responsiveness issue on this project, and I know you don't owe me a thing. I am just hoping you might be able to at least point me in the right direction on this issue? If I've found a real bug and am able to come up with a fix and tests, I'm happy to submit a PR, but I need at least a little guidance at this extremely early stage of my understanding of the zipkin4net internals.

We are using Zipkin on our distributed platform (React client, two Java and one .NET backend services) and we are so close to having a single trace represent a request's entire lifecycle, but the .NET app's losing the high half of the incoming trace ID is keeping that goal tantalizingly out of reach, so I am highly motivated to volunteer to help come up with a fix.

So again, I apologize, and again, I'm happy to contribute a fix if this in fact a bug and someone can give me a map to get started. (And again, if there is no bug and we're just doing something wrong, sorry!)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin4net/issues/226#issuecomment-461289725, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD616YswxopOs2ROAt9TgKMcV6EeQjiks5vK7QqgaJpZM4aALjr .

[codefromthecrypt]

as far as I can tell your code looks like it is correct. I dont know the nature of Guid but the end goal is to parse and generate 128 bit 32 char lowerhex trace id strings.

(there is b3-propagation repo.. main thing is high bits are to the left as you mention)

I think if you raise a pull request and cover both generating and parsing this data (ideally also proving it is sent through process if that is somehow unlikely to work) I can do a concept review even not knowing csharp. I have reviewed code like this in many languages I dont know for better or for worse ;)

On Wed, Jan 16, 2019, 1:51 AM Joel Potischman <notifications@github.com wrote:

I played around with this and found that if I modify Trace.cs as shown below, the incoming 128-bit X-B3-TraceId header gets correctly mapped to the trace CorrelationId:

BEFORE

// line 61CorrelationId = NumberUtils.LongToGuid(traceId); // line 77CorrelationId = NumberUtils.LongToGuid(CurrentSpan.TraceId);

AFTER

//line 61CorrelationId = Guid.Parse(CurrentSpan.TraceIdHigh.ToString("X") + CurrentSpan.TraceId.ToString("X")); //line 77CorrelationId = Guid.Parse(CurrentSpan.TraceIdHigh.ToString("X") + CurrentSpan.TraceId.ToString("X"));

Note that this is just very simple code to show the issue. I'm not suggesting this is the actual way to make the change! But it does seem to confirm my belief that the library does not correctly propagate 128-bit traces today.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin4net/issues/226#issuecomment-454486497, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD6164iamGCGilDVns_13bUAGHnvx0Iks5vDhUTgaJpZM4aALjr .

[jpotisch]

@adriancole Thank you so much. I really didn't know who to reach out to and saw your post so figured you were worth a shot. I already discovered how to break my quick proof-of-concept code, so I will see if I can get a little further along and then reach out to the gitter channel to see if I can make this PR-worthy.

And FYI guid is just a 128-bit data structure used a lot in Windows so it's a perfect fit for trace IDs.

Thanks again!