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

[pkg/stanza] Fix Stanza Key Value Parser custom pair delimiter bug #31048

Merged

Conversation

dpaasman00
Copy link
Contributor

Description:
Fixes a bug that would occur where quotes were not respected while pair parsing with a custom pair delimiter. This occurred because in the case of a custom pair delimiter strings.Split() was used which does not respect quotes. This bug was only an issue with custom pair delimiters because the default white space delimiter used strings.FieldsFuncwith a function that did respect quotes.

This fix adds the function splitPairs(input, pairDelimiter string) ([]string, error) which will split the given input on the given pairDelimiter.

Alternative solutions like strings.FieldsFunc and csv.NewReader were considered but not used because they require the delimiter be a rune. This would have worked in many cases except for when the delimiter is more than 1 unique character long. Since the delimiter shouldn't be limited to a single rune, those solutions were rejected.

A consequence of this new function respecting quotes is it will fail and return an error if any quoted value is ever malformed. For example, given the input k1=v1 k2='v2" the function will return an error because the single quote used for k2's value is never closed. It will also fail on the input k1='v1 k2=v2 k3=v3 for the same reason that the quote is never closed. I believe this is the behavior we want so that we don't return incorrect pairs for further parsing.

Link to tracking Issue:
Closes #31034

Testing:
Added in additional unit tests to verify correct behaviors.

Documentation:
I don't believe documentation is required as the user shouldn't see any changes to intended behavior.

@dpaasman00 dpaasman00 marked this pull request as ready for review February 5, 2024 20:53
@dpaasman00 dpaasman00 requested a review from a team February 5, 2024 20:53
pkg/stanza/operator/parser/keyvalue/keyvalue.go Outdated Show resolved Hide resolved
pkg/stanza/operator/parser/keyvalue/keyvalue.go Outdated Show resolved Hide resolved
pkg/stanza/operator/parser/keyvalue/keyvalue.go Outdated Show resolved Hide resolved
pkg/stanza/operator/parser/keyvalue/keyvalue.go Outdated Show resolved Hide resolved
pkg/stanza/operator/parser/keyvalue/keyvalue.go Outdated Show resolved Hide resolved
@dpaasman00 dpaasman00 requested a review from djaglowski February 6, 2024 19:22
@djaglowski djaglowski added the ready to merge Code review completed; ready to merge by maintainers label Feb 8, 2024
@djaglowski djaglowski merged commit fe61401 into open-telemetry:main Feb 8, 2024
147 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg/stanza ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pkg/stanza] Stanza Key Value Parser incorrectly parsing pairs with custom pair delimiter
3 participants