diff --git a/cmd/otel-allocator/allocation/allocatortest.go b/cmd/otel-allocator/allocation/allocatortest.go index 88312a80a5..f50c1aaacd 100644 --- a/cmd/otel-allocator/allocation/allocatortest.go +++ b/cmd/otel-allocator/allocation/allocatortest.go @@ -23,6 +23,19 @@ import ( "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target" ) +func MakeNNewTargetsWithoutPreAssigningCollectors(n int, startingIndex int) map[string]*target.Item { + toReturn := map[string]*target.Item{} + for i := startingIndex; i < n+startingIndex; i++ { + label := model.LabelSet{ + "i": model.LabelValue(strconv.Itoa(i)), + "total": model.LabelValue(strconv.Itoa(n + startingIndex)), + } + newTarget := target.NewItem(fmt.Sprintf("test-job-%d", i), fmt.Sprintf("test-url-%d", i), label, "") + toReturn[newTarget.Hash()] = newTarget + } + return toReturn +} + func colIndex(index, numCols int) int { if numCols == 0 { return -1 @@ -30,7 +43,7 @@ func colIndex(index, numCols int) int { return index % numCols } -func MakeNNewTargets(n int, numCollectors int, startingIndex int) map[string]*target.Item { +func MakeNNewTargetsWithPreAssigningCollectors(n int, numCollectors int, startingIndex int) map[string]*target.Item { toReturn := map[string]*target.Item{} for i := startingIndex; i < n+startingIndex; i++ { collector := fmt.Sprintf("collector-%d", colIndex(i, numCollectors)) diff --git a/cmd/otel-allocator/allocation/consistent_hashing_test.go b/cmd/otel-allocator/allocation/consistent_hashing_test.go index bbd4295202..3f9c4a4040 100644 --- a/cmd/otel-allocator/allocation/consistent_hashing_test.go +++ b/cmd/otel-allocator/allocation/consistent_hashing_test.go @@ -24,7 +24,7 @@ func TestCanSetSingleTarget(t *testing.T) { cols := MakeNCollectors(3, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) - c.SetTargets(MakeNNewTargets(1, 3, 0)) + c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(1, 3, 0)) actualTargetItems := c.TargetItems() assert.Len(t, actualTargetItems, 1) for _, item := range actualTargetItems { @@ -40,7 +40,7 @@ func TestRelativelyEvenDistribution(t *testing.T) { expectedDelta := (expectedPerCollector * 1.5) - expectedPerCollector c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) - c.SetTargets(MakeNNewTargets(numItems, 0, 0)) + c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(numItems, 0, 0)) actualTargetItems := c.TargetItems() assert.Len(t, actualTargetItems, numItems) actualCollectors := c.Collectors() @@ -54,7 +54,7 @@ func TestFullReallocation(t *testing.T) { cols := MakeNCollectors(10, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) - c.SetTargets(MakeNNewTargets(10000, 10, 0)) + c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(10000, 10, 0)) actualTargetItems := c.TargetItems() assert.Len(t, actualTargetItems, 10000) actualCollectors := c.Collectors() @@ -79,7 +79,7 @@ func TestNumRemapped(t *testing.T) { cols := MakeNCollectors(numInitialCols, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) - c.SetTargets(MakeNNewTargets(numItems, numInitialCols, 0)) + c.SetTargets(MakeNNewTargetsWithPreAssigningCollectors(numItems, numInitialCols, 0)) actualTargetItems := c.TargetItems() assert.Len(t, actualTargetItems, numItems) actualCollectors := c.Collectors() diff --git a/cmd/otel-allocator/allocation/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted_test.go index 417c0e5ed3..8acde5c9e7 100644 --- a/cmd/otel-allocator/allocation/least_weighted_test.go +++ b/cmd/otel-allocator/allocation/least_weighted_test.go @@ -15,6 +15,7 @@ package allocation import ( + "fmt" "math" "math/rand" "testing" @@ -50,7 +51,7 @@ func TestAddingAndRemovingTargets(t *testing.T) { cols := MakeNCollectors(3, 0) s.SetCollectors(cols) - initTargets := MakeNNewTargets(6, 3, 0) + initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 3, 0) // test that targets and collectors are added properly s.SetTargets(initTargets) @@ -60,7 +61,7 @@ func TestAddingAndRemovingTargets(t *testing.T) { assert.Len(t, s.TargetItems(), expectedTargetLen) // prepare second round of targets - tar := MakeNNewTargets(4, 3, 0) + tar := MakeNNewTargetsWithPreAssigningCollectors(4, 3, 0) // test that fewer targets are found - removed s.SetTargets(tar) @@ -125,7 +126,7 @@ func TestNoCollectorReassignment(t *testing.T) { for _, i := range cols { assert.NotNil(t, s.Collectors()[i.Name]) } - initTargets := MakeNNewTargets(6, 3, 0) + initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 3, 0) // test that targets and collectors are added properly s.SetTargets(initTargets) @@ -144,6 +145,56 @@ func TestNoCollectorReassignment(t *testing.T) { } +// Tests that the newly added collector instance does not get assigned any target when the targets remain the same +func TestNoAssignmentToNewCollector(t *testing.T) { + s, _ := New("least-weighted", logger) + + // instantiate only 1 collector + cols := MakeNCollectors(1, 0) + s.SetCollectors(cols) + + expectedColLen := len(cols) + assert.Len(t, s.Collectors(), expectedColLen) + + for _, i := range cols { + assert.NotNil(t, s.Collectors()[i.Name]) + } + + initialColsBeforeAddingNewCol := s.Collectors() + initTargets := MakeNNewTargetsWithoutPreAssigningCollectors(6, 0) + + // test that targets and collectors are added properly + s.SetTargets(initTargets) + + // verify + expectedTargetLen := len(initTargets) + targetItems := s.TargetItems() + assert.Len(t, targetItems, expectedTargetLen) + + // add another collector + newColName := fmt.Sprintf("collector-%d", len(cols)) + cols[newColName] = &Collector{ + Name: newColName, + NumTargets: 0, + } + s.SetCollectors(cols) + + // targets shall not change + newTargetItems := s.TargetItems() + assert.Equal(t, targetItems, newTargetItems) + + // initial collectors still should have the same targets + for colName, col := range s.Collectors() { + if colName != newColName { + assert.Equal(t, initialColsBeforeAddingNewCol[colName], col) + } + } + + // new collector should have no targets + newCollector := s.Collectors()[newColName] + assert.Equal(t, newCollector.NumTargets, 0) +} + func TestSmartCollectorReassignment(t *testing.T) { t.Skip("This test is flaky and fails frequently, see issue 1291") s, _ := New("least-weighted", logger) @@ -157,7 +208,7 @@ func TestSmartCollectorReassignment(t *testing.T) { for _, i := range cols { assert.NotNil(t, s.Collectors()[i.Name]) } - initTargets := MakeNNewTargets(6, 0, 0) + initTargets := MakeNNewTargetsWithPreAssigningCollectors(6, 0, 0) // test that targets and collectors are added properly s.SetTargets(initTargets) @@ -202,7 +253,7 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { cols := MakeNCollectors(3, 0) s.SetCollectors(cols) - targets := MakeNNewTargets(27, 3, 0) + targets := MakeNNewTargetsWithPreAssigningCollectors(27, 3, 0) s.SetTargets(targets) // Divisor needed to get 15% @@ -241,7 +292,7 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { assert.InDelta(t, i.NumTargets, count, math.Round(percent)) } // adding targets at 'random' - for _, item := range MakeNNewTargets(13, 3, 100) { + for _, item := range MakeNNewTargetsWithPreAssigningCollectors(13, 3, 100) { targets[item.Hash()] = item } s.SetTargets(targets) diff --git a/cmd/otel-allocator/allocation/strategy_test.go b/cmd/otel-allocator/allocation/strategy_test.go index c12529d8d8..4ccd2cb099 100644 --- a/cmd/otel-allocator/allocation/strategy_test.go +++ b/cmd/otel-allocator/allocation/strategy_test.go @@ -44,7 +44,7 @@ func BenchmarkGetAllTargetsByCollectorAndJob(b *testing.B) { b.Fail() } cols := MakeNCollectors(v.numCollectors, 0) - jobs := MakeNNewTargets(v.numJobs, v.numCollectors, 0) + jobs := MakeNNewTargetsWithPreAssigningCollectors(v.numJobs, v.numCollectors, 0) a.SetCollectors(cols) a.SetTargets(jobs) b.Run(fmt.Sprintf("%s_num_cols_%d_num_jobs_%d", s, v.numCollectors, v.numJobs), func(b *testing.B) { @@ -76,7 +76,7 @@ func Benchmark_Setting(b *testing.B) { for _, v := range table { a, _ := New(s, logger) cols := MakeNCollectors(v.numCollectors, 0) - targets := MakeNNewTargets(v.numTargets, v.numCollectors, 0) + targets := MakeNNewTargetsWithPreAssigningCollectors(v.numTargets, v.numCollectors, 0) b.Run(fmt.Sprintf("%s_num_cols_%d_num_jobs_%d", s, v.numCollectors, v.numTargets), func(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { diff --git a/cmd/otel-allocator/server/bench_test.go b/cmd/otel-allocator/server/bench_test.go index 8fcea90b0e..a9bb7a5528 100644 --- a/cmd/otel-allocator/server/bench_test.go +++ b/cmd/otel-allocator/server/bench_test.go @@ -50,7 +50,7 @@ func BenchmarkServerTargetsHandler(b *testing.B) { for _, v := range table { a, _ := allocation.New(allocatorName, logger) cols := allocation.MakeNCollectors(v.numCollectors, 0) - targets := allocation.MakeNNewTargets(v.numJobs, v.numCollectors, 0) + targets := allocation.MakeNNewTargetsWithPreAssigningCollectors(v.numJobs, v.numCollectors, 0) listenAddr := ":8080" a.SetCollectors(cols) a.SetTargets(targets)