Remove AttributeKeyValue, make map insert/upsert safe by copying the given value

[bogdandrutu]

• Fix a bug in the setters for AttributeValue, the setter needs to overwrite the previous value with the default value.

[codecov-io]

Codecov Report

Merging #781 into master will decrease coverage by 0.04%.
The diff coverage is 80.64%.

@@            Coverage Diff             @@
##           master     #781      +/-   ##
==========================================
- Coverage   85.54%   85.49%   -0.05%     
==========================================
  Files         155      155              
  Lines       11523    11547      +24     
==========================================
+ Hits         9857     9872      +15     
  Misses       1295     1295              
- Partials      371      380       +9     

Impacted Files	Coverage Δ
translator/internaldata/oc_to_traces.go	82.38% <44.44%> (ø)
translator/internaldata/traces_to_oc.go	79.34% <66.66%> (ø)
internal/data/common.go	90.36% <83.69%> (-4.48%)	⬇️
translator/internaldata/resource_to_oc.go	71.01% <85.71%> (ø)
translator/internaldata/oc_to_resource.go	91.89% <90.00%> (ø)
receiver/opencensusreceiver/octrace/opencensus.go	93.33% <0.00%> (+3.33%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84326d9...0f3c3a5. Read the comment docs.

[tigrannajaryan]

Alternatively it is possible to do:

*a.orig = otlpcommon.AttributeKeyValue{Type:..., StringValue...}

This seems shorter and no need for copyValue call or for emptyAttributeKeyValue var.

[bogdandrutu]

Tried this idea:

func (a AttributeValue) SetStringVal(v string) {
	*a.orig = otlpcommon.AttributeKeyValue{Key: a.orig.Key, Type: otlpcommon.AttributeKeyValue_STRING, StringValue: v}
}

func (a AttributeValue) SetIntVal(v int64) {
	*a.orig = otlpcommon.AttributeKeyValue{Key: a.orig.Key, Type: otlpcommon.AttributeKeyValue_INT, IntValue: v}
}

func (a AttributeValue) SetDoubleVal(v float64) {
	*a.orig = otlpcommon.AttributeKeyValue{Key: a.orig.Key, Type: otlpcommon.AttributeKeyValue_DOUBLE, DoubleValue: v}
}

func (a AttributeValue) SetBoolVal(v bool) {
	*a.orig = otlpcommon.AttributeKeyValue{Key: a.orig.Key, Type: otlpcommon.AttributeKeyValue_BOOL, BoolValue: v}
}

Benchmarks with your proposal:

goos: darwin
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector/internal/data
BenchmarkSetIntVal
BenchmarkSetIntVal-16    	346741474	         3.50 ns/op
PASS

Process finished with exit code 0

Benchmarks with current implementation:

goos: darwin
goarch: amd64
pkg: github.com/open-telemetry/opentelemetry-collector/internal/data
BenchmarkSetIntVal
BenchmarkSetIntVal-16    	601141042	         1.93 ns/op
PASS

Process finished with exit code 0

Should we worry about this? Happy to change if you think makes code better.

[tigrannajaryan]

Strange. Let's keep your code, it is fine. This may be a frequent operation so let's not loose performance.

[tigrannajaryan]

What is the benefit of using InsertString vs Insert?

[bogdandrutu]

you don't need to do an extra allocation of the AttributeValue that will be thrown away anyway.

[bogdandrutu]

@tigrannajaryan PTAL