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

[pkg/ottl/ottlfuncs] Added utf8 support to truncate_all function #36713

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[pkg/ottl/ottlfuncs] Added utf8 support to truncate_all function #36713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/ottl/ottlfuncs/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,8 @@ The `truncate_all` function truncates all string values in a `pcommon.Map` so th

`target` is a path expression to a `pcommon.Map` type field. `limit` is a non-negative integer.

If truncating at exactly the length results in a broken UTF-8 encoding, `truncate_all` will be truncated before the last UTF-8 character begins.

The map will be mutated such that the number of characters in all string values is less than or equal to the limit. Non-string values are ignored.

Examples:
Expand Down
12 changes: 11 additions & 1 deletion pkg/ottl/ottlfuncs/func_truncate_all.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-c
import (
"context"
"fmt"
"unicode/utf8"

"go.opentelemetry.io/collector/pdata/pcommon"

Expand Down Expand Up @@ -47,7 +48,16 @@ func TruncateAll[K any](target ottl.PMapGetter[K], limit int64) (ottl.ExprFunc[K
val.Range(func(_ string, value pcommon.Value) bool {
stringVal := value.Str()
if int64(len(stringVal)) > limit {
value.SetStr(stringVal[:limit])
truncatedStr := stringVal[:limit]
for !utf8.ValidString(truncatedStr) {
limit--
if limit == 0 {
value.SetStr("")
return true
}
truncatedStr = stringVal[:limit]
}
value.SetStr(truncatedStr)
Comment on lines +51 to +60
Copy link
Contributor

@jade-guiton-dd jade-guiton-dd Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since utf8.ValidString requires creating a slice and checking all of it every loop, I think a simpler and more efficient solution is to check if the byte after the slice is a valid rune start byte:

Suggested change
truncatedStr := stringVal[:limit]
for !utf8.ValidString(truncatedStr) {
limit--
if limit == 0 {
value.SetStr("")
return true
}
truncatedStr = stringVal[:limit]
}
value.SetStr(truncatedStr)
for limit > 0 && !utf8.RuneStart(stringVal[limit]) {
limit--
}
value.SetStr(stringVal[:limit])

(Neither solution works all that well if the string is not valid UTF-8 in the first place, and both will gladly cut in the middle of multi-codepoint grapheme clusters, but I'm assuming these edge cases are not much of a concern compared to outputting invalid UTF-8.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comment. The way you wrote is faster and simpler, so it can be preferred, of course, if we assume that the string is generally UTF-8 compatible. In the other way, if we consider that a UTF-8 character is a maximum of 4 bytes, we can also validate the string by going back at most 3 times. The choice is yours.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the simplest way to handle invalid UTF-8 input here would be to validate the whole string once; if it succeeds, proceed with the loop; otherwise, just cut at the byte level since it's not UTF-8 in the first place. What do you think?

}
return true
})
Expand Down
81 changes: 77 additions & 4 deletions pkg/ottl/ottlfuncs/func_truncate_all_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import (

func Test_truncateAll(t *testing.T) {
input := pcommon.NewMap()
input.PutStr("test", "hello world")
// 19 bytes. "hello world, " is 13 bytes, "世界" is 6 bytes.
input.PutStr("test", "hello world, 世界")
input.PutInt("test2", 3)
input.PutBool("test3", true)

Expand Down Expand Up @@ -57,17 +58,67 @@ func Test_truncateAll(t *testing.T) {
target: target,
limit: 100,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutStr("test", "hello world, 世界")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate broken first utf8 character encoding - 1",
target: target,
limit: 14,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world, ")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate broken first utf8 character encoding - 2",
target: target,
limit: 15,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world, ")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate first utf8 character exactly",
target: target,
limit: 16,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world, 世")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate broken second utf8 character encoding - 1",
target: target,
limit: 17,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world, 世")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate broken second utf8 character encoding - 2",
target: target,
limit: 18,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world, 世")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
},
{
name: "truncate exact",
target: target,
limit: 11,
limit: 19,
want: func(expectedMap pcommon.Map) {
expectedMap.PutStr("test", "hello world")
expectedMap.PutStr("test", "hello world, 世界")
expectedMap.PutInt("test2", 3)
expectedMap.PutBool("test3", true)
},
Expand Down Expand Up @@ -127,3 +178,25 @@ func Test_truncateAll_get_nil(t *testing.T) {
_, err = exprFunc(nil, nil)
assert.Error(t, err)
}

func Test_truncateAll_utf8_zero_limit(t *testing.T) {
input := pcommon.NewMap()
input.PutStr("test", "世界")

target := &ottl.StandardPMapGetter[pcommon.Map]{
Getter: func(_ context.Context, tCtx pcommon.Map) (any, error) {
return tCtx, nil
},
}

exprFunc, err := TruncateAll(target, 1)
assert.NoError(t, err)

result, err := exprFunc(nil, input)
assert.NoError(t, err)
assert.Nil(t, result)

expected := pcommon.NewMap()
expected.PutStr("test", "")
assert.Equal(t, expected, input)
}
Loading