jaegertracing / jaeger-client-csharp

🛑 This library is DEPRECATED!
https://jaegertracing.io/
Apache License 2.0
304 stars 67 forks source link

Default Hexadecimal ToString Removing Leading Zeros #183

Closed notcool11 closed 4 years ago

notcool11 commented 4 years ago

Requirement - what kind of business use case are you trying to solve?

When getting the string representation of a span, I want the full tracing information instead of a minimized string.

Problem - what in Jaeger blocks you from solving the requirement?

The ToString override in SpanContext is using the default Hexadecimal output .ToString("x")

        public string ContextAsString()
        {
            return $"{TraceId}:{SpanId}:{ParentId}:{((byte)Flags).**ToString("x")}";**
        }

The same problem happens in the Trace and Span objects:

        public override string ToString()
        {
            if (High == 0)
            {
                return Low.ToString("x");
            }

            return $"{High:x}{Low:x016}";
        }

The default implementation loses any leading zeros, so when looking for traces that have been written to a log file the values don't match.

Proposal - what do you suggest to solve the problem or improve the existing situation?

Add the field lengths anywhere there is a conversion between a number and hex, the specification should have this defined as 16 bytes for Trace Id and 8 bytes for Span Id. So change the formatting to be .ToString("x16") and .ToString("x8") respectively.

Any open questions to address

Thanks!!

yurishkuro commented 4 years ago

+1, there is a tracking ticket for this in the main repo: https://github.com/jaegertracing/jaeger/issues/1657

notcool11 commented 4 years ago

Thanks for pointing that out and linking them.

Falco20019 commented 4 years ago

@notcool11 @yurishkuro This has already been fixed in #171 but is not yet released as it‘s part of 0.4.0 milestone.

yurishkuro commented 4 years ago

oh, great, I didn't know. I will relink the meta ticket to that PR. Closing this.

Falco20019 commented 4 years ago

@notcool11 I try to release 0.4.0 soon. As it contains minor breaking changes, I hoped to add some more into it like the GrpcSender before releasing it. Most of the blockers are resolved. See #145 for parts of the discussion.

As this is currently a one-man-show here for C#, it sadly takes sometimes longer then appreciated. I currently don’t have a lot of spare time due to some projects, but I will still try to finalize 0.4.0.

yurishkuro commented 4 years ago

@Falco20019 thanks for all your hard work on the C# SDK!

Given the 0.x version, sometimes it's not worth waiting for other breaking changes and just release what's available.

Falco20019 commented 4 years ago

I thought I would also have the remaining tickets done in 1-2 weeks but then Corona hit. I will give it a look the next days and decide if I release 0.4.0 as is or if I can complete it.