Skip to content

Commit

Permalink
[pdata] Change outcome of misuse of oneof collection accessors. (#5034)
Browse files Browse the repository at this point in the history
Change misuse outcome of `pdata.AttributeMap.MapVal()` and `pdata.AttributeValueSlice.SliceVal()`
  function calls. In case of type mismatch, they now return an invalid zero-initialized instance
  instead of returning a valid detached collection.
  • Loading branch information
dmitryax authored Mar 22, 2022
1 parent 270e0cb commit 68374a6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 8 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
- Remove pdata `InternalRep` deprecated funcs (#5018)
- Remove service/defaultcomponents deprecated package (#5019)
- Remove deprecated UseOpenTelemetryForInternalMetrics (#5026)
- Change outcome of `pdata.Value.MapVal()` and `pdata.Value.SliceVal()` functions misuse. In case of
type mismatch, they now return an invalid zero-initialized instance instead of a detached
collection (#5034)

### 🚩 Deprecations 🚩

Expand Down
12 changes: 6 additions & 6 deletions model/internal/pdata/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,27 +233,27 @@ func (v Value) BoolVal() bool {
}

// MapVal returns the map value associated with this Value.
// If the Type() is not ValueTypeMap then returns an empty map. Note that modifying
// such empty map has no effect on this Value.
// If the Type() is not ValueTypeMap then returns an invalid map. Note that using
// such map can cause panic.
//
// Calling this function on zero-initialized Value will cause a panic.
func (v Value) MapVal() Map {
kvlist := v.orig.GetKvlistValue()
if kvlist == nil {
return NewMap()
return Map{}
}
return newMap(&kvlist.Values)
}

// SliceVal returns the slice value associated with this Value.
// If the Type() is not ValueTypeArray then returns an empty slice. Note that modifying
// such empty slice has no effect on this Value.
// If the Type() is not ValueTypeArray then returns an invalid slice. Note that using
// such slice can cause panic.
//
// Calling this function on zero-initialized Value will cause a panic.
func (v Value) SliceVal() Slice {
arr := v.orig.GetArrayValue()
if arr == nil {
return NewSlice()
return Slice{}
}
return newSlice(&arr.Values)
}
Expand Down
22 changes: 20 additions & 2 deletions model/internal/pdata/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func TestAttributeValueMap(t *testing.T) {
// Test nil KvlistValue case for MapVal() func.
orig := &otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_KvlistValue{KvlistValue: nil}}
m1 = Value{orig: orig}
assert.EqualValues(t, NewMap(), m1.MapVal())
assert.EqualValues(t, Map{}, m1.MapVal())
}

func TestNilOrigSetAttributeValue(t *testing.T) {
Expand Down Expand Up @@ -1010,7 +1010,7 @@ func TestAttributeValueArray(t *testing.T) {

// Test nil values case for SliceVal() func.
a1 = Value{orig: &otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_ArrayValue{ArrayValue: nil}}}
assert.EqualValues(t, NewSlice(), a1.SliceVal())
assert.EqualValues(t, Slice{}, a1.SliceVal())
}

func TestAttributeSliceWithNilValues(t *testing.T) {
Expand Down Expand Up @@ -1305,3 +1305,21 @@ func constructTestAttributeSubarray() Value {
value.SliceVal().AppendEmpty().SetStringVal("strTwo")
return value
}

func BenchmarkAttributeValueMapAccessor(b *testing.B) {
val := simpleValueMap()

b.ResetTimer()
for n := 0; n < b.N; n++ {
val.MapVal()
}
}

func BenchmarkAttributeValueArrayAccessor(b *testing.B) {
val := simpleValueArray()

b.ResetTimer()
for n := 0; n < b.N; n++ {
val.SliceVal()
}
}

0 comments on commit 68374a6

Please sign in to comment.