Add RemoveIf to attribute map, and rename Delete to Remove (open-tele…

…metry#4914)

* add DeleteIf to attribute map

* change Delete to Remove

* add deprecation version

* remove TODO, which isn't accurate

CHANGELOG.md

@@ -5,8 +5,13 @@
 ### 🛑 Breaking changes 🛑
 
 - Remove `Type` funcs in pdata (#4933)
+- Rename `pdata.AttributeMap.Delete` to `pdata.AttributeMap.Remove` (#4914)
 
-## 🧰 Bug fixes 🧰
+### 💡 Enhancements 💡
+
+- Add `pdata.AttributeMap.RemoveIf`, which is a more performant way to remove multiple keys (#4914)
+
+### 🧰 Bug fixes 🧰
 
 - Collector `Run` will now exit when a context cancels (#4954)
 

model/internal/pdata/common.go

@@ -513,7 +513,14 @@ func (am AttributeMap) Get(key string) (AttributeValue, bool) {
 
 // Delete deletes the entry associated with the key and returns true if the key
 // was present in the map, otherwise returns false.
+// Deprecated: [v0.46.0] Use Remove instead.
 func (am AttributeMap) Delete(key string) bool {
+	return am.Remove(key)
+}
+
+// Remove removes the entry associated with the key and returns true if the key
+// was present in the map, otherwise returns false.
+func (am AttributeMap) Remove(key string) bool {
 	for i := range *am.orig {
 		akv := &(*am.orig)[i]
 		if akv.Key == key {
@@ -525,6 +532,25 @@ func (am AttributeMap) Delete(key string) bool {
 	return false
 }
 
+// RemoveIf removes the entries for which the function in question returns true
+func (am AttributeMap) RemoveIf(f func(string, AttributeValue) bool) {
+	newLen := 0
+	for i := 0; i < len(*am.orig); i++ {
+		akv := &(*am.orig)[i]
+		if f(akv.Key, AttributeValue{&akv.Value}) {
+			continue
+		}
+		if newLen == i {
+			// Nothing to move, element is at the right place.
+			newLen++
+			continue
+		}
+		(*am.orig)[newLen] = (*am.orig)[i]
+		newLen++
+	}
+	*am.orig = (*am.orig)[:newLen]
+}
+
 // Insert adds the AttributeValue to the map when the key does not exist.
 // No action is applied to the map where the key already exists.
 //

model/internal/pdata/common_test.go

@@ -16,6 +16,7 @@ package pdata
 
 import (
 	"encoding/base64"
+	"fmt"
 	"strconv"
 	"testing"
 
@@ -137,14 +138,14 @@ func TestAttributeValueMap(t *testing.T) {
 	require.True(t, exists)
 	assert.Equal(t, NewAttributeValueString("somestr2"), got)
 
-	deleted := m1.MapVal().Delete("double_key")
-	assert.True(t, deleted)
+	removed := m1.MapVal().Remove("double_key")
+	assert.True(t, removed)
 	assert.EqualValues(t, 1, m1.MapVal().Len())
 	_, exists = m1.MapVal().Get("double_key")
 	assert.False(t, exists)
 
-	deleted = m1.MapVal().Delete("child_map")
-	assert.True(t, deleted)
+	removed = m1.MapVal().Remove("child_map")
+	assert.True(t, removed)
 	assert.EqualValues(t, 0, m1.MapVal().Len())
 	_, exists = m1.MapVal().Get("child_map")
 	assert.False(t, exists)
@@ -347,9 +348,9 @@ func TestNilAttributeMap(t *testing.T) {
 	upsertMapBytes.UpsertBytes("k", []byte{1, 2, 3, 4, 5})
 	assert.EqualValues(t, generateTestBytesAttributeMap(), upsertMapBytes)
 
-	deleteMap := NewAttributeMap()
-	assert.False(t, deleteMap.Delete("k"))
-	assert.EqualValues(t, NewAttributeMap(), deleteMap)
+	removeMap := NewAttributeMap()
+	assert.False(t, removeMap.Remove("k"))
+	assert.EqualValues(t, NewAttributeMap(), removeMap)
 
 	// Test Sort
 	assert.EqualValues(t, NewAttributeMap(), NewAttributeMap().Sort())
@@ -524,20 +525,20 @@ func TestAttributeMapWithEmpty(t *testing.T) {
 	assert.EqualValues(t, AttributeValueTypeBytes, val.Type())
 	assert.EqualValues(t, []byte{1}, val.BytesVal())
 
-	assert.True(t, sm.Delete("other_key"))
-	assert.True(t, sm.Delete("other_key_string"))
-	assert.True(t, sm.Delete("other_key_int"))
-	assert.True(t, sm.Delete("other_key_double"))
-	assert.True(t, sm.Delete("other_key_bool"))
-	assert.True(t, sm.Delete("other_key_bytes"))
-	assert.True(t, sm.Delete("yet_another_key"))
-	assert.True(t, sm.Delete("yet_another_key_string"))
-	assert.True(t, sm.Delete("yet_another_key_int"))
-	assert.True(t, sm.Delete("yet_another_key_double"))
-	assert.True(t, sm.Delete("yet_another_key_bool"))
-	assert.True(t, sm.Delete("yet_another_key_bytes"))
-	assert.False(t, sm.Delete("other_key"))
-	assert.False(t, sm.Delete("yet_another_key"))
+	assert.True(t, sm.Remove("other_key"))
+	assert.True(t, sm.Remove("other_key_string"))
+	assert.True(t, sm.Remove("other_key_int"))
+	assert.True(t, sm.Remove("other_key_double"))
+	assert.True(t, sm.Remove("other_key_bool"))
+	assert.True(t, sm.Remove("other_key_bytes"))
+	assert.True(t, sm.Remove("yet_another_key"))
+	assert.True(t, sm.Remove("yet_another_key_string"))
+	assert.True(t, sm.Remove("yet_another_key_int"))
+	assert.True(t, sm.Remove("yet_another_key_double"))
+	assert.True(t, sm.Remove("yet_another_key_bool"))
+	assert.True(t, sm.Remove("yet_another_key_bytes"))
+	assert.False(t, sm.Remove("other_key"))
+	assert.False(t, sm.Remove("yet_another_key"))
 
 	// Test that the initial key is still there.
 	val, exist = sm.Get("test_key")
@@ -730,6 +731,30 @@ func TestAttributeMap_Clear(t *testing.T) {
 	assert.Nil(t, *am.orig)
 }
 
+func TestAttributeMap_RemoveIf(t *testing.T) {
+	rawMap := map[string]AttributeValue{
+		"k_string": NewAttributeValueString("123"),
+		"k_int":    NewAttributeValueInt(123),
+		"k_double": NewAttributeValueDouble(1.23),
+		"k_bool":   NewAttributeValueBool(true),
+		"k_empty":  NewAttributeValueEmpty(),
+		"k_bytes":  NewAttributeValueBytes([]byte{}),
+	}
+	am := NewAttributeMapFromMap(rawMap)
+	assert.Equal(t, 6, am.Len())
+
+	am.RemoveIf(func(key string, val AttributeValue) bool {
+		return key == "k_int" || val.Type() == AttributeValueTypeBool
+	})
+	assert.Equal(t, 4, am.Len())
+	_, exists := am.Get("k_string")
+	assert.True(t, exists)
+	_, exists = am.Get("k_bool")
+	assert.False(t, exists)
+	_, exists = am.Get("k_int")
+	assert.False(t, exists)
+}
+
 func BenchmarkAttributeValue_CopyTo(b *testing.B) {
 	av := NewAttributeValueString("k")
 	c := NewAttributeValueInt(123)
@@ -802,6 +827,47 @@ func BenchmarkAttributeMap_RangeOverMap(b *testing.B) {
 	}
 }
 
+func BenchmarkAttributeMap_Remove(b *testing.B) {
+	b.StopTimer()
+	// Remove all of the even keys
+	keysToRemove := map[string]struct{}{}
+	for j := 0; j < 50; j++ {
+		keysToRemove[fmt.Sprintf("%d", j*2)] = struct{}{}
+	}
+	for i := 0; i < b.N; i++ {
+		m := NewAttributeMap()
+		for j := 0; j < 100; j++ {
+			m.InsertString(fmt.Sprintf("%d", j), "string value")
+		}
+		b.StartTimer()
+		for k := range keysToRemove {
+			m.Remove(k)
+		}
+		b.StopTimer()
+	}
+}
+
+func BenchmarkAttributeMap_RemoveIf(b *testing.B) {
+	b.StopTimer()
+	// Remove all of the even keys
+	keysToRemove := map[string]struct{}{}
+	for j := 0; j < 50; j++ {
+		keysToRemove[fmt.Sprintf("%d", j*2)] = struct{}{}
+	}
+	for i := 0; i < b.N; i++ {
+		m := NewAttributeMap()
+		for j := 0; j < 100; j++ {
+			m.InsertString(fmt.Sprintf("%d", j), "string value")
+		}
+		b.StartTimer()
+		m.RemoveIf(func(key string, _ AttributeValue) bool {
+			_, remove := keysToRemove[key]
+			return remove
+		})
+		b.StopTimer()
+	}
+}
+
 func BenchmarkStringMap_RangeOverMap(b *testing.B) {
 	const numElements = 20
 	rawOrig := make(map[string]string, numElements)

model/internal/pdata/metrics_test.go

@@ -357,7 +357,7 @@ func TestOtlpToFromInternalGaugeMutating(t *testing.T) {
 	assert.EqualValues(t, endTime+1, gaugeDataPoints.At(0).Timestamp())
 	gaugeDataPoints.At(0).SetDoubleVal(124.1)
 	assert.EqualValues(t, 124.1, gaugeDataPoints.At(0).DoubleVal())
-	gaugeDataPoints.At(0).Attributes().Delete("key0")
+	gaugeDataPoints.At(0).Attributes().Remove("key0")
 	gaugeDataPoints.At(0).Attributes().UpsertString("k", "v")
 	assert.EqualValues(t, newAttributes, gaugeDataPoints.At(0).Attributes())
 
@@ -441,7 +441,7 @@ func TestOtlpToFromInternalSumMutating(t *testing.T) {
 	assert.EqualValues(t, endTime+1, doubleDataPoints.At(0).Timestamp())
 	doubleDataPoints.At(0).SetDoubleVal(124.1)
 	assert.EqualValues(t, 124.1, doubleDataPoints.At(0).DoubleVal())
-	doubleDataPoints.At(0).Attributes().Delete("key0")
+	doubleDataPoints.At(0).Attributes().Remove("key0")
 	doubleDataPoints.At(0).Attributes().UpsertString("k", "v")
 	assert.EqualValues(t, newAttributes, doubleDataPoints.At(0).Attributes())
 
@@ -524,7 +524,7 @@ func TestOtlpToFromInternalHistogramMutating(t *testing.T) {
 	assert.EqualValues(t, startTime+1, histogramDataPoints.At(0).StartTimestamp())
 	histogramDataPoints.At(0).SetTimestamp(Timestamp(endTime + 1))
 	assert.EqualValues(t, endTime+1, histogramDataPoints.At(0).Timestamp())
-	histogramDataPoints.At(0).Attributes().Delete("key0")
+	histogramDataPoints.At(0).Attributes().Remove("key0")
 	histogramDataPoints.At(0).Attributes().UpsertString("k", "v")
 	assert.EqualValues(t, newAttributes, histogramDataPoints.At(0).Attributes())
 	histogramDataPoints.At(0).SetExplicitBounds([]float64{1})
@@ -608,7 +608,7 @@ func TestOtlpToFromInternalExponentialHistogramMutating(t *testing.T) {
 	assert.EqualValues(t, startTime+1, histogramDataPoints.At(0).StartTimestamp())
 	histogramDataPoints.At(0).SetTimestamp(Timestamp(endTime + 1))
 	assert.EqualValues(t, endTime+1, histogramDataPoints.At(0).Timestamp())
-	histogramDataPoints.At(0).Attributes().Delete("key0")
+	histogramDataPoints.At(0).Attributes().Remove("key0")
 	histogramDataPoints.At(0).Attributes().UpsertString("k", "v")
 	assert.EqualValues(t, newAttributes, histogramDataPoints.At(0).Attributes())
 	// Test that everything is updated.