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

[OTTL] Fix empty string matching for replace_all_patterns #36408

Conversation

pkcll
Copy link

@pkcll pkcll commented Nov 18, 2024

Fix empty string matching for replace_all_patterns, replace_all_matches OTTL functions

Description

OTTL functions replace_all_patterns, replace_all_matches apply pattern matching on all attribute values including non string types by casting value to string using value.Str() method (see here, here) . For non string types Str() returns empty string.
In case when empty string patterns is used that creates unintended side effect when pattern matching is applied for non string types.

Example:

  • replace_all_patterns(attributes, "value", "^$", "EMPTY_STRING_REPLACEMENT")
  • replace_all_matches(attributes, "value", "", "EMPTY_STRING_REPLACEMENT")

Current behavior:

  • Replaces all empty string attribute values with "EMPTY_STRING_REPLACEMENT" value.
  • Replaces all non string attribute values with "EMPTY_STRING_REPLACEMENT" and converts the type of attributes to string type.

Expected behavior:

  • Replaces empty string values in attributes of string type. Skips non string attribute values for matching.

Testing

Added new test case here and here

Copy link

linux-foundation-easycla bot commented Nov 18, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: pkcll / name: Pavel (c733e5b)

@evan-bradley
Copy link
Contributor

Thanks for opening this @pkcll. I agree this is an issue since replace_pattern and replace_match both check that the target is a string before working with it.

Could you please sign the CLA?

@pkcll
Copy link
Author

pkcll commented Nov 18, 2024

@evan-bradley

Could you please sign the CLA?

Will do shortly. Let me double check which one should I sign individual CLA or corporate

Copy link
Contributor

github-actions bot commented Dec 3, 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 Dec 3, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants