Skip to content

Commit 55899e3

Browse files
pellaredpellared
authored
Fix zero attribute value length for malformed UTF-8 (#9054)
Fixes #9053. Fix zero attribute value length truncation for a single malformed UTF-8 byte in the shared SDK normalizer and the independent auto-instrumented trace path. ## Benchmark `benchstat` on AMD EPYC 7571 with Go 1.27, `-cpu=1 -benchtime=200ms -count=8`, comparing upstream `main` with this branch (ns/op; no allocation changes): ```text goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/internal/attrnorm cpu: AMD EPYC 7571 │ /tmp/otel-9053-sdk-final-before.bench │ /tmp/otel-9053-sdk-final-after.bench │ │ sec/op │ sec/op vs base │ Truncate/Zero 6.784n ± 0% 7.207n ± 0% +6.22% (p=0.000 n=8) Truncate/ASCII 13.49n ± 0% 13.86n ± 0% +2.78% (p=0.000 n=8) Truncate/ValidUTF-8 25.53n ± 1% 27.14n ± 1% +6.31% (p=0.000 n=8) geomean 13.27n 13.94n +5.09% │ /tmp/otel-9053-sdk-final-before.bench │ /tmp/otel-9053-sdk-final-after.bench │ │ B/op │ B/op vs base │ Truncate/Zero 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ASCII 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ValidUTF-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ /tmp/otel-9053-sdk-final-before.bench │ /tmp/otel-9053-sdk-final-after.bench │ │ allocs/op │ allocs/op vs base │ Truncate/Zero 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ASCII 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ValidUTF-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean ``` ```text goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/trace cpu: AMD EPYC 7571 │ /tmp/otel-9053-trace-controlled-before.bench │ /tmp/otel-9053-trace-controlled-after.bench │ │ sec/op │ sec/op vs base │ Truncate/Zero 7.987n ± 0% 6.802n ± 1% -14.85% (p=0.000 n=8) Truncate/ASCII 16.32n ± 0% 15.17n ± 0% -7.05% (p=0.000 n=8) Truncate/ValidUTF-8 30.71n ± 0% 29.54n ± 1% -3.83% (p=0.000 n=8) geomean 15.88n 14.50n -8.69% │ /tmp/otel-9053-trace-controlled-before.bench │ /tmp/otel-9053-trace-controlled-after.bench │ │ B/op │ B/op vs base │ Truncate/Zero 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ASCII 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ValidUTF-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ /tmp/otel-9053-trace-controlled-before.bench │ /tmp/otel-9053-trace-controlled-after.bench │ │ allocs/op │ allocs/op vs base │ Truncate/Zero 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ASCII 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ Truncate/ValidUTF-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=8) ¹ geomean ² +0.00% ² ¹ all samples are equal ² summaries must be >0 to compute geomean ```
1 parent 75b1fe3 commit 55899e3

9 files changed

Lines changed: 40 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
3737

3838
### Fixed
3939

40+
- Truncate string attribute values to empty strings when the configured length limit is zero, including malformed UTF-8, in `go.opentelemetry.io/otel/trace`, `go.opentelemetry.io/otel/sdk/trace`, and `go.opentelemetry.io/otel/sdk/log`. (#9054)
4041
- Fix a data race in `NewPeriodicReader` where `r.inst` was assigned after the background goroutine was launched in `go.opentelemetry.io/otel/sdk/metric`. (#9028)
4142
- Prevent precision loss for large `int64` values in `Sum`, `Histogram`, and `ExponentialHistogram` aggregations in `go.opentelemetry.io/otel/sdk/metric`. (#8981)
4243
- Ensure `grpc.DialOption` values passed via `WithDialOption` take precedence over conflicting internally-computed defaults in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc`. (#8836)

‎internal/shared/attrnorm/truncate.go.tmpl‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,10 @@ func truncate(limit int, s string) string {
359359
continue
360360
}
361361

362+
// A zero limit also truncates a leading invalid byte or U+FFFD.
363+
if limit == 0 {
364+
return ""
365+
}
362366
_, size := utf8.DecodeRuneInString(s[i:])
363367
if size == 1 {
364368
// Invalid encoding.

‎internal/shared/attrnorm/truncate_test.go.tmpl‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ func TestTruncateAttr(t *testing.T) {
4747
attr: attribute.Bool(key, true),
4848
want: attribute.Bool(key, true),
4949
},
50+
{
51+
limit: 0,
52+
attr: attribute.String(key, "\x80"),
53+
want: attribute.String(key, ""),
54+
},
5055
{
5156
limit: 0,
5257
attr: attribute.BoolSlice(key, []bool{true, false}),
@@ -787,6 +792,8 @@ func TestTruncateString(t *testing.T) {
787792
groups: []group{
788793
{0, "Some text", ""},
789794
{0, "", ""},
795+
{0, "\x80", ""},
796+
{0, "\uFFFD", ""},
790797
},
791798
},
792799
}

‎sdk/internal/attrnorm/truncate.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,10 @@ func truncate(limit int, s string) string {
359359
continue
360360
}
361361

362+
// A zero limit also truncates a leading invalid byte or U+FFFD.
363+
if limit == 0 {
364+
return ""
365+
}
362366
_, size := utf8.DecodeRuneInString(s[i:])
363367
if size == 1 {
364368
// Invalid encoding.

‎sdk/internal/attrnorm/truncate_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ func TestTruncateAttr(t *testing.T) {
4747
attr: attribute.Bool(key, true),
4848
want: attribute.Bool(key, true),
4949
},
50+
{
51+
limit: 0,
52+
attr: attribute.String(key, "\x80"),
53+
want: attribute.String(key, ""),
54+
},
5055
{
5156
limit: 0,
5257
attr: attribute.BoolSlice(key, []bool{true, false}),
@@ -787,6 +792,8 @@ func TestTruncateString(t *testing.T) {
787792
groups: []group{
788793
{0, "Some text", ""},
789794
{0, "", ""},
795+
{0, "\x80", ""},
796+
{0, "\uFFFD", ""},
790797
},
791798
},
792799
}

‎sdk/log/internal/attrnorm/truncate.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,10 @@ func truncate(limit int, s string) string {
359359
continue
360360
}
361361

362+
// A zero limit also truncates a leading invalid byte or U+FFFD.
363+
if limit == 0 {
364+
return ""
365+
}
362366
_, size := utf8.DecodeRuneInString(s[i:])
363367
if size == 1 {
364368
// Invalid encoding.

‎sdk/log/internal/attrnorm/truncate_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ func TestTruncateAttr(t *testing.T) {
4747
attr: attribute.Bool(key, true),
4848
want: attribute.Bool(key, true),
4949
},
50+
{
51+
limit: 0,
52+
attr: attribute.String(key, "\x80"),
53+
want: attribute.String(key, ""),
54+
},
5055
{
5156
limit: 0,
5257
attr: attribute.BoolSlice(key, []bool{true, false}),
@@ -787,6 +792,8 @@ func TestTruncateString(t *testing.T) {
787792
groups: []group{
788793
{0, "Some text", ""},
789794
{0, "", ""},
795+
{0, "\x80", ""},
796+
{0, "\uFFFD", ""},
790797
},
791798
},
792799
}

‎trace/auto.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,10 @@ func truncate(limit int, s string) string {
407407
continue
408408
}
409409

410+
// A zero limit also truncates a leading invalid byte or U+FFFD.
411+
if limit == 0 {
412+
return ""
413+
}
410414
_, size := utf8.DecodeRuneInString(s[i:])
411415
if size == 1 {
412416
// Invalid encoding.

‎trace/auto_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,8 @@ func TestTruncate(t *testing.T) {
10971097
groups: []group{
10981098
{0, "Some text", ""},
10991099
{0, "", ""},
1100+
{0, "\x80", ""},
1101+
{0, "\uFFFD", ""},
11001102
},
11011103
},
11021104
}

0 commit comments

Comments
 (0)