Skip to content

fix(attrs): use base-10 only for int parsing (#373)#24

Merged
tobert merged 1 commit intomainfrom
fix/attr-hex-parsing
Feb 9, 2026
Merged

fix(attrs): use base-10 only for int parsing (#373)#24
tobert merged 1 commit intomainfrom
fix/attr-hex-parsing

Conversation

@tobert
Copy link
Owner

@tobert tobert commented Feb 9, 2026

Summary

  • Attribute values like 0x0, 0xFF, and 0o777 were silently parsed as integers due to strconv.ParseInt with base 0 auto-detecting hex/octal/binary prefixes
  • Changed to base-10 only parsing so these values remain strings as users expect
  • Added test cases for hex, large hex, and octal prefix strings

Test plan

  • New unit tests in otlpclient/protobuf_span_test.go for 0x0, 0xFF, 0o777
  • All existing tests pass (go test && go test ./...)

Fixes #373

🤖 Generated with Claude Code

…cion

strconv.ParseInt with base 0 auto-detects hex (0x), octal (0o), and
binary (0b) prefixes, causing attribute values like "0x0" and "0xFF"
to silently parse as integers instead of remaining strings. Users
passing hex strings as attribute values got unexpected type coercion.

Fixes #373

🤖 Claude <claude@anthropic.com>
Copilot AI review requested due to automatic review settings February 9, 2026 00:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates attribute parsing so only base-10 integers are parsed, preventing 0x../0o..-prefixed values from being silently coerced into ints.

Changes:

  • Switch strconv.ParseInt from base 0 autodetect to base 10 only in StringMapAttrsToProtobuf
  • Add unit tests asserting 0x0, 0xFF, and 0o777 stay as strings

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
otlpclient/protobuf_span.go Forces integer parsing to base-10 to avoid coercing hex/octal-prefixed strings into ints
otlpclient/protobuf_span_test.go Adds regression coverage for hex/octal-prefixed attribute values remaining strings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +195 to +197
"test 8 - hex string": "0x0",
"test 9 - hex string2": "0xFF",
"test 10 - octal str": "0o777",
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The PR description mentions binary-prefixed strings (e.g., 0b1010) were also being parsed via base-0 autodetection, but the new tests only cover hex and octal. Add a test case asserting a 0b... value remains a string to fully cover the stated behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +196 to +197
"test 9 - hex string2": "0xFF",
"test 10 - octal str": "0o777",
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Test case names like "hex string2" and "octal str" are a bit unclear/inconsistent. Consider renaming to something more descriptive and consistent (e.g., "hex string - uppercase" / "octal-prefixed string"), since these names show up in failures and help quickly identify intent.

Copilot uses AI. Check for mistakes.
@tobert tobert merged commit b7dbb42 into main Feb 9, 2026
1 check passed
@tobert tobert deleted the fix/attr-hex-parsing branch February 9, 2026 02:51
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.

2 participants