Skip to content

Commit c2622f0

Browse files
FedosinFedosin
authored
Merge pull request #19 from Fedosin/panic-less
Return errors instead of start to panic
2 parents a1f4954 + 543538a commit c2622f0

File tree

6 files changed

+155
-37
lines changed

6 files changed

+155
-37
lines changed

‎examples/main.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,18 @@ func main() {
9292
metricTransmitter := transmitter.NewLogTransmitter(nil)
9393

9494
// Create metric windows for stable and panic averages
95-
stableWindow := metrics.NewTimeWindow(cfg.StableWindow, time.Second)
96-
panicWindow := metrics.NewTimeWindow(
95+
stableWindow, err := metrics.NewTimeWindow(cfg.StableWindow, time.Second)
96+
if err != nil {
97+
log.Fatalf("Failed to create new stable time window: %v", err)
98+
}
99+
100+
panicWindow, err := metrics.NewTimeWindow(
97101
max(time.Second, time.Duration(float64(cfg.StableWindow)*cfg.PanicWindowPercentage/100)),
98102
time.Second,
99103
)
104+
if err != nil {
105+
log.Fatalf("Failed to create new panic time window: %v", err)
106+
}
100107

101108
// Create a mock metric collector
102109
collector := &MockMetricCollector{baseLoad: 80}

‎manager/scaler.go‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,23 @@ func NewScaler(
7171
var stableAgg, panicAgg timeWindowInterface
7272
switch algoType {
7373
case "linear":
74-
stableAgg = metrics.NewTimeWindow(cfg.StableWindow, granularity)
75-
panicAgg = metrics.NewTimeWindow(panicWindow, granularity)
74+
stableAgg, err = metrics.NewTimeWindow(cfg.StableWindow, granularity)
75+
if err != nil {
76+
return nil, fmt.Errorf("failed to create stable aggregator: %w", err)
77+
}
78+
panicAgg, err = metrics.NewTimeWindow(panicWindow, granularity)
79+
if err != nil {
80+
return nil, fmt.Errorf("failed to create panic aggregator: %w", err)
81+
}
7682
case "weighted":
77-
stableAgg = metrics.NewWeightedTimeWindow(cfg.StableWindow, granularity)
78-
panicAgg = metrics.NewWeightedTimeWindow(panicWindow, granularity)
83+
stableAgg, err = metrics.NewWeightedTimeWindow(cfg.StableWindow, granularity)
84+
if err != nil {
85+
return nil, fmt.Errorf("failed to create stable aggregator: %w", err)
86+
}
87+
panicAgg, err = metrics.NewWeightedTimeWindow(panicWindow, granularity)
88+
if err != nil {
89+
return nil, fmt.Errorf("failed to create panic aggregator: %w", err)
90+
}
7991
default:
8092
return nil, fmt.Errorf("unknown algorithm type: %s (expected 'linear' or 'weighted')", algoType)
8193
}
@@ -104,13 +116,27 @@ func (s *Scaler) ChangeAggregationAlgorithm(algoType string) error {
104116

105117
granularity := time.Second
106118

119+
var err error
120+
107121
switch algoType {
108122
case "linear":
109-
s.stableAggregator = metrics.NewTimeWindow(cfg.StableWindow, granularity)
110-
s.panicAggregator = metrics.NewTimeWindow(panicWindow, granularity)
123+
s.stableAggregator, err = metrics.NewTimeWindow(cfg.StableWindow, granularity)
124+
if err != nil {
125+
return fmt.Errorf("failed to create stable aggregator: %w", err)
126+
}
127+
s.panicAggregator, err = metrics.NewTimeWindow(panicWindow, granularity)
128+
if err != nil {
129+
return fmt.Errorf("failed to create panic aggregator: %w", err)
130+
}
111131
case "weighted":
112-
s.stableAggregator = metrics.NewWeightedTimeWindow(cfg.StableWindow, granularity)
113-
s.panicAggregator = metrics.NewWeightedTimeWindow(panicWindow, granularity)
132+
s.stableAggregator, err = metrics.NewWeightedTimeWindow(cfg.StableWindow, granularity)
133+
if err != nil {
134+
return fmt.Errorf("failed to create stable aggregator: %w", err)
135+
}
136+
s.panicAggregator,err = metrics.NewWeightedTimeWindow(panicWindow, granularity)
137+
if err != nil {
138+
return fmt.Errorf("failed to create panic aggregator: %w", err)
139+
}
114140
default:
115141
return fmt.Errorf("unknown algorithm type: %s (expected 'linear' or 'weighted')", algoType)
116142
}

‎metrics/time_window.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ func (t *TimeWindow) String() string {
7171

7272
// NewTimeWindow generates a new TimeWindow with the given
7373
// granularity.
74-
func NewTimeWindow(window, granularity time.Duration) *TimeWindow {
74+
func NewTimeWindow(window, granularity time.Duration) (*TimeWindow, error) {
7575
if granularity <= 0 {
76-
panic(fmt.Sprintf("granularity must be positive, got %v", granularity))
76+
return nil, fmt.Errorf("granularity must be positive, got %v", granularity)
7777
}
7878
if window < granularity {
79-
panic(fmt.Sprintf("window must be >= granularity, got window=%v, granularity=%v", window, granularity))
79+
return nil, fmt.Errorf("window must be >= granularity, got window=%v, granularity=%v", window, granularity)
8080
}
8181

8282
// Number of buckets is `window` divided by `granularity`, rounded up.
@@ -86,7 +86,7 @@ func NewTimeWindow(window, granularity time.Duration) *TimeWindow {
8686
buckets: make([]float64, int(nb)),
8787
granularity: granularity,
8888
window: window,
89-
}
89+
}, nil
9090
}
9191

9292
// IsEmpty returns true if no data has been recorded for the `window` period.

‎metrics/time_window_test.go‎

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ func TestTimeWindowSimple(t *testing.T) {
103103
for _, tt := range tests {
104104
t.Run(tt.name, func(t *testing.T) {
105105
// New implementation test.
106-
buckets := NewTimeWindow(2*time.Minute, tt.granularity)
106+
buckets, err := NewTimeWindow(2*time.Minute, tt.granularity)
107+
if err != nil {
108+
t.Fatalf("NewTimeWindow failed: %v", err)
109+
}
107110
if !buckets.IsEmpty(trunc1) {
108111
t.Error("Unexpected non empty result")
109112
}
@@ -130,7 +133,10 @@ func TestTimeWindowSimple(t *testing.T) {
130133

131134
func TestTimeWindowManyReps(t *testing.T) {
132135
trunc1 := time.Now().Truncate(granularity)
133-
buckets := NewTimeWindow(time.Minute, granularity)
136+
buckets, err := NewTimeWindow(time.Minute, granularity)
137+
if err != nil {
138+
t.Fatalf("NewTimeWindow failed: %v", err)
139+
}
134140
for p := range 5 {
135141
trunc1 = trunc1.Add(granularity)
136142
for t := range 5 {
@@ -164,7 +170,10 @@ func TestTimeWindowManyReps(t *testing.T) {
164170
func TestTimeWindowManyRepsWithNonMonotonicalOrder(t *testing.T) {
165171
start := time.Now().Truncate(granularity)
166172
end := start
167-
buckets := NewTimeWindow(time.Minute, granularity)
173+
buckets, err := NewTimeWindow(time.Minute, granularity)
174+
if err != nil {
175+
t.Fatalf("NewTimeWindow failed: %v", err)
176+
}
168177

169178
d := []int{0, 3, 2, 1, 4}
170179
for p := range 5 {
@@ -200,7 +209,10 @@ func TestTimeWindowManyRepsWithNonMonotonicalOrder(t *testing.T) {
200209

201210
func TestTimeWindowWindowAverage(t *testing.T) {
202211
now := time.Now()
203-
buckets := NewTimeWindow(5*time.Second, granularity)
212+
buckets, err := NewTimeWindow(5*time.Second, granularity)
213+
if err != nil {
214+
t.Fatalf("NewTimeWindow failed: %v", err)
215+
}
204216

205217
// This verifies that we properly use firstWrite. Without that we'd get 0.2.
206218
buckets.Record(now, 1)
@@ -285,7 +297,10 @@ func TestTimeWindowWindowAverage(t *testing.T) {
285297
func TestTimeWindowAverageWithLargeGap(t *testing.T) {
286298
now := time.Now()
287299
// Create a window with 30 buckets (60s window / 2s granularity)
288-
buckets := NewTimeWindow(60*time.Second, 2*time.Second)
300+
buckets, err := NewTimeWindow(60*time.Second, 2*time.Second)
301+
if err != nil {
302+
t.Fatalf("NewTimeWindow failed: %v", err)
303+
}
289304

290305
// Record some initial data
291306
for i := range 10 {
@@ -320,7 +335,10 @@ func TestTimeWindowAverageWithLargeGap(t *testing.T) {
320335
// TestTimeWindowAverageNegativeValues tests that the window can handle and average negative values correctly.
321336
func TestTimeWindowAverageNegativeValues(t *testing.T) {
322337
now := time.Now()
323-
buckets := NewTimeWindow(5*time.Second, granularity)
338+
buckets, err := NewTimeWindow(5*time.Second, granularity)
339+
if err != nil {
340+
t.Fatalf("NewTimeWindow failed: %v", err)
341+
}
324342

325343
// Record negative values
326344
buckets.Record(now, -10)
@@ -346,7 +364,10 @@ func TestTimeWindowAverageNegativeValues(t *testing.T) {
346364
func TestTimeWindowAverageBoundaryConditions(t *testing.T) {
347365
now := time.Now()
348366
// Small window to make wraparound easier to test
349-
buckets := NewTimeWindow(10*time.Second, 2*time.Second) // 5 buckets
367+
buckets, err := NewTimeWindow(10*time.Second, 2*time.Second) // 5 buckets
368+
if err != nil {
369+
t.Fatalf("NewTimeWindow failed: %v", err)
370+
}
350371

351372
// Fill all buckets
352373
for i := range 5 {
@@ -380,7 +401,10 @@ func TestTimeWindowAverageBoundaryConditions(t *testing.T) {
380401

381402
func TestDescendingRecord(t *testing.T) {
382403
now := time.Now()
383-
buckets := NewTimeWindow(5*time.Second, 1*time.Second)
404+
buckets, err := NewTimeWindow(5*time.Second, 1*time.Second)
405+
if err != nil {
406+
t.Fatalf("NewTimeWindow failed: %v", err)
407+
}
384408

385409
for i := 8 * time.Second; i >= 0*time.Second; i -= time.Second {
386410
buckets.Record(now.Add(i), 5)
@@ -395,7 +419,10 @@ func TestDescendingRecord(t *testing.T) {
395419

396420
func TestTimeWindowHoles(t *testing.T) {
397421
now := time.Now()
398-
buckets := NewTimeWindow(5*time.Second, granularity)
422+
buckets, err := NewTimeWindow(5*time.Second, granularity)
423+
if err != nil {
424+
t.Fatalf("NewTimeWindow failed: %v", err)
425+
}
399426

400427
for i := range 5 {
401428
buckets.Record(now.Add(time.Duration(i)*time.Second), float64(i+1))
@@ -431,7 +458,10 @@ func TestTimeWindowHoles(t *testing.T) {
431458

432459
func TestTimeWindowResizeWindow(t *testing.T) {
433460
startTime := time.Now()
434-
buckets := NewTimeWindow(5*time.Second, granularity)
461+
buckets, err := NewTimeWindow(5*time.Second, granularity)
462+
if err != nil {
463+
t.Fatalf("NewTimeWindow failed: %v", err)
464+
}
435465

436466
// Fill the whole bucketing list with rollover.
437467
buckets.Record(startTime, 1)
@@ -523,7 +553,10 @@ func TestTimeWindowWindowUpdate3sGranularity(t *testing.T) {
523553
trunc1 := time.Now().Truncate(granularity)
524554

525555
// So two buckets here (ceil(5/3)=ceil(1.6(6))=2).
526-
buckets := NewTimeWindow(5*time.Second, granularity)
556+
buckets, err := NewTimeWindow(5*time.Second, granularity)
557+
if err != nil {
558+
t.Fatalf("NewTimeWindow failed: %v", err)
559+
}
527560
if got, want := len(buckets.buckets), 2; got != want {
528561
t.Fatalf("Initial bucket count = %d, want: %d", got, want)
529562
}
@@ -602,7 +635,10 @@ func TestTimeWindowWindowUpdate3sGranularity(t *testing.T) {
602635

603636
func TestTimeWindowWindowUpdateNoOp(t *testing.T) {
604637
startTime := time.Now().Add(-time.Minute)
605-
buckets := NewTimeWindow(5*time.Second, granularity)
638+
buckets, err := NewTimeWindow(5*time.Second, granularity)
639+
if err != nil {
640+
t.Fatalf("NewTimeWindow failed: %v", err)
641+
}
606642
buckets.Record(startTime, 19.82)
607643
if got, want := buckets.firstWrite, buckets.lastWrite; !got.Equal(want) {
608644
t.Errorf("FirstWrite = %v, want: %v", got, want)
@@ -619,8 +655,10 @@ func BenchmarkWindowAverage(b *testing.B) {
619655
for _, wl := range []int{30, 60, 120, 240, 600} {
620656
b.Run(fmt.Sprintf("%v-win-len", wl), func(b *testing.B) {
621657
tn := time.Now().Truncate(time.Second) // To simplify everything.
622-
buckets := NewTimeWindow(time.Duration(wl)*time.Second,
623-
time.Second /*granularity*/)
658+
buckets, err := NewTimeWindow(time.Duration(wl)*time.Second, time.Second)
659+
if err != nil {
660+
b.Fatalf("NewTimeWindow failed: %v", err)
661+
}
624662
// Populate with some random data.
625663
for i := range wl {
626664
buckets.Record(tn.Add(time.Duration(i)*time.Second), rand.Float64()*100)
@@ -671,7 +709,10 @@ func (t *TimeWindow) forEachBucket(now time.Time, acc func(time time.Time, bucke
671709

672710
func TestTimeWindowAverageWithZeros(t *testing.T) {
673711
now := time.Now()
674-
buckets := NewTimeWindow(10*time.Second, granularity)
712+
buckets, err := NewTimeWindow(10*time.Second, granularity)
713+
if err != nil {
714+
t.Fatalf("NewTimeWindow failed: %v", err)
715+
}
675716

676717
// Fill the window with zeros
677718
for i := range 10 {
@@ -696,7 +737,10 @@ func TestTimeWindowAverageWithZeros(t *testing.T) {
696737

697738
func TestTimeWindowAverageWithPositiveValuesThenZeros(t *testing.T) {
698739
now := time.Now()
699-
buckets := NewTimeWindow(10*time.Second, granularity)
740+
buckets, err := NewTimeWindow(10*time.Second, granularity)
741+
if err != nil {
742+
t.Fatalf("NewTimeWindow failed: %v", err)
743+
}
700744

701745
// Fill the window with random positive values
702746
for i := range 10 {
@@ -725,3 +769,15 @@ func TestTimeWindowAverageWithPositiveValuesThenZeros(t *testing.T) {
725769
t.Errorf("WindowAverage of zeros (with gap) = %v, want: %v", got, want)
726770
}
727771
}
772+
773+
func TestNegativeWindowAndGranularity(t *testing.T) {
774+
_, err := NewTimeWindow(10*time.Second, -1*time.Second)
775+
if err == nil {
776+
t.Errorf("NewTimeWindow should fail with negative granularity")
777+
}
778+
779+
_, err = NewTimeWindow(-10*time.Second, 1*time.Second)
780+
if err == nil {
781+
t.Errorf("NewTimeWindow should fail with negative window")
782+
}
783+
}

‎metrics/weighted_time_window.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,19 @@ type WeightedTimeWindow struct {
3535

3636
// NewWeightedTimeWindow generates a new WeightedTimeWindow with the given
3737
// granularity.
38-
func NewWeightedTimeWindow(window, granularity time.Duration) *WeightedTimeWindow {
38+
func NewWeightedTimeWindow(window, granularity time.Duration) (*WeightedTimeWindow, error) {
39+
tw, err := NewTimeWindow(window, granularity)
40+
if err != nil {
41+
return nil, err
42+
}
43+
3944
// Number of buckets is `window` divided by `granularity`, rounded up.
4045
// e.g. 60s / 2s = 30.
4146
nb := math.Ceil(float64(window) / float64(granularity))
4247
return &WeightedTimeWindow{
43-
TimeWindow: NewTimeWindow(window, granularity),
48+
TimeWindow: tw,
4449
smoothingCoeff: computeSmoothingCoeff(nb),
45-
}
50+
}, nil
4651
}
4752

4853
// WindowAverage returns the exponential weighted average. This means

‎metrics/weighted_time_window_test.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@ import (
2525

2626
func TestTimeWindowWeightedAverage(t *testing.T) {
2727
now := time.Now()
28-
buckets := NewWeightedTimeWindow(5*time.Second, granularity)
28+
buckets, err := NewWeightedTimeWindow(5*time.Second, granularity)
29+
if err != nil {
30+
t.Fatalf("NewWeightedTimeWindow failed: %v", err)
31+
}
2932

3033
buckets.Record(now, 2)
3134
expectedAvg := 2 * buckets.smoothingCoeff // 2*dm = dm.
@@ -68,7 +71,10 @@ func TestTimeWindowWeightedAverage(t *testing.T) {
6871

6972
func TestWeightedTimeWindowResizeWindow(t *testing.T) {
7073
startTime := time.Now()
71-
buckets := NewWeightedTimeWindow(5*time.Second, granularity)
74+
buckets, err := NewWeightedTimeWindow(5*time.Second, granularity)
75+
if err != nil {
76+
t.Fatalf("NewWeightedTimeWindow failed: %v", err)
77+
}
7278

7379
if got, want := buckets.smoothingCoeff, computeSmoothingCoeff(5); math.Abs(got-want) > weightPrecision {
7480
t.Errorf("DecayMultipler = %v, want: %v", got, want)
@@ -113,7 +119,10 @@ func TestWeightedTimeWindowResizeWindow(t *testing.T) {
113119

114120
func TestWeightedTimeWindowAverageWithZeros(t *testing.T) {
115121
now := time.Now()
116-
buckets := NewWeightedTimeWindow(10*time.Second, granularity)
122+
buckets, err := NewWeightedTimeWindow(10*time.Second, granularity)
123+
if err != nil {
124+
t.Fatalf("NewWeightedTimeWindow failed: %v", err)
125+
}
117126

118127
// Fill the window with zeros
119128
for i := range 10 {
@@ -138,7 +147,10 @@ func TestWeightedTimeWindowAverageWithZeros(t *testing.T) {
138147

139148
func TestWeightedTimeWindowAverageWithPositiveValuesThenZeros(t *testing.T) {
140149
now := time.Now()
141-
buckets := NewWeightedTimeWindow(10*time.Second, granularity)
150+
buckets, err := NewWeightedTimeWindow(10*time.Second, granularity)
151+
if err != nil {
152+
t.Fatalf("NewWeightedTimeWindow failed: %v", err)
153+
}
142154

143155
// Fill the window with random positive values
144156
for i := range 10 {
@@ -167,3 +179,15 @@ func TestWeightedTimeWindowAverageWithPositiveValuesThenZeros(t *testing.T) {
167179
t.Errorf("WindowAverage of zeros (with gap) = %v, want: %v", got, want)
168180
}
169181
}
182+
183+
func TestWeightedTimeWindowNegativeWindowAndGranularity(t *testing.T) {
184+
_, err := NewWeightedTimeWindow(10*time.Second, -1*time.Second)
185+
if err == nil {
186+
t.Errorf("NewWeightedTimeWindow should fail with negative granularity")
187+
}
188+
189+
_, err = NewWeightedTimeWindow(-10*time.Second, 1*time.Second)
190+
if err == nil {
191+
t.Errorf("NewWeightedTimeWindow should fail with negative window")
192+
}
193+
}

0 commit comments

Comments
 (0)