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

Tags with string type displayed as integers in UI, bigint js problem. #146

Closed
pewpewp3w opened this issue Dec 19, 2017 · 7 comments
Closed
Assignees
Labels

Comments

@pewpewp3w
Copy link

Hello
Description
JavaScript cannot work with integers larger than 53 bits.
If I have ES doc content like { "_index": "jaeger-span-2017-12-19", "_type": "span", "_id": "AWBw1prODizXRv2ex6jq", "_version": 1, "_score": null, "_source": { "traceID": "ed96bcc0c5d5f49c", "spanID": "10dfe23ba1399824", "parentSpanID": "9abc43610cc1f349", "operationName": "someName", "references": [], "startTime": 1513721598591874, "duration": 124751, "tags": [ { "key": "someId", "type": "string", "value": "567036784275220050" }...,
UI will show tag someId with value 567036784275220030 (as integer).

Steps to reproduce the issue:

  1. Write span with string type tag with value > 9007199254740991
  2. Try to search/display trace in UI

Describe the results you received:
UI casts strings to integers.

Describe the results you expected:
String type value should be shown as string.

Additional environment details (Browser, etc.):
Jaeger 1.0 with Zipkin collector.

@tiffon tiffon self-assigned this Dec 24, 2017
@tiffon tiffon added the bug label Dec 24, 2017
@tiffon
Copy link
Member

tiffon commented Dec 24, 2017

@pewpewp3w Thanks for reporting this! It should be a simple fix...

@tiffon
Copy link
Member

tiffon commented Dec 24, 2017

@yurishkuro There is a bug in the way tags are being presented; the type is not properly retained.

Right now the process is rather blunt:

function parseOrPass(value) {
try {
return JSON.parse(value);
} catch (_) {
return value;
}
}

I was thinking an improvement would be to leave type:string values alone and attempt to JSON.parse(...) any other type of value, falling back to the original form (likely a string).

This will satisfy the types I've observed in our use.

I wanted to run the above by you to make sure I'm not missing anything.


Looks like the spec for the type field is fairly structured for tags (strings, booleans and "numeric type"), but that leaves some variance for the numeric types.

So far I've seen the following for tags:

  • string
  • bool
  • int64
  • float64
  • integer

And, apparently, any type is valid for log fields.

@yurishkuro
Copy link
Member

There are 5 value types supported by the backend domain model for both tags and log fields.

	// StringType indicates the value is a unicode string
	StringType ValueType = iota
	// BoolType indicates the value is a Boolean encoded as int64 number 0 or 1
	BoolType
	// Int64Type indicates the value is an int64 number
	Int64Type
	// Float64Type indicates the value is a float64 number stored as int64
	Float64Type
	// BinaryType indicates the value is binary blob stored as a byte array
	BinaryType

When they are converted to JSON we're using the native Go value and allow Go's "encoding/json" package encode it to the JSON string. There are test fixtures showing how the encoding work.

Our converted doesn't take special care to handle large 64bit integers that are not representable in Javascript.

So we may want to do something about that. On the other hand, the actual issue here seems to be purely how the UI handles the values, i.e. { "key": "someId", "type": "string", "value": "567036784275220050" } is already a string, correctly quoted, so I don't know why UI is converting it to a number.

@tiffon
Copy link
Member

tiffon commented Dec 25, 2017

@yurishkuro Great. And, for the most part, the tag values need no further adjustments.

The only exception I've seen is when the value is a JSON string. I think it's still nice to convert these to objects. So, I'll keep values as-is unless JSON.parse(...) results in an object, in which case I'll work with the resulting object.

@yurishkuro
Copy link
Member

the main thing is that we should not apply JSON.parse() to values that are declared as string types. But that still won't address the problem where backend returns large 64bit numbers as a number - it might be better if very large numbers are actually converted to strings.

@tiffon
Copy link
Member

tiffon commented Dec 25, 2017

@yurishkuro Seems like dealing with large numbers needs to happen in the query service, at least initially. Right now, the UI can't do anything but deserialize the JSON, and by that time it's too late—there is nothing to do but convert numeric types to JavaScript's number type.

Re JSON.parse(), I am applying it to values declared as string types. But, I'm checking to see if it yields an object. This is useful in the case of the value being a JSON string representing an object. I've seen this quite a bit in tags, especially for response. The main benefit is the value is presented in the table with whitespace and syntax highlighting.

@yurishkuro
Copy link
Member

ok. I can make a change in the backend

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

No branches or pull requests

3 participants