Fix sdk/log record attr value limit #6032

MrAlias · 2024-12-11T18:44:22Z

Copy of #5997

Correctness

set an attribute value length limit such that for each attribute value:

if it is a string, if it exceeds that limit (counting any character in it as 1), SDKs MUST truncate that value, so that its length is at most equal to the limit...

Our current implementation truncates on number of bytes not characters.

Unit tests are added/updated to validate this fix and prevent regressions.

Performance

goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
                        │ commit-e9c7aac2(old).txt │       commit-878043b9(new).txt       │
                        │          sec/op          │    sec/op     vs base                │
Truncate/Unlimited-8                 0.8323n ±  3%   0.7367n ± 3%  -11.49% (p=0.000 n=10)
Truncate/Zero-8                       1.923n ± 32%    1.359n ± 2%  -29.34% (p=0.000 n=10)
Truncate/Short-8                    14.6050n ±  4%   0.8785n ± 1%  -93.98% (p=0.000 n=10)
Truncate/ASCII-8                      8.205n ±  2%    3.601n ± 7%  -56.12% (p=0.000 n=10)
Truncate/ValidUTF-8-8                11.335n ±  1%    7.206n ± 1%  -36.43% (p=0.000 n=10)
Truncate/InvalidUTF-8-8               58.26n ±  1%    36.61n ± 1%  -37.17% (p=0.000 n=10)
Truncate/MixedUTF-8-8                 81.16n ±  1%    52.30n ± 1%  -35.56% (p=0.000 n=10)
geomean                               10.04n          4.601n       -54.16%

                        │ commit-e9c7aac2(old).txt │      commit-878043b9(new).txt       │
                        │           B/op           │    B/op     vs base                 │
Truncate/Unlimited-8                  0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/Zero-8                       0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/Short-8                      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/ASCII-8                      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/ValidUTF-8-8                 0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/InvalidUTF-8-8               16.00 ± 0%     16.00 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/MixedUTF-8-8                 32.00 ± 0%     32.00 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                          ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                        │ commit-e9c7aac2(old).txt │      commit-878043b9(new).txt       │
                        │        allocs/op         │ allocs/op   vs base                 │
Truncate/Unlimited-8                  0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/Zero-8                       0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/Short-8                      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/ASCII-8                      0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/ValidUTF-8-8                 0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/InvalidUTF-8-8               1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
Truncate/MixedUTF-8-8                 1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                          ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

codecov · 2024-12-11T18:47:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.2%. Comparing base (58fdf2a) to head (878043b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #6032   +/-   ##
=====================================
  Coverage   82.2%   82.2%           
=====================================
  Files        273     273           
  Lines      23647   23674   +27     
=====================================
+ Hits       19449   19476   +27     
  Misses      3851    3851           
  Partials     347     347

see 1 file with indirect coverage changes

Truncate based on characters not byte length.

### Added - Add `Reset` method to `SpanRecorder` in `go.opentelemetry.io/otel/sdk/trace/tracetest`. (#5994) - Add `EnabledInstrument` interface in `go.opentelemetry.io/otel/sdk/metric/internal/x`. This is an experimental interface that is implemented by synchronous instruments provided by `go.opentelemetry.io/otel/sdk/metric`. Users can use it to avoid performing computationally expensive operations when recording measurements. It does not fall within the scope of the OpenTelemetry Go versioning and stability [policy](./VERSIONING.md) and it may be changed in backwards incompatible ways or removed in feature releases. (#6016) ### Changed - The default global API now supports full auto-instrumentation from the `go.opentelemetry.io/auto` package. See that package for more information. (#5920) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5929) - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5929) - Performance improvements for attribute value `AsStringSlice`, `AsFloat64Slice`, `AsInt64Slice`, `AsBoolSlice`. (#6011) - Change `EnabledParameters` to have a `Severity` field instead of a getter and setter in `go.opentelemetry.io/otel/log`. (#6009) ### Fixed - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5954) - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5954) - Fix inconsistent request body closing in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5954) - Fix invalid exemplar keys in `go.opentelemetry.io/otel/exporters/prometheus`. (#5995) - Fix attribute value truncation in `go.opentelemetry.io/otel/sdk/trace`. (#5997) - Fix attribute value truncation in `go.opentelemetry.io/otel/sdk/log`. (#6032)

MrAlias added this to the v1.33.0 milestone Dec 11, 2024

MrAlias force-pushed the log-attr-val-truncate branch from 3c1df34 to 9fd0b09 Compare December 11, 2024 18:45

MrAlias added 2 commits December 11, 2024 10:50

Update TestTruncate to validate on char (not byte)

e9c7aac

Fix sdk/log record attr value limit

878043b

Truncate based on characters not byte length.

MrAlias force-pushed the log-attr-val-truncate branch from 9fd0b09 to 878043b Compare December 11, 2024 19:17

MrAlias added bug Something isn't working pkg:SDK Related to an SDK package area:logs Part of OpenTelemetry logs labels Dec 11, 2024

MrAlias marked this pull request as ready for review December 11, 2024 19:18

MrAlias requested review from XSAM, dashpole, pellared and dmathieu as code owners December 11, 2024 19:18

pellared approved these changes Dec 11, 2024

View reviewed changes

XSAM approved these changes Dec 12, 2024

View reviewed changes

dmathieu approved these changes Dec 12, 2024

View reviewed changes

pellared merged commit aa95895 into open-telemetry:main Dec 12, 2024
37 of 38 checks passed

MrAlias deleted the log-attr-val-truncate branch December 12, 2024 15:31

MrAlias mentioned this pull request Dec 12, 2024

Release v1.33.0 #6035

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sdk/log record attr value limit #6032

Fix sdk/log record attr value limit #6032

MrAlias commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading

Fix sdk/log record attr value limit #6032

Fix sdk/log record attr value limit #6032

Conversation

MrAlias commented Dec 11, 2024 • edited Loading

Correctness

Performance

codecov bot commented Dec 11, 2024 • edited Loading

Codecov Report

MrAlias commented Dec 11, 2024 •

edited

Loading

codecov bot commented Dec 11, 2024 •

edited

Loading