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) Fix issue with the hash value of a match subgroup in the replace_pattern editors #29408

Merged
merged 11 commits into from
Jan 8, 2024
Merged

(pkg/ottl) Fix issue with the hash value of a match subgroup in the replace_pattern editors #29408

merged 11 commits into from
Jan 8, 2024

Conversation

rnishtala-sumo
Copy link
Contributor

@rnishtala-sumo rnishtala-sumo commented Nov 20, 2023 •
edited
Loading

Description: Fix issue with the hash value of a match group in replace_pattern*

Link to tracking Issue: #29409

@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from 7a24658 to ec49210 Compare November 21, 2023 01:04
@rnishtala-sumo rnishtala-sumo changed the title (pkg/ottl) Fix issue with the hash value of a match group in replace_pattern* (pkg/ottl) Fix issue with the hash value of a match subgroup in the replace_pattern editors Nov 21, 2023
@rnishtala-sumo rnishtala-sumo marked this pull request as ready for review November 21, 2023 16:04
@rnishtala-sumo rnishtala-sumo requested a review from a team November 21, 2023 16:04
@rnishtala-sumo rnishtala-sumo mentioned this pull request Nov 21, 2023
Closed
@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from f601866 to 854533e Compare November 22, 2023 22:23
@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from 8bfb768 to d6ceb8f Compare November 29, 2023 18:38
@github-actions github-actions bot added the cmd/otelcontribcol otelcontribcol command label Nov 29, 2023
TylerHelmuth
TylerHelmuth approved these changes Nov 29, 2023
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.

@evan-bradley please take a look

evan-bradley
evan-bradley reviewed Nov 30, 2023
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.

Thanks for finding and addressing this issue.

pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns.go Outdated Show resolved Hide resolved
@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from 333f25d to da61c1f Compare December 1, 2023 20:11
evan-bradley
evan-bradley reviewed Dec 4, 2023
View reviewed changes
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/functions.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_all_patterns_test.go Show resolved Hide resolved
@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from f99b50f to b84a738 Compare December 5, 2023 18:31
@TylerHelmuth
Copy link
Member

@rnishtala-sumo ya we had some CI issues today that are now resolved

evan-bradley
evan-bradley reviewed Dec 20, 2023
View reviewed changes
pkg/ottl/ottlfuncs/func_replace_pattern.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_pattern_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_test.go Outdated Show resolved Hide resolved
evan-bradley
evan-bradley approved these changes Dec 20, 2023
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.

Overall looks good to me, I think the remaining changes should be straightforward. Thanks for your patience throughout the review process.

pkg/ottl/ottlfuncs/func_replace_pattern_test.go Outdated Show resolved Hide resolved
pkg/ottl/ottlfuncs/func_replace_pattern_test.go Outdated Show resolved Hide resolved
@evan-bradley
Copy link
Contributor

@TylerHelmuth Please give this another look when you can.

@rnishtala-sumo rnishtala-sumo force-pushed the fix-hash-value branch 2 times, most recently from b2b5f88 to ed1d7c0 Compare December 21, 2023 02:10
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 4, 2024
@rnishtala-sumo
Copy link
Contributor Author

rnishtala-sumo commented Jan 4, 2024 •
edited
Loading

@TylerHelmuth @evan-bradley is this PR ready to be merged?

@evan-bradley
Copy link
Contributor

I'm feeling pretty good about these changes, but would like for @TylerHelmuth to give it another review since the code and functionality have changed pretty substantially since he last reviewed it.

@evan-bradley evan-bradley removed the Stale label Jan 4, 2024
TylerHelmuth
TylerHelmuth approved these changes Jan 8, 2024
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.

@rnishtala-sumo thanks for sticking with this and @evan-bradley thank you for the thorough review

@TylerHelmuth TylerHelmuth added the ready to merge Code review completed; ready to merge by maintainers label Jan 8, 2024
@TylerHelmuth TylerHelmuth merged commit 0613fb6 into open-telemetry:main Jan 8, 2024
89 of 90 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 8, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
@rnishtala-sumo @TylerHelmuth @cparkins
(pkg/ottl) Fix issue with the hash value of a match subgroup in the r…
ea66787 
…eplace_pattern editors (open-telemetry#29408)

**Description:** Fix issue with the hash value of a match group in
replace_pattern*

**Link to tracking Issue:**
open-telemetry#29409

---------

Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com>
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

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

Assignees

@dmitryax dmitryax

Labels
cmd/otelcontribcol otelcontribcol command pkg/ottl ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Milestone
v0.92.0
Development

Successfully merging this pull request may close these issues.
4 participants
@rnishtala-sumo @TylerHelmuth @evan-bradley @dmitryax