Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jaeger-query] Leading zeros stripped from trace IDs #1578

Closed
tiffon opened this issue Jun 1, 2019 · 8 comments · Fixed by #1956
Closed

[jaeger-query] Leading zeros stripped from trace IDs #1578

tiffon opened this issue Jun 1, 2019 · 8 comments · Fixed by #1956

Comments

@tiffon
Copy link
Member

tiffon commented Jun 1, 2019

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

I would like to view traces in Jaeger UI where the trace ID has leading 0s.

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

When jaeger-query returns data for traces where there are leading 0s in the trace ID, the leading 0s are stripped from the trace ID, within the data. E.g. for a trace with ID 0123, all of the spans returned refer to trace ID 123.

For each trace returned by jaeger-query, this applies to:

  • The trace ID on the root of the trace data object
  • The trace ID fields in each span
  • The trace ID fields in internal references

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

The zeros should not be stripped.

@yurishkuro
Copy link
Member

Can you explain why this is a problem that 0s are stripped?

@tiffon
Copy link
Member Author

tiffon commented Jun 1, 2019

The trace IDs are treated as strings, not numbers. Because they don't match, various aspects of the UI state do not work out, like referring from a span to the UI state for the trace the span is contained within.

@yurishkuro
Copy link
Member

I am asking what user-facing problems this issue is trying to fix, not the internal implementation details.

@tiffon
Copy link
Member Author

tiffon commented Jun 1, 2019

Thank you for the clarification.

Collapsing and expanding parents.

@yurishkuro
Copy link
Member

yurishkuro commented Jun 13, 2019

So currently this is by-design behavior of the TraceID type - its ToString() method returns the shortest possible string representing the trace ID (essentially a base-16 number). I think we should change it because in practice only a small percentage of IDs will start with 0, so the optimization for payload size by using shorter string doesn't make sense.

The change does break a lot of unit tests though.

@vprithvi
Copy link
Contributor

I had discussed this offline with @tiffon and came up with the alternative of the UI doing a 302 redirect from any traceID with leading 0s to a canonical traceID (shortest string representation). Do you think it's a reasonable approach?

@yurishkuro
Copy link
Member

That would be a good thing to do in any case, because even if we make a fix on the server side, people might look up trace by ID they get from the logs, which could have leading 0s stripped. I don't think it would be a good idea to reject requests on the server if traceID != fromHexString(traceID).toString()

@davidmichaelkarr
Copy link

I just noticed this thread, because I think I'm running into this problem in our enterprise. Assuming this is the same problem I'm seeing, this issue is not a cosmetic issue. It appears that we are generating logs between different services that sometimes have the leading zero, and sometimes do not. When we searched for a particular trace-id in our Splunk instance, we found log entries associated with the "client" service, but not the "server" service. I then dug into the actual logs and did some pattern analysis, and that's when I discovered that the logs in the "server" service had the correct trace-id, but with the leading zero removed.

If it matters, we appear to be using Jaeger version 0.33.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants