[BUG] contrib/sirupsen/logrus: Span IDs changed and missing support for 128-bit Trace IDs

[diegommm]

Tracer Version(s)

1.72.1

Go Version(s)

1.24.1

Bug Report

Span IDs changed

The current version is saving the uint64 values of Span and Trace IDs in the injected logrus entry. With logrus configured to output JSON, they become JSON numbers, and it would appear that some intermediate agent would change those numbers. Examples of changed Span IDs:

Span ID written as JSON number by logrus	Observed in DataDog Logs Explorer
289953633037034736	2899536330370347
3651777087658905286	3651777087658905
5454641429838043524	5454641429838043

To solve this, I just called strconv.FormatUint so that those values are converted to JSON string, hence cannot be changed to match any system's precision or whatever problem it would have. They now reach DataDog Logs Explorer as expected.

Support for 128-bit Trace IDs

The current code only supports 64-bit Trace IDs. I solved this by type-asserting the Span Context to ddtrace.SpanContextW3C and using TraceID128() method if the assertion is true.

Reproduction Code

No response

Error Logs

No response

Go Env Output

No response

[mtoffl01]

Hi there,

We are planning to support this in v2 of dd-trace-go. The official release for v2 is imminent, with the ETA still being finalized.
You can check out the work here: #3116

Let me know if you have any further questions.

[diegommm]

Hi @mtoffl01! Thank you for your time and your reply. I understand that the upcoming v2 will solve this, and I do really want to use it. However, given that:

• V2 ETA is still a work in progress, so v1 is still the prod-ready option
• If there is a v2, it means things break with respect to v1
• This further means that even when v2 is out, it will take some time for us to adapt to the changes
• Also, I wouldn't adopt the very first v2 release, I would wait for the first few patch releases of any library's major version before importing it in production code. It is totally understandable to find bugs in new iterations, but it needs to be accounted for
• The changes needed to fix this and unblock us in v1 are almost trivial

I don't mean to stretch the life of v1 or add a burden to the transition (I know reviewing, merging, etc. takes time as well that you could invest in v2), it's just that it's a rather trivial change that has a fix for a very common use case of a DataDog feature that is now the default (and switching back would seem awkward).

Also, I assume that v1 would need to accept at least fixes (not features) for some time before being completely deprecated in favour of v2, and supporting DataDog's default of 128-bit Trace IDs would sounds like a reasonable fix.

So, especially since the changes are very small and there will be some time until we can move to v2, could you please reconsider the linked PR?

Thank you!

[mtoffl01]

Hi @diegommm ,

Thanks for all the feedback!

If we were to break your request up into 2 separate requests, is one of the following behaviors more of a problem than the other, for your user-experience?

1. logrus integration injecting uint64 trace & span IDs rather than string
2. logrus integration injecting 64 bit trace IDs instead of 128bit

I ask because we deliberately decided to support 128bit trace ID logging on v2 because we did not want to break v1 users' log environments. The uint -> string conversion was just a byproduct of this change, but could feasibly be done in v1.