Skip to content

Commit

Permalink
[pdata] Deprecate map.Upsert/Insert/Update
Browse files Browse the repository at this point in the history
  • Loading branch information
dmitryax committed Aug 27, 2022
1 parent 1eda0c4 commit 3f8ca9b
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
- Deprecate builder distribution flags, use configuration. (#5946)
- Enforce naming conventions for Invalid[Trace|Span]ID: (#5969)
- Deprecate funcs `pcommon.InvalidTraceID` and `pcommon.InvalidSpanID` in favor of vars `pcommon.EmptyTraceID` and `pcommon.EmptySpanID`
- Deprecate `Update` and `Upsert` methods of `pcommon.Map` (#5975)

### 💡 Enhancements 💡

Expand All @@ -48,6 +49,8 @@
- Change pdata generated types to use type definition instead of aliases. (#5936)
- Improves documentation, and makes code easier to read/understand.
- Log `InstrumentationScope` attributes in `loggingexporter` (#5976)
- Add `UpsertEmpty`, `UpsertEmptyMap` and `UpsertEmptySlice` methods to `pcommon.Map` (#5975)
- Add `SetEmptyMapVal` and `SetEmptySliceVal` methods to `pcommon.Value` (#5975)

### 🧰 Bug fixes 🧰

Expand Down
7 changes: 2 additions & 5 deletions exporter/loggingexporter/internal/otlptext/databuffer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,9 @@ func TestNestedArraySerializesCorrectly(t *testing.T) {
func TestNestedMapSerializesCorrectly(t *testing.T) {
ava := pcommon.NewValueMap()
av := ava.MapVal()
av.Insert("foo", pcommon.NewValueString("test"))
av.UpsertString("foo", "test")

ava2 := pcommon.NewValueMap()
av2 := ava2.MapVal()
av2.InsertInt("bar", 13)
av.Insert("zoo", ava2)
av.UpsertEmptyMap("zoo").UpsertInt("bar", 13)

expected := `{
-> foo: STRING(test)
Expand Down
52 changes: 52 additions & 0 deletions pdata/pcommon/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,22 @@ func (v Value) SetBytesVal(bv ImmutableByteSlice) {
v.getOrig().Value = &otlpcommon.AnyValue_BytesValue{BytesValue: bv.getOrig()}
}

// SetEmptyMapVal sets value to an empty map and returns it.
// Calling this function on zero-initialized Value will cause a panic.
func (v Value) SetEmptyMapVal() Map {
kv := &otlpcommon.AnyValue_KvlistValue{KvlistValue: &otlpcommon.KeyValueList{}}
v.getOrig().Value = kv
return newMap(&kv.KvlistValue.Values)
}

// SetEmptySliceVal sets value to an empty slice and returns it.
// Calling this function on zero-initialized Value will cause a panic.
func (v Value) SetEmptySliceVal() Slice {
av := &otlpcommon.AnyValue_ArrayValue{ArrayValue: &otlpcommon.ArrayValue{}}
v.getOrig().Value = av
return newSlice(&av.ArrayValue.Values)
}

// copyTo copies the value to Value. Will panic if dest is nil.
func (v Value) copyTo(dest *otlpcommon.AnyValue) {
switch ov := v.getOrig().Value.(type) {
Expand Down Expand Up @@ -648,6 +664,7 @@ func (m Map) RemoveIf(f func(string, Value) bool) {
//
// Important: this function should not be used if the caller has access to
// the raw value to avoid an extra allocation.
// NOTE: The method will be deprecated in 0.60.0. Use Get and CopyTo instead.
func (m Map) Insert(k string, v Value) {
if _, existing := m.Get(k); !existing {
*m.getOrig() = append(*m.getOrig(), newAttributeKeyValue(k, v))
Expand Down Expand Up @@ -710,6 +727,7 @@ func (m Map) InsertBytes(k string, v ImmutableByteSlice) {
//
// Important: this function should not be used if the caller has access to
// the raw value to avoid an extra allocation.
// Deprecated: [0.59.0] Use Get and CopyTo instead.
func (m Map) Update(k string, v Value) {
if av, existing := m.Get(k); existing {
v.copyTo(av.getOrig())
Expand Down Expand Up @@ -764,6 +782,7 @@ func (m Map) UpdateBytes(k string, v ImmutableByteSlice) {
//
// Important: this function should not be used if the caller has access to
// the raw value to avoid an extra allocation.
// Deprecated: [0.59.0] Use UpsertEmpty instead.
func (m Map) Upsert(k string, v Value) {
if av, existing := m.Get(k); existing {
v.copyTo(av.getOrig())
Expand All @@ -772,6 +791,17 @@ func (m Map) Upsert(k string, v Value) {
}
}

// UpsertEmpty inserts or updates an empty value to the map under given key
// and return the updated/inserted value.
func (m Map) UpsertEmpty(k string) Value {
if av, existing := m.Get(k); existing {
av.getOrig().Value = nil
return newValue(av.getOrig())
}
*m.getOrig() = append(*m.getOrig(), otlpcommon.KeyValue{Key: k})
return newValue(&(*m.getOrig())[len(*m.getOrig())-1].Value)
}

// UpsertString performs the Insert or Update action. The Value is
// inserted to the map that did not originally have the key. The key/value is
// updated to the map where the key already existed.
Expand Down Expand Up @@ -827,6 +857,28 @@ func (m Map) UpsertBytes(k string, v ImmutableByteSlice) {
}
}

// UpsertEmptyMap inserts or updates an empty map under given key and returns it.
func (m Map) UpsertEmptyMap(k string) Map {
kvl := otlpcommon.AnyValue_KvlistValue{KvlistValue: &otlpcommon.KeyValueList{Values: []otlpcommon.KeyValue(nil)}}
if av, existing := m.Get(k); existing {
av.getOrig().Value = &kvl
} else {
*m.getOrig() = append(*m.getOrig(), otlpcommon.KeyValue{Key: k, Value: otlpcommon.AnyValue{Value: &kvl}})
}
return Map(internal.NewMap(&kvl.KvlistValue.Values))
}

// UpsertEmptySlice inserts or updates an empty clice under given key and returns it.
func (m Map) UpsertEmptySlice(k string) Slice {
vl := otlpcommon.AnyValue_ArrayValue{ArrayValue: &otlpcommon.ArrayValue{Values: []otlpcommon.AnyValue(nil)}}
if av, existing := m.Get(k); existing {
av.getOrig().Value = &vl
} else {
*m.getOrig() = append(*m.getOrig(), otlpcommon.KeyValue{Key: k, Value: otlpcommon.AnyValue{Value: &vl}})
}
return Slice(internal.NewSlice(&vl.ArrayValue.Values))
}

// Sort sorts the entries in the Map so two instances can be compared.
// Returns the same instance to allow nicer code like:
//
Expand Down
71 changes: 71 additions & 0 deletions pdata/pcommon/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ func TestNilOrigSetValue(t *testing.T) {
av = NewValueEmpty()
av.SetBytesVal(NewImmutableByteSlice([]byte{1, 2, 3}))
assert.Equal(t, NewImmutableByteSlice([]byte{1, 2, 3}), av.BytesVal())

av = NewValueEmpty()
av.SetEmptyMapVal().UpsertString("k", "v")
assert.Equal(t, NewMapFromRaw(map[string]interface{}{"k": "v"}), av.MapVal())

av = NewValueEmpty()
av.SetEmptySliceVal().AppendEmpty().SetIntVal(1)
assert.Equal(t, NewSliceFromRaw([]interface{}{1}), av.SliceVal())
}

func TestValueEqual(t *testing.T) {
Expand Down Expand Up @@ -345,6 +353,69 @@ func TestMap(t *testing.T) {
assert.EqualValues(t, NewMap(), NewMap().Sort())
}

func TestMapUpsertEmpty(t *testing.T) {
m := NewMap()
v := m.UpsertEmpty("k1")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k1": nil,
}), m)

v.SetBoolVal(true)
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k1": true,
}), m)

v = m.UpsertEmpty("k1")
v.SetIntVal(1)
v2, ok := m.Get("k1")
assert.True(t, ok)
assert.Equal(t, int64(1), v2.IntVal())
}

func TestMapUpsertEmptyMap(t *testing.T) {
m := NewMap()
childMap := m.UpsertEmptyMap("k1")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k1": map[string]interface{}{},
}), m)
childMap.UpsertEmptySlice("k2").AppendEmpty().SetStringVal("val")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k1": map[string]interface{}{
"k2": []interface{}{"val"},
},
}), m)

childMap.UpsertEmptyMap("k2").UpsertInt("k3", 1)
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k1": map[string]interface{}{
"k2": map[string]interface{}{"k3": 1},
},
}), m)
}

func TestMapUpsertEmptySlice(t *testing.T) {
m := NewMap()
childSlice := m.UpsertEmptySlice("k")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k": []interface{}{},
}), m)
childSlice.AppendEmpty().SetDoubleVal(1.1)
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k": []interface{}{1.1},
}), m)

m.UpsertEmptySlice("k")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k": []interface{}{},
}), m)
childSliceVal, ok := m.Get("k")
assert.True(t, ok)
childSliceVal.SliceVal().AppendEmpty().SetEmptySliceVal().AppendEmpty().SetStringVal("val")
assert.EqualValues(t, NewMapFromRaw(map[string]interface{}{
"k": []interface{}{[]interface{}{"val"}},
}), m)
}

func TestMapWithEmpty(t *testing.T) {
origWithNil := []otlpcommon.KeyValue{
{},
Expand Down
14 changes: 6 additions & 8 deletions pdata/ptrace/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,12 @@ var tracesOTLPFull = func() Traces {
sp.Attributes().UpsertInt("int", 1)
sp.Attributes().UpsertDouble("double", 1.1)
sp.Attributes().UpsertBytes("bytes", pcommon.NewImmutableByteSlice([]byte("foo")))
arr := pcommon.NewValueSlice()
arr.SliceVal().AppendEmpty().SetIntVal(1)
arr.SliceVal().AppendEmpty().SetStringVal("str")
sp.Attributes().Upsert("array", arr)
kvList := pcommon.NewValueMap()
kvList.MapVal().Upsert("int", pcommon.NewValueInt(1))
kvList.MapVal().Upsert("string", pcommon.NewValueString("string"))
sp.Attributes().Upsert("kvList", kvList)
arr := sp.Attributes().UpsertEmptySlice("array")
arr.AppendEmpty().SetIntVal(1)
arr.AppendEmpty().SetStringVal("str")
kvList := sp.Attributes().UpsertEmptyMap("kvList")
kvList.UpsertInt("int", 1)
kvList.UpsertString("string", "string")
// Add events.
event := sp.Events().AppendEmpty()
event.SetName("eventName")
Expand Down

0 comments on commit 3f8ca9b

Please sign in to comment.