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

[Bug]: span tags of type int64 may lose precision #3958

Closed
yurishkuro opened this issue Oct 9, 2022 · 13 comments · Fixed by #4034
Closed

[Bug]: span tags of type int64 may lose precision #3958

yurishkuro opened this issue Oct 9, 2022 · 13 comments · Fixed by #4034
Assignees
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

What happened?

UI shows span tag values with a precision loss (jaegertracing/jaeger-ui#982)

Steps to reproduce

  1. save a trace with span tag of type int64 that has a very large value
  2. view the trace in the UI - it may show rounded up value ([Bug]: span tags of type int64 may lose precision jaeger-ui#982)

Expected behavior

UI should show accurate value stored in the backend.

The issue stems from Javascript not being able to represent full resolution of 64-bit integers. The common workaround for that is to send large numbers as string values. The relevant code is here:

value = kv.Int64()

Relevant log output

No response

Screenshot

No response

Additional context

The fix should include a fixture with large-enough value that would lose precision when parsed by Javascript.

Jaeger backend version

No response

SDK

No response

Pipeline

No response

Stogage backend

No response

Operating system

No response

Deployment model

No response

Deployment configs

No response

@yurishkuro yurishkuro added bug help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Oct 9, 2022
@shubbham1215
Copy link
Contributor

shubbham1215 commented Oct 12, 2022

Hi @yurishkuro , as part of my academics, I am looking forward to contribute for an open source project and my first Pull Request. Can I contribute to this issue? You can also assign me #3949 as that also looks a good first issue.

@yurishkuro
Copy link
Member Author

You are welcome to.

@shubbham1215
Copy link
Contributor

Could you please assign the issues to me? I have to send a screenshot of the issues showing that I have been assigned the issues.

@yurishkuro
Copy link
Member Author

assigned

@shubbham1215
Copy link
Contributor

Hi @yurishkuro , I have got a basic understanding of jaeger after going through some of your videos on Youtube. I have built jaeger from source and able to run the demo of hotrod example. But I am not exactly sure how to send a span tag of type int64 that has a very large value. Could you please elaborate more on this?Thanks.

@yurishkuro
Copy link
Member Author

There is a generator in cmd/tracegen, you can add a code there to set a span tag with large int64 value (bigger than the JavaScript limit of 2^53 – 1, e.g. val := int64(1) << 55)

@yurishkuro
Copy link
Member Author

You could also just add that tag to a text fixture and make sure it is converted to a string value in the ui-json output

model/converter/json/fixtures/domain_01.json

@shubbham1215
Copy link
Contributor

Will be raising a PR within the coming week

@shubbham1215
Copy link
Contributor

shubbham1215 commented Nov 7, 2022

I am new to Go language. I have managed to do this and seen that the large value is shown as a string in ui_01-actual.json
Is this the correct fix?

value = kv.Bool()
		case model.Int64Type:
			value = kv.Int64()
			if kv.Int64() > 9007199254740991 || kv.Int64() < -9007199254740991 {
				kv.VType = 0
				value = fmt.Sprintf("%v", value)
			}
		case model.Float64Type:

@yurishkuro
Copy link
Member Author

yep, looks right, but use named constants

@shubbham1215
Copy link
Contributor

Additional context

The fix should include a fixture with large-enough value that would lose precision when parsed by Javascript.

Where do I add this fixture? I added it in domain_01.json and ui_01.json but am getting below error after running the test TestFromDomain
Diff:
--- Expected
+++ Actual
@@ -65,5 +65,5 @@
{
- "key":"js_limit",
- "vType":"INT64",
- "vInt64":9223372036854775222
+ "key": "js_limit",
+ "type": "string",
+ "value": "9223372036854775222"
},
Test: TestFromDomain
FAIL
FAIL github.com/jaegertracing/jaeger/model/converter/json 0.005s

@yurishkuro
Copy link
Member Author

those look like the right files. The failure seems to be from the type, not so much from the value.

@yurishkuro
Copy link
Member Author

I recommend creating a WIP PR, it's easier to discuss there, when there's code.

shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Nov 7, 2022
shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Nov 8, 2022
 [Bug]: span tags of type int64 may lose precision jaegertracing#3958
shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Nov 8, 2022
shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Nov 8, 2022
 [Bug]: span tags of type int64 may lose precision jaegertracing#3958

Signed-off-by: Shubham Sawaiker <[email protected]>
yurishkuro pushed a commit that referenced this issue Nov 9, 2022
### Avoid value precision loss

Signed-off-by: Shubham Sawaiker
[[email protected]](mailto:[email protected])


  ###  Which problem is this PR solving?

-   Resolves:  #3958

### Short description of the changes

- avoid display value precision loss in case of value overflow
Number.MAX_VALUE

Previous PR was made from main branch because of which CI jobs were
failing.
First PR link : #4023

Signed-off-by: Shubham Sawaiker <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Mar 5, 2023
shubbham1215 added a commit to shubbham1215/jaeger that referenced this issue Mar 5, 2023
 [Bug]: span tags of type int64 may lose precision jaegertracing#3958

Signed-off-by: Shubham Sawaiker <[email protected]>
Signed-off-by: shubbham1215 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
2 participants