feat(pkg/ottl) Adding an optional replacementFormat argument to the replace_pattern editors that specified the format of the replacement string #27686
Conversation
rnishtala-sumo
requested review from
TylerHelmuth,
bogdandrutu and
evan-bradley
as code owners
rnishtala-sumo
force-pushed
the
ottl-replace-pattern
branch
from
7e8d850 to
b607372
Compare
| want: func(expectedValue pcommon.Value) { | ||
| expectedValue.SetStr("application 0f2407f2d83337b1f757eb1754a7643ce0e8fba620bc605c54566cd6dfd838beotherarg=notsensitive key1 key2") | ||
| expectedValue.SetStr("application passwd=0f2407f2d83337b1f757eb1754a7643ce0e8fba620bc605c54566cd6dfd838be otherarg=notsensitive key1 key2") |
There was a problem hiding this comment.
This is the overall goal of this PR. To be able to add an optional prefix to a hash string. It can also add a prefix to a regular string.
So these are all the ways that a prefix could be added with this change
- replace_pattern(name, "^kube_([0-9A-Za-z]+_)", "k8s.$$1.")
- replace_pattern(name, "^kube_([0-9A-Za-z]+_)", "$$1.", "k8s.")
- replace_pattern(name, "^kube_([0-9A-Za-z]+_)", "$$1.", "k8s.", SHA256)
Note that a prefix string is not included in the hash value
There was a problem hiding this comment.
There was a problem hiding this comment.
@rnishtala-sumo I am a little confused why this PR is needed. Can you open a new issue to describe the problem and how this solves it?
There was a problem hiding this comment.
rnishtala-sumo
force-pushed
the
ottl-replace-pattern
branch
from
b607372 to
48f60fc
Compare
…replace_pattern editors
rnishtala-sumo
force-pushed
the
ottl-replace-pattern
branch
3 times, most recently
from
398d4c6 to
ae7cd31
Compare
| replacementFormat: ottl.NewTestingOptional[string]("%s (passwd)"), | ||
| function: optionalArg, | ||
| want: func(expectedValue pcommon.Value) { | ||
| expectedValue.SetStr("application 0f2407f2d83337b1f757eb1754a7643ce0e8fba620bc605c54566cd6dfd838be (passwd) otherarg=notsensitive key1 key2") |
There was a problem hiding this comment.
support for suffixes
There was a problem hiding this comment.
@TylerHelmuth @evan-bradley we're able to support a suffix as well using the suggested format string approach as mentioned in this comment
rnishtala-sumo
force-pushed
the
ottl-replace-pattern
branch
from
ae7cd31 to
ee00bae
Compare
rnishtala-sumo
changed the title
There was a problem hiding this comment.
From my perspective, after ready through the issue I believe it makes more sense for functions like HASH to only be applied to the extracted groups which avoids the need of providing a format?
ie: replace_pattern(name, "^kube_(...)$", "prefix-$$1--infix-suffix", SHA256)
| @@ -7,7 +7,7 @@ change_type: enhancement | |||
| component: pkg/ottl | |||
|
|
|||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | |||
| note: Add optional Converter parameters to replacement Editors | |||
There was a problem hiding this comment.
Is this updating an existing unpublished feature?
There was a problem hiding this comment.
there will be a new changelog for this. Will revert this change.
|
@MovieStoreGuy Can you add that comment to the issue. It's definitely worth discussing more. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description: Adding an optional replacementFormat argument to the replace_pattern editors that specified the format of the replacement string
Testing: Unit tests were added for this new optional argument
Documentation:
https://github.com/rnishtala-sumo/opentelemetry-collector-contrib/blob/ottl-replace-pattern/pkg/ottl/ottlfuncs/README.md#replace_pattern
The following changelog could be reused for this PR
Related to this issue: #22787