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

Deprecate Array attribute in favor of *Slice types #2162

Merged
merged 11 commits into from Aug 12, 2021

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Aug 5, 2021

Resolves #2158

Add *Slice attribute types as a replacement for the deprecated Array type.

Benchmark Results

To compare the existing Array type with the new *Slice types the */Array/KeyValue benchmark shows the "before" and the equivalent *Slice/Value shows the "after" for the encoding. Similarly for the */Array/Emit and *Slice/Emit.

$ go test -bench=.
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/attribute
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
BenchmarkBool/Value-8     	832225669	         1.363 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/KeyValue-8  	141931221	         8.458 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/AsBool-8    	833652847	         1.439 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Any-8       	58587181	        20.87 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Emit-8      	149373596	         7.977 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Array/KeyValue-8         	 6737637	       175.4 ns/op	       6 B/op	       2 allocs/op
BenchmarkBool/Array/Emit-8             	 4434680	       271.0 ns/op	      24 B/op	       1 allocs/op
BenchmarkBoolSlice/Value-8             	26688757	        46.21 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/KeyValue-8          	22950921	        49.40 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/AsBoolSlice-8       	525599371	         2.272 ns/op	       0 B/op	       0 allocs/op
BenchmarkBoolSlice/Any-8               	16773865	        71.35 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/Emit-8              	 3257484	       348.7 ns/op	      27 B/op	       4 allocs/op
BenchmarkInt/Value-8                   	877902837	         1.364 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/KeyValue-8                	142272414	         8.431 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Any-8                     	60339177	        19.81 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Emit-8                    	100000000	        10.14 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Array/KeyValue-8          	 6178926	       201.1 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt/Array/Emit-8              	 3916032	       301.4 ns/op	      16 B/op	       1 allocs/op
BenchmarkIntSlice/Value-8              	21200500	        55.04 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/KeyValue-8           	17387926	        65.81 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/Any-8                	13910582	        86.29 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/Emit-8               	 2681140	       439.6 ns/op	      40 B/op	       4 allocs/op
BenchmarkInt8/Any-8                    	61987556	        20.22 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt16/Any-8                   	59654839	        20.37 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt32/Any-8                   	57865954	        20.38 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Value-8                 	806119342	         1.358 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/KeyValue-8              	141895544	         8.436 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/AsInt64-8               	830603088	         1.483 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Any-8                   	57967998	        21.18 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Emit-8                  	100000000	        10.36 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Array/KeyValue-8        	 6219757	       196.6 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64/Array/Emit-8            	 3721778	       323.0 ns/op	      16 B/op	       1 allocs/op
BenchmarkInt64Slice/Value-8            	23372196	        51.28 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/KeyValue-8         	18154530	        58.24 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/AsInt64Slice-8     	494594601	         2.464 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64Slice/Any-8              	15182502	        78.44 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/Emit-8             	 2585980	       461.1 ns/op	      40 B/op	       4 allocs/op
BenchmarkFloat64/Value-8               	875715577	         1.437 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/KeyValue-8            	142405081	         8.438 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/AsFloat64-8           	821869407	         1.407 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Any-8                 	60712789	        20.73 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Emit-8                	 8718868	       137.9 ns/op	      16 B/op	       2 allocs/op
BenchmarkFloat64/Array/KeyValue-8      	 5793618	       200.9 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64/Array/Emit-8          	 2527147	       472.9 ns/op	      16 B/op	       1 allocs/op
BenchmarkFloat64Slice/Value-8          	22137026	        53.27 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/KeyValue-8       	18504712	        66.35 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/AsFloat64Slice-8 	510276062	         2.478 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64Slice/Any-8            	15239098	        89.51 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/Emit-8           	 1998475	       596.2 ns/op	      40 B/op	       4 allocs/op
BenchmarkString/Value-8                	183353606	         6.529 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/KeyValue-8             	134076531	         8.963 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/AsString-8             	630018024	         2.001 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Any-8                  	53771318	        23.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Emit-8                 	168047347	         7.531 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Array/KeyValue-8       	 4995198	       244.7 ns/op	      96 B/op	       2 allocs/op
BenchmarkString/Array/Emit-8           	 3748335	       319.4 ns/op	      48 B/op	       1 allocs/op
BenchmarkStringSlice/Value-8           	12286879	        97.78 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/KeyValue-8        	12281880	        97.38 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/AsStringSlice-8   	486037562	         2.488 ns/op	       0 B/op	       0 allocs/op
BenchmarkStringSlice/Any-8             	 9544404	       126.2 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/Emit-8            	 2722761	       447.6 ns/op	      96 B/op	       4 allocs/op
BenchmarkBytes/Any-8                   	36801850	        32.62 ns/op	       0 B/op	       0 allocs/op

The take-away:

  • The encoding of []bool types takes a bit more memory, []string take a bit less, and number types are equivalent.
  • There is no impact to memory allocations for encoding.
  • Decoding with Emit users more memory and allocations. These extra allocations seem to be coming from the fmt.Sprint function and the resizing of the underlying buffer it uses to build the string representation. I built a PoC for a BoolSlice that optimized the buffer size prior to build a string directly and saw the allocations drop back to down. This seems like a place for future optimization if it is determined worth it.

@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #2162 (54384df) into main (df384a9) will increase coverage by 0.1%.
The diff coverage is 83.2%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2162     +/-   ##
=======================================
+ Coverage   72.6%   72.8%   +0.1%     
=======================================
  Files        177     177             
  Lines      12000   12140    +140     
=======================================
+ Hits        8719    8840    +121     
- Misses      3043    3063     +20     
+ Partials     238     237      -1     
Impacted Files Coverage Δ
attribute/kv.go 55.3% <0.0%> (-34.7%) ⬇️
attribute/type_string.go 40.0% <ø> (ø)
exporters/zipkin/model.go 94.3% <27.2%> (-5.1%) ⬇️
attribute/value.go 93.4% <91.3%> (+0.2%) ⬆️
attribute/key.go 100.0% <100.0%> (ø)
exporters/jaeger/jaeger.go 94.3% <100.0%> (ø)
...p/otlpmetric/internal/metrictransform/attribute.go 100.0% <100.0%> (+7.7%) ⬆️
...tlp/otlptrace/internal/tracetransform/attribute.go 100.0% <100.0%> (+7.7%) ⬆️
sdk/resource/process.go 100.0% <100.0%> (ø)
...s/otlp/otlptrace/internal/connection/connection.go 16.4% <0.0%> (+1.5%) ⬆️

@MrAlias MrAlias added this to In progress in OpenTelemetry Go 1.0.0 via automation Aug 5, 2021
@MrAlias MrAlias force-pushed the array-attr branch 2 times, most recently from 3126a20 to 552c59c Compare August 6, 2021 00:01
@MrAlias
Copy link
Contributor Author

MrAlias commented Aug 6, 2021

I'm going to split off some of the cleanup tasks into other PRs to reduce the size here.

OpenTelemetry Go 1.0.0 automation moved this from In progress to Reviewer approved Aug 11, 2021
attribute/kv.go Outdated Show resolved Hide resolved
@MrAlias MrAlias merged commit 87d09df into open-telemetry:main Aug 12, 2021
OpenTelemetry Go 1.0.0 automation moved this from Reviewer approved to Done Aug 12, 2021
@MrAlias MrAlias deleted the array-attr branch August 12, 2021 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Array attribute value type design leads to dropped user data
4 participants