feat(ottl): Add a new ottl trim function that trims leading and trailing whitespace from a string #36400
Conversation
rnishtala-sumo
requested review from
TylerHelmuth,
bogdandrutu,
evan-bradley and
a team
as code owners
|
@rnishtala-sumo can you add to the description the issue this PR closes |
…ing whitespace from a string
rnishtala-sumo
force-pushed
the
ottl-trim-function
branch
from
c73ca02 to
b2c24fd
Compare
rnishtala-sumo
force-pushed
the
ottl-trim-function
branch
from
9c7f12c to
22bb9aa
Compare
rnishtala-sumo
force-pushed
the
ottl-trim-function
branch
from
22bb9aa to
d088773
Compare
pkg/ottl/ottlfuncs/func_trim.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if replacement.IsEmpty() || replacementString == "" { |
There was a problem hiding this comment.
Above line 34, lets to a check for if replacement == "" and error if it is. Calling Trim(name, "") implies a nop, but we'd be trimming " ".
There was a problem hiding this comment.
@TylerHelmuth isn't an empty replacement string expected since the argument is optional?
There was a problem hiding this comment.
I'm saying we shouldn't allow a user to set the optional parameter to be "".
There was a problem hiding this comment.
IsEmpty returns false if the user sets the optional parameter to any value I thought, even default values like "".
There was a problem hiding this comment.
Yes, replacement.isEmpty() returns false with a default value of "" for the replacementString.
However, the replacementString is set to "" under two conditions
- When the replacement isn't provided (uses default
"") - When the
replacementStringis explicitly set to""
If we return an error when the replacementString is set to "", we would then make it a mandatory parameter
I've pushed a commit such that when the replacementString is set to "" in either of the above conditions, there is no effect on the original string (default behavior).
… nothing gets trimmed (default)
| } | ||
|
|
||
| func trim[K any](target ottl.StringGetter[K], replacement ottl.Optional[string]) ottl.ExprFunc[K] { | ||
| replacementString := replacement.Get() |
There was a problem hiding this comment.
@rnishtala-sumo we do need to still check if replacement is empty or not. If IsEmpty() returns true then we need to use a default cut string of " ". If IsEmpty() returns false then we can use whatever the value of Get returns.
Description
A field test exists in an event, which contains the string " this is a test ".
I would like to be able to transform this using an ottl function trim, that I would define as trim(attribute["test"]), which would trim this down to "this is a test".
this is a testLink to the issue - #34100