Skip to content

Commit

Permalink
Merge branch 'main' into rminternal
Browse files Browse the repository at this point in the history
  • Loading branch information
Alex Boten authored Mar 7, 2022
2 parents 29dd0f6 + f980c9e commit fa8397c
Show file tree
Hide file tree
Showing 15 changed files with 150 additions and 53 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/api-compatibility.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ jobs:
steps:

- name: Checkout-Main
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
ref: ${{ github.base_ref }}
path: ${{ github.base_ref }}

- name: Checkout-HEAD
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
path: ${{ github.head_ref }}

- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/build-and-test-windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ jobs:
runs-on: windows-latest
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down
20 changes: 10 additions & 10 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down Expand Up @@ -43,9 +43,9 @@ jobs:
needs: [setup-environment]
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down Expand Up @@ -74,9 +74,9 @@ jobs:
needs: [setup-environment]
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down Expand Up @@ -128,9 +128,9 @@ jobs:
needs: [setup-environment]
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down Expand Up @@ -161,9 +161,9 @@ jobs:
needs: [setup-environment]
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
- name: Setup Go Environment
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/builder-integration-test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ jobs:
steps:

- name: Set up Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17

- name: Check out code into the Go module directory
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Test
run: cd ./cmd/builder && ./test/test.sh
4 changes: 2 additions & 2 deletions .github/workflows/builder-release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ jobs:
steps:
-
name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
fetch-depth: 0
-
name: Set up Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17
-
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/changelog.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'dependencies') && !contains(github.event.pull_request.labels.*.name, 'Skip Changelog')}}

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v3

- name: Check for CHANGELOG changes
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/check-links.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
with:
fetch-depth: 0
- uses: gaurav-nelson/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Setup Go
uses: actions/setup-go@v2.2.0
uses: actions/setup-go@v3
with:
go-version: 1.17

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/contrib-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- name: Setup Permissions
run: sudo chmod -R 777 $GITHUB_WORKSPACE /github /__w/_temp
- name: Checkout Repo
uses: actions/checkout@v2
uses: actions/checkout@v3
- name: Run Contrib Tests
run: |
contrib_path=/tmp/opentelemetry-collector-contrib
Expand Down
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
### 🛑 Breaking changes 🛑

- Remove `Type` funcs in pdata (#4933)
- Rename `pdata.AttributeMap.Delete` to `pdata.AttributeMap.Remove` (#4914)
- pdata: deprecate funcs working with InternalRep (#4957)

## 🧰 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)

Expand Down
26 changes: 26 additions & 0 deletions model/internal/pdata/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
//
Expand Down
108 changes: 87 additions & 21 deletions model/internal/pdata/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package pdata

import (
"encoding/base64"
"fmt"
"strconv"
"testing"

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit fa8397c

Please sign in to comment.