Fix slice-valued attributes when used as map keys #2223

Aneurysm9 · 2021-09-06T17:46:22Z

Signed-off-by: Anthony J Mirabella a9@aneurysm9.com

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

codecov · 2021-09-06T17:48:51Z

Codecov Report

Merging #2223 (fa25af5) into main (df2bdbb) will decrease coverage by 0.0%.
The diff coverage is 59.0%.

@@           Coverage Diff           @@
##            main   #2223     +/-   ##
=======================================
- Coverage   72.8%   72.7%   -0.1%     
=======================================
  Files        176     176             
  Lines      12178   12186      +8     
=======================================
  Hits        8866    8866             
- Misses      3070    3078      +8     
  Partials     242     242

Impacted Files	Coverage Δ
attribute/value.go	`88.8% <59.0%> (-4.7%)`	⬇️

XSAM

I am not sure we need to rewrite the whole logic of all Slice operations. The problem is Go can't hash []string, but it can hash *[]string. So if we store/get a pointer of the slice, we can solve this problem.

For instance, for the StringSlice:

func StringSliceValue(v []string) Value {
	cp := make([]string, len(v))
	copy(cp, v)
	return Value{
		vtype: STRINGSLICE,
-		slice: cp,
+		slice: &cp,
	}
}

func (v Value) AsStringSlice() []string {
-	if s, ok := v.slice.([]string); ok {
-		return s
+	if s, ok := v.slice.(*[]string); ok {
+		return *s
	}
	return nil
}

func (v Value) Emit() string {
	switch v.Type() {
	case ARRAY, BOOLSLICE, INT64SLICE, FLOAT64SLICE, STRINGSLICE:
-		return fmt.Sprint(v.slice)
+		return fmt.Sprint(reflect.ValueOf(v.slice).Elem())

Aneurysm9 · 2021-09-08T00:10:48Z

I think there is a secondary concern with the current implementation that this PR addresses which is that it does not freeze the representation of the provided slice as was done with the prior array implementation. This means that slice values can be changed after they are set as attributes and will change the attribute values, which may lead to unexpected results.

MrAlias · 2021-09-08T15:26:32Z

I think there is a secondary concern with the current implementation that this PR addresses which is that it does not freeze the representation of the provided slice as was done with the prior array implementation. This means that slice values can be changed after they are set as attributes and will change the attribute values, which may lead to unexpected results.

The passed slice values are copied to a new slice to prevent just this.

MrAlias · 2021-09-08T15:38:50Z

This looks to be adding an allocation to most of the slice methods. I'm guessing because references are escaping to the sliceValue function.

$ 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     	843200180	         1.362 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/KeyValue-8  	141956227	         8.504 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/AsBool-8    	835002997	         1.432 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Any-8       	56752633	        20.81 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Emit-8      	149114601	         8.439 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Array/KeyValue-8         	 6628058	       176.9 ns/op	       6 B/op	       2 allocs/op
BenchmarkBool/Array/Emit-8             	 4142497	       276.9 ns/op	      24 B/op	       1 allocs/op
BenchmarkBoolSlice/Value-8             	 5420106	       220.6 ns/op	      30 B/op	       3 allocs/op
BenchmarkBoolSlice/KeyValue-8          	 5002653	       295.3 ns/op	      30 B/op	       3 allocs/op
BenchmarkBoolSlice/AsBoolSlice-8       	19226403	        56.29 ns/op	       8 B/op	       1 allocs/op
BenchmarkBoolSlice/Any-8               	 4851915	       240.2 ns/op	      30 B/op	       3 allocs/op
BenchmarkBoolSlice/Emit-8              	 4447971	       270.8 ns/op	      24 B/op	       1 allocs/op
BenchmarkInt/Value-8                   	863892535	         1.382 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/KeyValue-8                	141507870	         8.492 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Any-8                     	60095469	        19.39 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Emit-8                    	100000000	        10.31 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Array/KeyValue-8          	 6309688	       190.2 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt/Array/Emit-8              	 4139844	       294.6 ns/op	      16 B/op	       1 allocs/op
BenchmarkIntSlice/Value-8              	 5289876	       228.9 ns/op	      72 B/op	       3 allocs/op
BenchmarkIntSlice/KeyValue-8           	 4230032	       255.3 ns/op	      72 B/op	       3 allocs/op
BenchmarkIntSlice/Any-8                	 4029014	       264.6 ns/op	      72 B/op	       3 allocs/op
BenchmarkIntSlice/Emit-8               	 4086672	       296.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkInt8/Any-8                    	56067631	        19.33 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt16/Any-8                   	62364078	        19.12 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt32/Any-8                   	58327666	        21.49 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Value-8                 	875788021	         1.437 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/KeyValue-8              	141532423	         8.448 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/AsInt64-8               	832411033	         1.478 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Any-8                   	59651652	        21.10 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Emit-8                  	100000000	        10.98 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Array/KeyValue-8        	 5856169	       247.7 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64/Array/Emit-8            	 3235882	       362.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkInt64Slice/Value-8            	 5114036	       235.6 ns/op	      72 B/op	       3 allocs/op
BenchmarkInt64Slice/KeyValue-8         	 4943851	       237.8 ns/op	      72 B/op	       3 allocs/op
BenchmarkInt64Slice/AsInt64Slice-8     	10167373	       116.0 ns/op	      56 B/op	       3 allocs/op
BenchmarkInt64Slice/Any-8              	 4635265	       264.1 ns/op	      72 B/op	       3 allocs/op
BenchmarkInt64Slice/Emit-8             	 3556449	       347.7 ns/op	      16 B/op	       1 allocs/op
BenchmarkFloat64/Value-8               	830878761	         1.455 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/KeyValue-8            	138633600	         8.516 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/AsFloat64-8           	795928496	         1.501 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Any-8                 	59382187	        24.10 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Emit-8                	 8588161	       139.5 ns/op	      16 B/op	       2 allocs/op
BenchmarkFloat64/Array/KeyValue-8      	 5874248	       207.6 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64/Array/Emit-8          	 2531536	       496.9 ns/op	      16 B/op	       1 allocs/op
BenchmarkFloat64Slice/Value-8          	 4871056	       356.5 ns/op	      72 B/op	       3 allocs/op
BenchmarkFloat64Slice/KeyValue-8       	 4563218	       253.2 ns/op	      72 B/op	       3 allocs/op
BenchmarkFloat64Slice/AsFloat64Slice-8 	 9809776	       124.2 ns/op	      56 B/op	       3 allocs/op
BenchmarkFloat64Slice/Any-8            	 4445758	       274.7 ns/op	      72 B/op	       3 allocs/op
BenchmarkFloat64Slice/Emit-8           	 2461478	       499.1 ns/op	      16 B/op	       1 allocs/op
BenchmarkString/Value-8                	177216351	         6.725 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/KeyValue-8             	127416421	         9.414 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/AsString-8             	648566374	         2.079 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Any-8                  	52950224	        23.96 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Emit-8                 	156119772	         8.155 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Array/KeyValue-8       	 4489150	       262.0 ns/op	      96 B/op	       2 allocs/op
BenchmarkString/Array/Emit-8           	 3600332	       337.2 ns/op	      48 B/op	       1 allocs/op
BenchmarkStringSlice/Value-8           	 3690963	       316.7 ns/op	     120 B/op	       3 allocs/op
BenchmarkStringSlice/KeyValue-8        	 3766921	       333.4 ns/op	     120 B/op	       3 allocs/op
BenchmarkStringSlice/AsStringSlice-8   	 4997282	       238.1 ns/op	     112 B/op	       3 allocs/op
BenchmarkStringSlice/Any-8             	 2928495	       397.7 ns/op	     120 B/op	       3 allocs/op
BenchmarkStringSlice/Emit-8            	 3196767	       416.4 ns/op	      48 B/op	       1 allocs/op
BenchmarkBytes/Any-8                   	56228438	        20.27 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	go.opentelemetry.io/otel/attribute	96.952s
go test -bench=.  101.63s user 1.59s system 106% cpu 1:37.29 total

main for reference:

$ 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     	836777030	         1.362 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/KeyValue-8  	142302812	         8.453 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/AsBool-8    	829789330	         1.440 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Any-8       	57961297	        20.32 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Emit-8      	151251918	         8.039 ns/op	       0 B/op	       0 allocs/op
BenchmarkBool/Array/KeyValue-8         	 6854890	       180.8 ns/op	       6 B/op	       2 allocs/op
BenchmarkBool/Array/Emit-8             	 4348897	       277.3 ns/op	      24 B/op	       1 allocs/op
BenchmarkBoolSlice/Value-8             	27577914	        43.28 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/KeyValue-8          	24781233	        49.58 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/AsBoolSlice-8       	518330706	         2.273 ns/op	       0 B/op	       0 allocs/op
BenchmarkBoolSlice/Any-8               	16825333	        71.73 ns/op	      27 B/op	       2 allocs/op
BenchmarkBoolSlice/Emit-8              	 3429951	       356.3 ns/op	      27 B/op	       4 allocs/op
BenchmarkInt/Value-8                   	872055820	         1.415 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/KeyValue-8                	142396792	         8.417 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Any-8                     	60824240	        20.68 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Emit-8                    	100000000	        12.04 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt/Array/KeyValue-8          	 4797216	       215.7 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt/Array/Emit-8              	 3560059	       331.1 ns/op	      16 B/op	       1 allocs/op
BenchmarkIntSlice/Value-8              	21344516	        59.54 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/KeyValue-8           	17772919	        66.70 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/Any-8                	14000749	        87.23 ns/op	      48 B/op	       2 allocs/op
BenchmarkIntSlice/Emit-8               	 2634862	       451.8 ns/op	      40 B/op	       4 allocs/op
BenchmarkInt8/Any-8                    	60368385	        20.26 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt16/Any-8                   	58757239	        21.29 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt32/Any-8                   	57048292	        22.17 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Value-8                 	880253342	         1.412 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/KeyValue-8              	142218999	         8.432 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/AsInt64-8               	802441760	         1.531 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Any-8                   	59352590	        21.40 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Emit-8                  	100000000	        10.58 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64/Array/KeyValue-8        	 6246181	       195.4 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64/Array/Emit-8            	 3535020	       334.1 ns/op	      16 B/op	       1 allocs/op
BenchmarkInt64Slice/Value-8            	23638441	        50.27 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/KeyValue-8         	20832636	        59.44 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/AsInt64Slice-8     	476564612	         2.406 ns/op	       0 B/op	       0 allocs/op
BenchmarkInt64Slice/Any-8              	15291912	        76.81 ns/op	      48 B/op	       2 allocs/op
BenchmarkInt64Slice/Emit-8             	 2635155	       450.9 ns/op	      40 B/op	       4 allocs/op
BenchmarkFloat64/Value-8               	852685413	         1.435 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/KeyValue-8            	142022947	         8.589 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/AsFloat64-8           	827919326	         1.511 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Any-8                 	61388091	        21.64 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64/Emit-8                	 8640360	       145.5 ns/op	      16 B/op	       2 allocs/op
BenchmarkFloat64/Array/KeyValue-8      	 5193076	       207.1 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64/Array/Emit-8          	 2415973	       496.5 ns/op	      16 B/op	       1 allocs/op
BenchmarkFloat64Slice/Value-8          	23691865	        50.90 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/KeyValue-8       	20460295	        57.43 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/AsFloat64Slice-8 	494869453	         2.541 ns/op	       0 B/op	       0 allocs/op
BenchmarkFloat64Slice/Any-8            	15183091	        80.96 ns/op	      48 B/op	       2 allocs/op
BenchmarkFloat64Slice/Emit-8           	 1959252	       626.9 ns/op	      40 B/op	       4 allocs/op
BenchmarkString/Value-8                	182819140	         6.571 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/KeyValue-8             	134711890	         8.999 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/AsString-8             	609514870	         2.034 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Any-8                  	54585226	        23.48 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Emit-8                 	164563641	         7.809 ns/op	       0 B/op	       0 allocs/op
BenchmarkString/Array/KeyValue-8       	 4976325	       249.6 ns/op	      96 B/op	       2 allocs/op
BenchmarkString/Array/Emit-8           	 3592950	       330.3 ns/op	      48 B/op	       1 allocs/op
BenchmarkStringSlice/Value-8           	13330621	        92.57 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/KeyValue-8        	12136434	        99.54 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/AsStringSlice-8   	493171647	         2.840 ns/op	       0 B/op	       0 allocs/op
BenchmarkStringSlice/Any-8             	 9076705	       131.2 ns/op	      72 B/op	       2 allocs/op
BenchmarkStringSlice/Emit-8            	 2652626	       455.4 ns/op	      96 B/op	       4 allocs/op
BenchmarkBytes/Any-8                   	65258142	        20.59 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	go.opentelemetry.io/otel/attribute	90.838s
go test -bench=.  96.13s user 1.49s system 107% cpu 1:31.18 total

attribute/value.go

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

Aneurysm9 · 2021-09-08T15:59:40Z

The allocations appear to be a shell game. The initial approach had one more allocation when creating and some number more for As*Slice, but 3 fewer allocations for Emit. The revised approach seems to only add one allocation for Emit.

goos: darwin
goarch: amd64
pkg: go.opentelemetry.io/otel/attribute
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkBool/Value-12          945428712                1.255 ns/op           0 B/op          0 allocs/op
BenchmarkBool/KeyValue-12       164952742                7.256 ns/op           0 B/op          0 allocs/op
BenchmarkBool/AsBool-12         1000000000               1.120 ns/op           0 B/op          0 allocs/op
BenchmarkBool/Any-12            49957537                23.70 ns/op            0 B/op          0 allocs/op
BenchmarkBool/Emit-12           149066998                8.066 ns/op           0 B/op          0 allocs/op
BenchmarkBool/Array/KeyValue-12                  8617027               138.9 ns/op             6 B/op          2 allocs/op
BenchmarkBool/Array/Emit-12                      5524315               218.2 ns/op            24 B/op          1 allocs/op
BenchmarkBoolSlice/Value-12                     31364276                36.58 ns/op           27 B/op          2 allocs/op
BenchmarkBoolSlice/KeyValue-12                  27678004                40.06 ns/op           27 B/op          2 allocs/op
BenchmarkBoolSlice/AsBoolSlice-12               663107059                1.801 ns/op           0 B/op          0 allocs/op
BenchmarkBoolSlice/Any-12                       19237486                60.60 ns/op           27 B/op          2 allocs/op
BenchmarkBoolSlice/Emit-12                       3307632               363.4 ns/op            51 B/op          5 allocs/op
BenchmarkInt/Value-12                           947344844                1.288 ns/op           0 B/op          0 allocs/op
BenchmarkInt/KeyValue-12                        165034988                7.266 ns/op           0 B/op          0 allocs/op
BenchmarkInt/Any-12                             51273104                22.55 ns/op            0 B/op          0 allocs/op
BenchmarkInt/Emit-12                            126149493                9.507 ns/op           0 B/op          0 allocs/op
BenchmarkInt/Array/KeyValue-12                   7746669               153.8 ns/op            48 B/op          2 allocs/op
BenchmarkInt/Array/Emit-12                       4821517               253.2 ns/op            16 B/op          1 allocs/op
BenchmarkIntSlice/Value-12                      23371071                50.87 ns/op           48 B/op          2 allocs/op
BenchmarkIntSlice/KeyValue-12                   21198273                56.74 ns/op           48 B/op          2 allocs/op
BenchmarkIntSlice/Any-12                        13785559                88.11 ns/op           48 B/op          2 allocs/op
BenchmarkIntSlice/Emit-12                        2809785               433.7 ns/op            64 B/op          5 allocs/op
BenchmarkInt8/Any-12                            50602508                23.12 ns/op            0 B/op          0 allocs/op
BenchmarkInt16/Any-12                           52675263                22.76 ns/op            0 B/op          0 allocs/op
BenchmarkInt32/Any-12                           50934777                23.52 ns/op            0 B/op          0 allocs/op
BenchmarkInt64/Value-12                         949211068                1.271 ns/op           0 B/op          0 allocs/op
BenchmarkInt64/KeyValue-12                      162428517                7.376 ns/op           0 B/op          0 allocs/op
BenchmarkInt64/AsInt64-12                       1000000000               1.140 ns/op           0 B/op          0 allocs/op
BenchmarkInt64/Any-12                           52908094                23.08 ns/op            0 B/op          0 allocs/op
BenchmarkInt64/Emit-12                          124959349                9.568 ns/op           0 B/op          0 allocs/op
BenchmarkInt64/Array/KeyValue-12                 7664266               155.2 ns/op            48 B/op          2 allocs/op
BenchmarkInt64/Array/Emit-12                     4691046               252.7 ns/op            16 B/op          1 allocs/op
BenchmarkInt64Slice/Value-12                    24218652                43.53 ns/op           48 B/op          2 allocs/op
BenchmarkInt64Slice/KeyValue-12                 24652048                49.76 ns/op           48 B/op          2 allocs/op
BenchmarkInt64Slice/AsInt64Slice-12             663842793                1.887 ns/op           0 B/op          0 allocs/op
BenchmarkInt64Slice/Any-12                      14292572                73.01 ns/op           48 B/op          2 allocs/op
BenchmarkInt64Slice/Emit-12                      2792497               458.9 ns/op            64 B/op          5 allocs/op
BenchmarkFloat64/Value-12                       951058573                1.268 ns/op           0 B/op          0 allocs/op
BenchmarkFloat64/KeyValue-12                    162319906                7.359 ns/op           0 B/op          0 allocs/op
BenchmarkFloat64/AsFloat64-12                   1000000000               1.140 ns/op           0 B/op          0 allocs/op
BenchmarkFloat64/Any-12                         51923562                23.10 ns/op            0 B/op          0 allocs/op
BenchmarkFloat64/Emit-12                         8894858               132.8 ns/op            16 B/op          2 allocs/op
BenchmarkFloat64/Array/KeyValue-12               7387184               159.2 ns/op            48 B/op          2 allocs/op
BenchmarkFloat64/Array/Emit-12                   2819343               426.5 ns/op            16 B/op          1 allocs/op
BenchmarkFloat64Slice/Value-12                  27424429                42.77 ns/op           48 B/op          2 allocs/op
BenchmarkFloat64Slice/KeyValue-12               23884056                48.20 ns/op           48 B/op          2 allocs/op
BenchmarkFloat64Slice/AsFloat64Slice-12         664038387                1.800 ns/op           0 B/op          0 allocs/op
BenchmarkFloat64Slice/Any-12                    17319286                68.54 ns/op           48 B/op          2 allocs/op
BenchmarkFloat64Slice/Emit-12                    1979728               599.6 ns/op            64 B/op          5 allocs/op
BenchmarkString/Value-12                        791629076                1.529 ns/op           0 B/op          0 allocs/op
BenchmarkString/KeyValue-12                     155188635                7.719 ns/op           0 B/op          0 allocs/op
BenchmarkString/AsString-12                     469816766                2.519 ns/op           0 B/op          0 allocs/op
BenchmarkString/Any-12                          46641709                24.82 ns/op            0 B/op          0 allocs/op
BenchmarkString/Emit-12                         157739887                7.667 ns/op           0 B/op          0 allocs/op
BenchmarkString/Array/KeyValue-12                6036362               197.1 ns/op            96 B/op          2 allocs/op
BenchmarkString/Array/Emit-12                    4987232               241.7 ns/op            48 B/op          1 allocs/op
BenchmarkStringSlice/Value-12                   14229850                84.40 ns/op           72 B/op          2 allocs/op
BenchmarkStringSlice/KeyValue-12                13919032                85.17 ns/op           72 B/op          2 allocs/op
BenchmarkStringSlice/AsStringSlice-12           662135470                1.796 ns/op           0 B/op          0 allocs/op
BenchmarkStringSlice/Any-12                     10364113               115.0 ns/op            72 B/op          2 allocs/op
BenchmarkStringSlice/Emit-12                     2856714               419.6 ns/op           120 B/op          5 allocs/op
BenchmarkBytes/Any-12                           69046149                17.34 ns/op            0 B/op          0 allocs/op
PASS
ok      go.opentelemetry.io/otel/attribute      89.608s

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

jmacd · 2021-09-08T21:50:53Z

Could we instead prevent whatever is doing so from using attribute.Value in a map key?

jmacd · 2021-09-08T21:59:30Z

I see that this is because an attribute.Distinct value is being used, which has a Resource value with a []string in it.
I think I like the proposed fix. It does add memory allocation, but the impact seems limited.
The metrics API would also panic, and this fix will cause it to not panic.

jmacd

(I don't think the performance of slice-valued attributes is important. The extra allocation cost may be exchanged for a smaller-size Value object.)

Aneurysm9 requested review from dashpole, evantorrie, jmacd, MadVikingGod, MrAlias, paivagustavo, pellared and XSAM as code owners September 6, 2021 17:46

Fix slice-valued attributes when used as map keys

fe29e9f

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

Aneurysm9 force-pushed the fix/slice_attrs branch from 5c4e702 to fe29e9f Compare September 6, 2021 17:47

XSAM reviewed Sep 7, 2021

View reviewed changes

MrAlias reviewed Sep 8, 2021

View reviewed changes

attribute/value.go Outdated Show resolved Hide resolved

store pointers to slice values to make them usable as map keys

adaedc1

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

Emit slice-typed attribute values as slices, not pointers to slices

c037114

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>

jmacd approved these changes Sep 8, 2021

View reviewed changes

XSAM approved these changes Sep 9, 2021

View reviewed changes

MrAlias approved these changes Sep 9, 2021

View reviewed changes

Merge branch 'main' into fix/slice_attrs

fa25af5

MrAlias merged commit eaacfaa into open-telemetry:main Sep 9, 2021

Aneurysm9 deleted the fix/slice_attrs branch September 9, 2021 16:06

MrAlias mentioned this pull request Sep 20, 2021

Prerelease v1.0.0 #2250

Merged

forsaken628 mentioned this pull request Jan 30, 2022

otlpmetric with slice-valued attributes not merge #2573

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix slice-valued attributes when used as map keys #2223

Fix slice-valued attributes when used as map keys #2223

Aneurysm9 commented Sep 6, 2021

codecov bot commented Sep 6, 2021 •

edited

XSAM left a comment

Aneurysm9 commented Sep 8, 2021

MrAlias commented Sep 8, 2021

MrAlias commented Sep 8, 2021

Aneurysm9 commented Sep 8, 2021

jmacd commented Sep 8, 2021

jmacd commented Sep 8, 2021

jmacd left a comment

Fix slice-valued attributes when used as map keys #2223

Fix slice-valued attributes when used as map keys #2223

Conversation

Aneurysm9 commented Sep 6, 2021

codecov bot commented Sep 6, 2021 • edited

Codecov Report

XSAM left a comment

Choose a reason for hiding this comment

Aneurysm9 commented Sep 8, 2021

MrAlias commented Sep 8, 2021

MrAlias commented Sep 8, 2021

Aneurysm9 commented Sep 8, 2021

jmacd commented Sep 8, 2021

jmacd commented Sep 8, 2021

jmacd left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 6, 2021 •

edited