[bugfix] Fix search by trace:id for short IDs with leading zeros - #5587
Conversation
ruslan-mikhailov
changed the title
| Operands: nil, | ||
| }) | ||
| } else { | ||
| operand := o.RHS.(Static) |
There was a problem hiding this comment.
it feels strange to me to perform this normalization in these very specific places. should we instead do this in newBinaryOperation and then we can feel confident that trace id's are represented the same everywhere in the ast regardless?
There was a problem hiding this comment.
That's a very nice idea, thank you for that! Fixed
|
I was thinking about this and realized that it doesn't fix cases like: Unfortunately, to fix this completely I feel like we'd need some kind of |
|
One more thought! Should we stop comparing strings and start comparing byte slices? |
aca6b06 to
fe52e9b
Compare
|
This PR has been automatically marked as stale because it has not had any activity in the past 60 days. |
fe52e9b to
a9ef7bc
Compare
|
+ rebase from latest main to revive the branch and resolve conflicts |
| RHS: rhs, | ||
| } | ||
|
|
||
| if attr, ok := lhs.(Attribute); ok && attr.Intrinsic == IntrinsicTraceID { |
There was a problem hiding this comment.
a unit test on newBinaryOperation to lock this down would be nice.
joe-elliott
left a comment
joe-elliott
left a comment
There was a problem hiding this comment.
a small test on newBinaryOperation to lock down the behavior would be nice, but other than that lgtm
|
+ unit test |
9d2867f to
d66b313
Compare
2 participants
What this PR does: fixed trace:id queries with leading zeroes like
{ trace:id = "00000000000000008bb87d091822a2f9" }Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]