-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[pkg/ottl] Fix escape sequences in strings #30839
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding this solution - I remember spending a while on that regex and was unable to come up with a suitable replacement.
Please add some new scenarios in e2e
that put strings to the test. Some scenarios I think would be useful:
- Using a string that is a single backslash
- Using a string that is 2 backslashes
- Using a string that is 3 backslashes
- Using a string that is 4 backslashes
- A test for the
a("\\", "b")
scenario.
plus any edge cases you can think of.
@TylerHelmuth Alright, I added several tests to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy that the new and existing test cases prove this regex works. @evan-bradley please review
@quentinmit please add a bug_fix changelog entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @quentinmit!
Can you update ottlfuncs/README.md
to remove all references to #23238?
@evan-bradley @TylerHelmuth I added a changelog entry and updated the README. Thanks! |
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Fix ottl parsing to properly handle escape sequences. @TylerHelmuth **Link to tracking Issue:** open-telemetry#23238 **Testing:** <Describe what testing was performed and which tests were added.> Added a new unit test with the example from the linked issue **Documentation:** <Describe the documentation added.> None (pure bugfix)
Description:
Fix ottl parsing to properly handle escape sequences.
@TylerHelmuth
Link to tracking Issue:
#23238
Testing:
Added a new unit test with the example from the linked issue
Documentation:
None (pure bugfix)