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.

Sign up for GitHub

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

Jump to bottom

[pkg/ottl] Add ParseKeyValue function #31035

Merged

evan-bradley merged 9 commits into open-telemetry:main from observIQ:ottl/key-value-parser

Feb 15, 2024

Contributor

dpaasman00 commented Feb 5, 2024

Description:
Adds a ParseKeyValue converter function that parses out key values pairs into a pcommon.Map. It takes a StringGetter target argument and 2 optional arguments for the pair delimiter and key value delimiter. This is an adaptation of the Stanza Key Value Parser operator to provide feature parity.

Given the following input string "k1=v1 k2=v2 k3=v3", the function would return the following map:

{ "k1": "v1", "k2": "v2", "k3": "v3" }

Link to tracking Issue:
Closes #30998

Testing:
Added unit tests and e2e test.

Documentation:
Added function documentation.


          write ottl key value func

479838a

linux-foundation-easycla bot commented Feb 5, 2024 •

edited

Loading

The committers listed above are authorized under a signed CLA.

✅ login: dpaasman00 / name: Dakota Paasman (6b0a26d, e1bbb38, 0dbc0d9, 885a566, e36dbd7, 2ecf32d, 479838a, 967289c, ca573d9)

github-actions bot added the pkg/ottl label

github-actions bot requested review from bogdandrutu, evan-bradley, kentquirk and TylerHelmuth

February 5, 2024 15:24


          add e2e tests, documentation, and changelog entry

e36dbd7

BinaryFissionGames reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

BinaryFissionGames reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

dehaansa reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value_test.go Show resolved Hide resolved

dpaasman00 commented

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved


          internal feedback

885a566

dpaasman00 force-pushed the ottl/key-value-parser branch from 39ca655 to 885a566 Compare

February 5, 2024 19:06

BinaryFissionGames reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

dehaansa reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved


          name changes & remove inQuotes

967289c

dehaansa approved these changes

View reviewed changes

dpaasman00 marked this pull request as ready for review

February 5, 2024 21:47

dpaasman00 requested a review from a team

February 5, 2024 21:47

github-actions bot assigned bryan-aguilar


          update parsing to reflect changes made to stanza keyvalue parser

2ecf32d

djaglowski reviewed

View reviewed changes

Member

djaglowski left a comment

I'm not familiar enough with ottl code standards to approve this but I can confirm it appears to implement equivalent parsing functionality to keyvalue_parser from pkg/stanza

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved


          add license

6b0a26d

evan-bradley reviewed

View reviewed changes

Contributor

evan-bradley left a comment

Looks good overall, just a few minor notes.

pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value.go Show resolved Hide resolved

github-actions bot added the internal/core label

dpaasman00 requested a review from evan-bradley

February 12, 2024 19:07


          put pair parsing into internal pkg, add some more unit tests, update …

0dbc0d9

…fmt specifier for errs

dpaasman00 force-pushed the ottl/key-value-parser branch from db6905b to 0dbc0d9 Compare

February 12, 2024 19:10


          update how empty string is handled

ca573d9

evan-bradley reviewed

View reviewed changes

Contributor

evan-bradley left a comment

Thanks for your persistence, I think we're getting close.

pkg/ottl/ottlfuncs/func_parse_key_value.go Show resolved Hide resolved

pkg/ottl/ottlfuncs/func_parse_key_value_test.go Show resolved Hide resolved

evan-bradley reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Show resolved Hide resolved

pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved

evan-bradley mentioned this pull request

[pkg/ottl]: Add ParseCSV converter #31081

Merged

TylerHelmuth reviewed

View reviewed changes

pkg/ottl/ottlfuncs/func_parse_key_value.go Outdated Show resolved Hide resolved

dpaasman00 requested review from TylerHelmuth and evan-bradley

February 15, 2024 12:30

Contributor Author

dpaasman00 commented Feb 15, 2024

Unsure if I should update Stanza in this PR similar to how it was done in #31081 or if that's a separate PR?

dpaasman00 force-pushed the ottl/key-value-parser branch from 7cb8012 to 2454b66 Compare

February 15, 2024 12:35


          move key-value split into parseutils

e1bbb38

dpaasman00 force-pushed the ottl/key-value-parser branch from 2454b66 to e1bbb38 Compare

February 15, 2024 12:35

TylerHelmuth approved these changes

View reviewed changes

Member

TylerHelmuth commented Feb 15, 2024

@dpaasman00 we'll update stanza in a separate PR

evan-bradley approved these changes

View reviewed changes

evan-bradley merged commit d0c0e97 into open-telemetry:main

142 checks passed

dpaasman00 mentioned this pull request

[pkg/stanza] Update KeyValue parser to use parseutils pkg #31291

Merged

djaglowski pushed a commit that referenced this pull request


          [pkg/stanza] Update KeyValue parser to use parseutils pkg (#31291)

609be0a

**Description:** <Describe what has changed.>

Updates the KeyValue parser to use the parseutils pkg and subsequent
functions

**Link to tracking Issue:** N/A

Follows up on this
[comment](#31035 (comment))
about merging functionality between Stanza and OTTL key value parsing

**Testing:** 

Unit tests still pass, had to update one because of different wording in
err message

**Documentation:** N/A

dpaasman00 mentioned this pull request

REQUEST: New membership for dpaasman00 open-telemetry/community#1963

Closed

6 tasks

XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request


          [pkg/ottl] Add ParseKeyValue function (open-telemetry#31035)

4fc983e

**Description:** <Describe what has changed.>
Adds a `ParseKeyValue` converter function that parses out key values
pairs into a `pcommon.Map`. It takes a `StringGetter` target argument
and 2 optional arguments for the pair delimiter and key value delimiter.
This is an adaptation of the Stanza Key Value Parser operator to provide
feature parity.

Given the following input string `"k1=v1 k2=v2 k3=v3"`, the function
would return the following map:
```
{ "k1": "v1", "k2": "v2", "k3": "v3" }
```

**Link to tracking Issue:** <Issue number if applicable>
Closes open-telemetry#30998 

**Testing:** <Describe what testing was performed and which tests were
added.>
Added unit tests and e2e test.

**Documentation:** <Describe the documentation added.>
Added function documentation.

XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request


          [pkg/stanza] Update KeyValue parser to use parseutils pkg (open-telem…

949bdd1

…etry#31291)

**Description:** <Describe what has changed.>

Updates the KeyValue parser to use the parseutils pkg and subsequent
functions

**Link to tracking Issue:** N/A

Follows up on this
[comment](open-telemetry#31035 (comment))
about merging functionality between Stanza and OTTL key value parsing

**Testing:** 

Unit tests still pass, had to update one because of different wording in
err message

**Documentation:** N/A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

BinaryFissionGames BinaryFissionGames left review comments

djaglowski djaglowski left review comments

TylerHelmuth TylerHelmuth approved these changes

evan-bradley evan-bradley approved these changes

dehaansa dehaansa approved these changes

bogdandrutu Awaiting requested review from bogdandrutu bogdandrutu is a code owner

kentquirk Awaiting requested review from kentquirk

Labels

internal/core pkg/ottl