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

Adding an optional replacementFormat argument to the replace_pattern editors that specified the format of the replacement string #30837

Merged
merged 6 commits into from
Feb 14, 2024

Conversation

rnishtala-sumo
Copy link
Contributor

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

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.

I looked over this quickly and I definitely prefer this solution to #30403. I'll try an do an in-depth review later this week.

Also the latest comment in the issue highlights why I am glad we are taking our time with this issue, as neither PRs would help that user. Currying and/or some other generic looping solution continues to look promising.

@rnishtala-sumo rnishtala-sumo force-pushed the ottl-replace-pattern branch 2 times, most recently from d28cecd to eafac30 Compare February 7, 2024 17:56
@rnishtala-sumo
Copy link
Contributor Author

rnishtala-sumo commented Feb 7, 2024

@TylerHelmuth @evan-bradley thanks for looking at this! 👍 Please let me know if there are any other concerns

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.

We also need this updated in the replace_[all_]matches functions for consistency.

pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_pattern.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_pattern.go Outdated Show resolved Hide resolved
@rnishtala-sumo
Copy link
Contributor Author

@evan-bradley I've added support for replacement format in the replace_[all_]matches editors as well.

@rnishtala-sumo rnishtala-sumo force-pushed the ottl-replace-pattern branch 3 times, most recently from a6aa6da to df5422d Compare February 9, 2024 23:54
@rnishtala-sumo rnishtala-sumo force-pushed the ottl-replace-pattern branch 2 times, most recently from c98078a to 9761559 Compare February 12, 2024 18:10
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 again for your persistence. Just a few readme nits at this point.

pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/README.md Outdated Show resolved Hide resolved
@TylerHelmuth TylerHelmuth merged commit 69b2d51 into open-telemetry:main Feb 14, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 14, 2024
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…editors that specified the format of the replacement string (open-telemetry#30837)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants