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.

Sign up for GitHub

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

Resolves #982 #983

Closed
wants to merge 1 commit into from
Closed

Resolves #982 #983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/jaeger-ui/src/model/transform-trace-data.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ describe('transformTraceData()', () => {
],
startTime,
duration,
tags: [],
tags: [
{
key: "bigintkey",
type: "int64",
value: 780177723283783680
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a) this isn't a test by itself since it's not checking anything
b) defining data in code is not equivalent to parsing the data from a JSON

},
],
processID: 'p1',
},
{
Expand Down
13 changes: 12 additions & 1 deletion packages/jaeger-ui/src/model/transform-trace-data.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,18 @@ export default function transformTraceData(data: TraceData & { spans: SpanData[]
const max = data.spans.length;
for (let i = 0; i < max; i++) {
const span: Span = data.spans[i] as Span;
const { startTime, duration, processID } = span;
const { startTime, duration, processID, tags } = span;

// avoid display value precision loss in case of value overflow Number.MAX_VALUE
if(tags) {
for(let j = 0; j < tags.length; j++) {
const tag = tags[j]
if(Number.isInteger(tag.value) && !Number.isSafeInteger(tag.value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the type of tag.value at this point? If it's a string, no need to convert, if it's a number then the precision may already be lost. I would've thought the fix needs to be on the server side such that big numeric numbers are sent as strings in JSON.

Copy link
Author

@SealinGp SealinGp Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"if it's number then the precision may already be lost"

but i got the right console output after i converted it to bigint, it confused me.


transform-trace-data.tsx:103 tag.value 780177723283783700 number
transform-trace-data.tsx:107 tag.value convert to big int 780177723283783680 string

image

api return

{
    "key": "xxx_id",
    "type": "int64",
    "value": 780177723283783680
},

i'll consider output string when it is big numeric numbers. it's a way to solve it after all.

Copy link
Author

@SealinGp SealinGp Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • should business server side return below data instead
{
    "key": "xx_id1",
    "type": "int64",
    "value": 123
},
{
    "key": "xx_id2",
    "type": "string",
    "value": "780177723283783680"
},
{
    "key": "xx_id3",
    "type": "int64",
    "value": "780177723283783680" // json:"value,string" will be able to do it
}
type KeyValue struct {
	Key   string      `json:"key"`
	Type  ValueType   `json:"type,omitempty"`
	Value interface{} `json:"value"` //json output string ?
}

all ways seems to be strange in some logic way 😂

tag.value = BigInt(tag.value).toString();
}
}
}

//
let spanID = span.spanID;
// check for start / end time for the trace
Expand Down