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

Properly parse unsigned integer trace id #1305

Merged
merged 7 commits into from
Oct 26, 2023

Conversation

vivekmahajan
Copy link
Contributor

  • for large integer values the decodeUnsignedLongToHex throws an NumberFormatException e.g

java.lang.NumberFormatException: For input string: "14293359187408121545"

@vivekmahajan vivekmahajan changed the title Properply parse unsigned integer span id value Properply parse unsigned integer trace id Oct 18, 2023
@vivekmahajan
Copy link
Contributor Author

vivekmahajan commented Oct 18, 2023

cc @seglo @linkleonard @dvgica

@vivekmahajan vivekmahajan changed the title Properply parse unsigned integer trace id Properly parse unsigned integer trace id Oct 18, 2023
@vivekmahajan vivekmahajan marked this pull request as draft October 19, 2023 08:47
@vivekmahajan vivekmahajan marked this pull request as ready for review October 19, 2023 13:53
@dvgica
Copy link
Contributor

dvgica commented Oct 19, 2023

LGTM but the maintainers will have to have a look. Maybe @hughsimpson ?

Copy link
Contributor

@hughsimpson hughsimpson left a comment

Choose a reason for hiding this comment

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

Can't tell you how many times I've had to override this behaviour myself, At least twice,

@hughsimpson
Copy link
Contributor

These flaky tests are killing me, I'm going to try to make them more reliable before merging this

@hughsimpson hughsimpson merged commit 8fd9f24 into kamon-io:master Oct 26, 2023
1 check passed
@hughsimpson
Copy link
Contributor

Merged! Will cut a release with this soon

@vivekmahajan vivekmahajan deleted the fix-unsigned-int-issue branch October 26, 2023 10:49
@vmahajanhopper
Copy link
Contributor

Hi @hughsimpson , do you know when the new version will be released? We are facing this issue in production and will be helpful if we can have this fix. Thanks!

hughsimpson added a commit to hughsimpson/Kamon that referenced this pull request Nov 7, 2023
* Fix issue

* import Long

* Add tests

* fix test name

* checking upper bound

* fix tests

---------

Co-authored-by: Vivek Mahajan <[email protected]>
Co-authored-by: hughsimpson <[email protected]>
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 this pull request may close these issues.

4 participants