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.

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 string convert case function (lower, upper, snake) #16083

Merged
merged 28 commits into from
Nov 8, 2022
Merged

[pkg/ottl] Add string convert case function (lower, upper, snake) #16083

merged 28 commits into from
Nov 8, 2022

Conversation

clarkjohnd
Copy link
Contributor

Description:

Adds a function (ConvertCase(target, toCase)) to OTTL that converts strings (target) to the following options (toCase):

  • Lowercase lower
  • Uppercase upper
  • Snakecase snake

Originally part of #15379, migrated from metrictransformprocessor to OTTL functions to be able to be used in set operations within transformprocessor instead in separate pull request.

Existing linked PRs will be closed after this one.

@TylerHelmuth @dmitryax

Link to tracking Issue: #15379, #16070

Testing:

Plenty of test cases for each type of case as well as handling nil and empty inputs to the target string and case type. Function tested in dev build with dev branch of transform processor.

Documentation:

READMEs updated with documented function as well as example use case in transform processor as per other functions.

@clarkjohnd
Add snake case func
921948b 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add upper and lower case convert
5a9d25e 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add snake case test cases
0973ce6 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add lower and upper test cases
4c2fffa 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add default case op
d8d3481 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add no op test cases
f698f30 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add nil target test
54ba250 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add non-string test case
c39079e 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Update README
73b6bcb 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add example
acec955 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Merge branch 'main' into ottl-case-convert-function
2d7240a
@clarkjohnd
Add changelog
5628845 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd clarkjohnd requested a review from a team November 4, 2022 20:13
@clarkjohnd
Update changelog issue
e64a9c1 
Signed-off-by: John Clark <[email protected]>
TylerHelmuth
TylerHelmuth reviewed Nov 4, 2022
View reviewed changes
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

@clarkjohnd thanks for doing this so quickly.

pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
TylerHelmuth
TylerHelmuth reviewed Nov 7, 2022
View reviewed changes
pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
@clarkjohnd
Fix import
5311aea 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Add camel case comment
ceb45b9 
Signed-off-by: John Clark <[email protected]>
@clarkjohnd
Copy link
Contributor Author

@TylerHelmuth cheers for the feedback, updated with using that library and for the sake of it while doing the work have added camel case conversion using that library too, probably not useful for metric names but some might find it useful for attribute names or values etc. at some point. Existing tests pass using that library with snake case function.

@clarkjohnd
Remove dedicated functions
b173d68 
Signed-off-by: John Clark <[email protected]>
TylerHelmuth
TylerHelmuth reviewed Nov 8, 2022
View reviewed changes
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

@clarkjohnd couple more nits then we'll be good to go. Thanks!

.chloggen/ottl-case-convert-function.yaml Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_convert_case.go Outdated Show resolved Hide resolved
TylerHelmuth
TylerHelmuth approved these changes Nov 8, 2022
View reviewed changes
Copy link
Member

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

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

@clarkjohnd thanks for your first contribution!

@clarkjohnd
Copy link
Contributor Author

I'll follow up with a PR to get the function working in transform processor.

evan-bradley
evan-bradley approved these changes Nov 8, 2022
View reviewed changes
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for taking this one.

@TylerHelmuth
Copy link
Member

@clarkjohnd please run make gotidy to fix build failures

@clarkjohnd
make gotidy output
7e0f6df 
Signed-off-by: John Clark <[email protected]>
@dmitryax dmitryax added the ready to merge Code review completed; ready to merge by maintainers label Nov 8, 2022
@bogdandrutu bogdandrutu merged commit eee7123 into open-telemetry:main Nov 8, 2022
@clarkjohnd clarkjohnd deleted the ottl-case-convert-function branch November 8, 2022 22:03
@clarkjohnd clarkjohnd mentioned this pull request Nov 9, 2022
Merged
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
@clarkjohnd @shalper2
[pkg/ottl] Add string convert case function (lower, upper, snake) (op…
ba6edee 
…en-telemetry#16083)

* Add snake case func

Signed-off-by: John Clark <[email protected]>

* Add upper and lower case convert

Signed-off-by: John Clark <[email protected]>

* Add snake case test cases

Signed-off-by: John Clark <[email protected]>

* Add lower and upper test cases

Signed-off-by: John Clark <[email protected]>

* Add default case op

Signed-off-by: John Clark <[email protected]>

* Add no op test cases

Signed-off-by: John Clark <[email protected]>

* Add nil target test

Signed-off-by: John Clark <[email protected]>

* Add non-string test case

Signed-off-by: John Clark <[email protected]>

* Update README

Signed-off-by: John Clark <[email protected]>

* Add example

Signed-off-by: John Clark <[email protected]>

* Add changelog

Signed-off-by: John Clark <[email protected]>

* Update changelog issue

Signed-off-by: John Clark <[email protected]>

* Fixes in line with PR 15709

Signed-off-by: John Clark <[email protected]>

* Add error handling for unset case types

Signed-off-by: John Clark <[email protected]>

* Split case functions and refactor

Signed-off-by: John Clark <[email protected]>

* Refactor tests and add error test for case

Signed-off-by: John Clark <[email protected]>

* Update readme and change error messages

Signed-off-by: John Clark <[email protected]>

* Switch to snake case library

Signed-off-by: John Clark <[email protected]>

* Add camel case

Signed-off-by: John Clark <[email protected]>

* Change validation

Signed-off-by: John Clark <[email protected]>

* Fix import

Signed-off-by: John Clark <[email protected]>

* Add camel case comment

Signed-off-by: John Clark <[email protected]>

* Remove dedicated functions

Signed-off-by: John Clark <[email protected]>

* Final fixes and tidy up

Signed-off-by: John Clark <[email protected]>

* make gotidy output

Signed-off-by: John Clark <[email protected]>

Signed-off-by: John Clark <[email protected]>
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerHelmuth TylerHelmuth TylerHelmuth approved these changes

@evan-bradley evan-bradley evan-bradley approved these changes

@dmitryax dmitryax dmitryax approved these changes

@bogdandrutu bogdandrutu Awaiting requested review from bogdandrutu bogdandrutu is a code owner

@kentquirk kentquirk Awaiting requested review from kentquirk

@jpkrohling jpkrohling Awaiting requested review from jpkrohling jpkrohling is a code owner

Assignees

@bogdandrutu bogdandrutu

Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@clarkjohnd @TylerHelmuth @dmitryax @evan-bradley @bogdandrutu