From f0320211214fe8b65416dae85bc6f84586fbdff8 Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Thu, 1 Feb 2024 18:28:53 +0900 Subject: [PATCH 1/6] add grace --- alert.go | 2 +- alert_test.go | 14 ++++++++- throttler.go | 34 +++++++++++++++++++-- throttler_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 118 insertions(+), 9 deletions(-) diff --git a/alert.go b/alert.go index 6bfd93c..2c06471 100644 --- a/alert.go +++ b/alert.go @@ -95,7 +95,7 @@ func (a *Alert) shouldAlert() bool { return false } t := NewThrottler() - return !t.IsThrottled(a.Error) + return !t.IsThrottledOrGraced(a.Error) } func (a *Alert) isDoNotAlert() bool { diff --git a/alert_test.go b/alert_test.go index ea39e76..8e1f2b5 100644 --- a/alert_test.go +++ b/alert_test.go @@ -159,6 +159,14 @@ func TestAlert_shouldAlert(t *testing.T) { }, want: true, }, + {name: "shouldAlert_graced_false", + fields: fields{ + Error: errors.New("alert this"), + DoNotAlertErrors: []error{ + errors.New("do not alert"), errors.New("if this error then don't alert")}, + }, + want: false, + }, {name: "shouldAlert_true_disable_throttling", fields: fields{ Error: errors.New("do not alert"), @@ -173,6 +181,9 @@ func TestAlert_shouldAlert(t *testing.T) { if tt.name == "shouldAlert_true_disable_throttling" { os.Setenv("THROTTLE_ENABLED", "false") } + if tt.name == "shouldAlert_graced_false" { + os.Setenv("GRACE_DURATION", "20") + } a := &Alert{ Error: tt.fields.Error, DoNotAlertErrors: tt.fields.DoNotAlertErrors, @@ -180,7 +191,8 @@ func TestAlert_shouldAlert(t *testing.T) { if err := a.RemoveCurrentThrotting(); err != nil { t.Errorf("Alert.Notify() error = %+v", err) } - if got := a.shouldAlert(); got != tt.want { + got := a.shouldAlert() + if got != tt.want { t.Errorf("Alert.shouldAlert() = %v, want %v", got, tt.want) } }) diff --git a/throttler.go b/throttler.go index 1b34bc5..4f6bcba 100644 --- a/throttler.go +++ b/throttler.go @@ -13,6 +13,7 @@ import ( type Throttler struct { CacheOpt string ThrottleDuration int + GraceDuration int } // ErrorOccurrence store error time and error @@ -27,6 +28,7 @@ func NewThrottler() Throttler { t := Throttler{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 5, // default 5mn + GraceDuration: 0, // default 0sc } if len(os.Getenv("THROTTLE_DURATION")) != 0 { duration, err := strconv.Atoi(os.Getenv("THROTTLE_DURATION")) @@ -35,6 +37,13 @@ func NewThrottler() Throttler { } t.ThrottleDuration = duration } + if len(os.Getenv("GRACE_DURATION")) != 0 { + grace, err := strconv.Atoi(os.Getenv("GRACE_DURATION")) + if err != nil { + return t + } + t.GraceDuration = grace + } if len(os.Getenv("THROTTLE_DISKCACHE_DIR")) != 0 { t.CacheOpt = os.Getenv("THROTTLE_DISKCACHE_DIR") @@ -44,13 +53,24 @@ func NewThrottler() Throttler { } // IsThrottled checks if the error has been throttled. If not, throttle it -func (t *Throttler) IsThrottled(ocError error) bool { +func (t *Throttler) IsThrottledOrGraced(ocError error) bool { dc, err := t.getDiskCache() if err != nil { return false } - cachedTime, throttled := dc.Get(ocError.Error()) + cachedDetectionTime, hasCachedDetectionTime := dc.Get(fmt.Sprintf("%v_detectionTime", ocError.Error())) + if !hasCachedDetectionTime { + now := time.Now().Format(time.RFC3339) + cachedDetectionTime = []byte(now) + dc.Set(fmt.Sprintf("%v_detectionTime", ocError.Error()), cachedDetectionTime) + } + if !isOverGraceDuration(string(cachedDetectionTime), t.GraceDuration) { + // grace duration is not over yet, do nothing + return true + } + + cachedTime, throttled := dc.Get(ocError.Error()) if throttled && !isOverThrottleDuration(string(cachedTime), t.ThrottleDuration) { // already throttled and not over throttling duration, do nothing return true @@ -63,6 +83,16 @@ func (t *Throttler) IsThrottled(ocError error) bool { return false } +func isOverGraceDuration(cachedTime string, graceDuration int) bool { + detectionTime, err := time.Parse(time.RFC3339, string(cachedTime)) + if err != nil { + return false + } + now := time.Now() + diff := int(now.Sub(detectionTime).Seconds()) + return diff >= graceDuration +} + func isOverThrottleDuration(cachedTime string, throttleDuration int) bool { throttledTime, err := time.Parse(time.RFC3339, string(cachedTime)) if err != nil { diff --git a/throttler_test.go b/throttler_test.go index 78a88c9..491a7a0 100644 --- a/throttler_test.go +++ b/throttler_test.go @@ -21,6 +21,7 @@ func TestNewThrottler(t *testing.T) { want: Throttler{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 5, + GraceDuration: 0, }, }, { @@ -28,6 +29,7 @@ func TestNewThrottler(t *testing.T) { want: Throttler{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 7, + GraceDuration: 5, }, }, { @@ -35,28 +37,33 @@ func TestNewThrottler(t *testing.T) { want: Throttler{ CacheOpt: "new_cache_dir", ThrottleDuration: 8, + GraceDuration: 0, }, }, } for _, tt := range tests { if tt.name == "change duration" { os.Setenv("THROTTLE_DURATION", "7") + os.Setenv("GRACE_DURATION", "5") } else if tt.name == "change both" { os.Setenv("THROTTLE_DURATION", "8") + os.Setenv("GRACE_DURATION", "0") os.Setenv("THROTTLE_DISKCACHE_DIR", "new_cache_dir") } t.Run(tt.name, func(t *testing.T) { - if got := NewThrottler(); !reflect.DeepEqual(got, tt.want) { + got := NewThrottler() + if !reflect.DeepEqual(got, tt.want) { t.Errorf("NewThrottler() = %v, want %v", got, tt.want) } }) } } -func TestThrottler_IsThrottled(t *testing.T) { +func TestThrottler_IsThrottledOrGraced(t *testing.T) { type fields struct { CacheOpt string ThrottleDuration int + GraceDuration int } type args struct { ocError error @@ -72,6 +79,7 @@ func TestThrottler_IsThrottled(t *testing.T) { fields: fields{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 5, + GraceDuration: 0, }, args: args{ ocError: errors.New("test_throttling"), @@ -83,6 +91,19 @@ func TestThrottler_IsThrottled(t *testing.T) { fields: fields{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 5, + GraceDuration: 0, + }, + args: args{ + ocError: errors.New("test_throttling"), + }, + want: true, + }, + { + name: "graced_true", + fields: fields{ + CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), + ThrottleDuration: 5, + GraceDuration: 25, }, args: args{ ocError: errors.New("test_throttling"), @@ -95,14 +116,14 @@ func TestThrottler_IsThrottled(t *testing.T) { th := &Throttler{ CacheOpt: tt.fields.CacheOpt, ThrottleDuration: tt.fields.ThrottleDuration, + GraceDuration: tt.fields.GraceDuration, } if tt.name == "throttled_true" { if err := th.ThrottleError(tt.args.ocError); err != nil { t.Errorf("testing failed : %+v", err) } - } - if got := th.IsThrottled(tt.args.ocError); got != tt.want { + if got := th.IsThrottledOrGraced(tt.args.ocError); got != tt.want { t.Errorf("Throttler.IsThrottled() = %v, want %v", got, tt.want) } err := th.CleanThrottlingCache() @@ -118,6 +139,7 @@ func TestThrottler_ThrottleError(t *testing.T) { type fields struct { CacheOpt string ThrottleDuration int + GraceDuration int } type args struct { errObj error @@ -133,6 +155,7 @@ func TestThrottler_ThrottleError(t *testing.T) { fields: fields{ CacheOpt: fmt.Sprintf("/tmp/cache/%v_throttler_disk_cache", os.Getenv("APP_NAME")), ThrottleDuration: 5, + GraceDuration: 0, }, args: args{ errObj: errors.New("test_throttling"), @@ -144,6 +167,7 @@ func TestThrottler_ThrottleError(t *testing.T) { fields: fields{ CacheOpt: "/no_permission_dir", ThrottleDuration: 5, + GraceDuration: 0, }, args: args{ errObj: errors.New("test_throttling"), @@ -162,7 +186,7 @@ func TestThrottler_ThrottleError(t *testing.T) { if err := th.ThrottleError(tt.args.errObj); (err != nil) != tt.wantErr { t.Errorf("Throttler.ThrottleError() error = %v, wantErr %v", err, tt.wantErr) } - if tt.name == "default" && !th.IsThrottled(tt.args.errObj) { + if tt.name == "default" && !th.IsThrottledOrGraced(tt.args.errObj) { t.Errorf("Throttler.ThrottleError() error = %v, wantErr %v", errors.New("throttling failed"), tt.wantErr) } if !tt.wantErr { @@ -229,6 +253,7 @@ func Test_isOverThrottleDuration(t *testing.T) { type args struct { cachedTime string throttleDuration int + graceDuration int } tests := []struct { name string @@ -240,6 +265,7 @@ func Test_isOverThrottleDuration(t *testing.T) { args: args{ cachedTime: time.Now().Add(-3 * time.Minute).Format(time.RFC3339), // -3 minutes => pass 2 minutes durations throttleDuration: 2, + graceDuration: 0, }, want: true, }, @@ -248,6 +274,7 @@ func Test_isOverThrottleDuration(t *testing.T) { args: args{ cachedTime: time.Now().Add(1 * time.Minute).Format(time.RFC3339), // 1 minute ahead of current < throtte duration 2 throttleDuration: 2, + graceDuration: 0, }, want: false, }, @@ -260,3 +287,43 @@ func Test_isOverThrottleDuration(t *testing.T) { }) } } + +func Test_isOverGraceDuration(t *testing.T) { + type args struct { + cachedTime string + throttleDuration int + graceDuration int + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "Test_isOverGraceDuration_true", + args: args{ + cachedTime: time.Now().Add(-3 * time.Minute).Format(time.RFC3339), // -3 minutes => pass 2 minutes durations + throttleDuration: 0, + graceDuration: 2, + }, + want: true, + }, + { + name: "Test_isOverGraceDuration_false", + args: args{ + cachedTime: time.Now().Add(1 * time.Minute).Format(time.RFC3339), // 1 minute ahead of current < throtte duration 2 + throttleDuration: 0, + graceDuration: 2, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isOverGraceDuration(tt.args.cachedTime, tt.args.graceDuration); got != tt.want { + t.Errorf("isOverGraceDuration() = %v, want %v", got, tt.want) + } + }) + } + +} From c25eac1265b756743a95e95e0a35d52c5550cf1c Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Fri, 2 Feb 2024 10:31:04 +0900 Subject: [PATCH 2/6] fix the test case --- throttler_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/throttler_test.go b/throttler_test.go index 491a7a0..694d023 100644 --- a/throttler_test.go +++ b/throttler_test.go @@ -49,6 +49,8 @@ func TestNewThrottler(t *testing.T) { os.Setenv("THROTTLE_DURATION", "8") os.Setenv("GRACE_DURATION", "0") os.Setenv("THROTTLE_DISKCACHE_DIR", "new_cache_dir") + } else if tt.name == "default" { + os.Setenv("GRACE_DURATION", "") } t.Run(tt.name, func(t *testing.T) { got := NewThrottler() From 131fe276e31a5be64182701622afb3eb5bc90737 Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Fri, 2 Feb 2024 11:08:03 +0900 Subject: [PATCH 3/6] update grace test --- throttler_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/throttler_test.go b/throttler_test.go index 694d023..74e5aae 100644 --- a/throttler_test.go +++ b/throttler_test.go @@ -304,18 +304,18 @@ func Test_isOverGraceDuration(t *testing.T) { { name: "Test_isOverGraceDuration_true", args: args{ - cachedTime: time.Now().Add(-3 * time.Minute).Format(time.RFC3339), // -3 minutes => pass 2 minutes durations + cachedTime: time.Now().Add(-5 * time.Second).Format(time.RFC3339), // 2 sec after grace duration is over throttleDuration: 0, - graceDuration: 2, + graceDuration: 3, }, want: true, }, { name: "Test_isOverGraceDuration_false", args: args{ - cachedTime: time.Now().Add(1 * time.Minute).Format(time.RFC3339), // 1 minute ahead of current < throtte duration 2 + cachedTime: time.Now().Add(2 * time.Second).Format(time.RFC3339), // still 8 sec left for grace duration throttleDuration: 0, - graceDuration: 2, + graceDuration: 10, }, want: false, }, From 09d568ddac777b0327a34bc9dd886bdc2e6414af Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Tue, 6 Feb 2024 15:27:43 +0900 Subject: [PATCH 4/6] code review fixes --- alert_test.go | 2 +- throttler.go | 24 ++++++++++++++++++------ throttler_test.go | 6 +++--- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/alert_test.go b/alert_test.go index 8e1f2b5..717890d 100644 --- a/alert_test.go +++ b/alert_test.go @@ -182,7 +182,7 @@ func TestAlert_shouldAlert(t *testing.T) { os.Setenv("THROTTLE_ENABLED", "false") } if tt.name == "shouldAlert_graced_false" { - os.Setenv("GRACE_DURATION", "20") + os.Setenv("THROTTLE_GRACE_SECONDS", "20") } a := &Alert{ Error: tt.fields.Error, diff --git a/throttler.go b/throttler.go index 4f6bcba..c48d3d5 100644 --- a/throttler.go +++ b/throttler.go @@ -37,8 +37,8 @@ func NewThrottler() Throttler { } t.ThrottleDuration = duration } - if len(os.Getenv("GRACE_DURATION")) != 0 { - grace, err := strconv.Atoi(os.Getenv("GRACE_DURATION")) + if len(os.Getenv("THROTTLE_GRACE_SECONDS")) != 0 { + grace, err := strconv.Atoi(os.Getenv("THROTTLE_GRACE_SECONDS")) if err != nil { return t } @@ -61,11 +61,9 @@ func (t *Throttler) IsThrottledOrGraced(ocError error) bool { cachedDetectionTime, hasCachedDetectionTime := dc.Get(fmt.Sprintf("%v_detectionTime", ocError.Error())) if !hasCachedDetectionTime { - now := time.Now().Format(time.RFC3339) - cachedDetectionTime = []byte(now) - dc.Set(fmt.Sprintf("%v_detectionTime", ocError.Error()), cachedDetectionTime) + cachedDetectionTime = t.InitGrace(ocError) } - if !isOverGraceDuration(string(cachedDetectionTime), t.GraceDuration) { + if cachedDetectionTime != nil && !isOverGraceDuration(string(cachedDetectionTime), t.GraceDuration) { // grace duration is not over yet, do nothing return true } @@ -75,6 +73,7 @@ func (t *Throttler) IsThrottledOrGraced(ocError error) bool { // already throttled and not over throttling duration, do nothing return true } + // if it has not throttled yet or over throttle duration, throttle it and return false to send notification // Rethrottler will also renew the timestamp in the throttler cache. if err = t.ThrottleError(ocError); err != nil { @@ -109,12 +108,25 @@ func (t *Throttler) ThrottleError(errObj error) error { if err != nil { return err } + now := time.Now().Format(time.RFC3339) err = dc.Set(errObj.Error(), []byte(now)) return err } +// ThrottleError throttle the alert within the limited duration +func (t *Throttler) InitGrace(errObj error) []byte { + dc, err := t.getDiskCache() + if err != nil { + return nil + } + now := time.Now().Format(time.RFC3339) + cachedDetectionTime := []byte(now) + err = dc.Set(fmt.Sprintf("%v_detectionTime", errObj.Error()), cachedDetectionTime) + return cachedDetectionTime +} + // CleanThrottlingCache clean all the diskcache in throttling cache directory func (t *Throttler) CleanThrottlingCache() (err error) { dc, err := t.getDiskCache() diff --git a/throttler_test.go b/throttler_test.go index 74e5aae..4fd9c73 100644 --- a/throttler_test.go +++ b/throttler_test.go @@ -44,13 +44,13 @@ func TestNewThrottler(t *testing.T) { for _, tt := range tests { if tt.name == "change duration" { os.Setenv("THROTTLE_DURATION", "7") - os.Setenv("GRACE_DURATION", "5") + os.Setenv("THROTTLE_GRACE_SECONDS", "5") } else if tt.name == "change both" { os.Setenv("THROTTLE_DURATION", "8") - os.Setenv("GRACE_DURATION", "0") + os.Setenv("THROTTLE_GRACE_SECONDS", "0") os.Setenv("THROTTLE_DISKCACHE_DIR", "new_cache_dir") } else if tt.name == "default" { - os.Setenv("GRACE_DURATION", "") + os.Setenv("THROTTLE_GRACE_SECONDS", "") } t.Run(tt.name, func(t *testing.T) { got := NewThrottler() From 492f4baa78e0afb64ef7586b7898a62cf1c1c5d5 Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Tue, 6 Feb 2024 15:37:48 +0900 Subject: [PATCH 5/6] add error handler --- throttler.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/throttler.go b/throttler.go index c48d3d5..469ebe1 100644 --- a/throttler.go +++ b/throttler.go @@ -124,7 +124,10 @@ func (t *Throttler) InitGrace(errObj error) []byte { now := time.Now().Format(time.RFC3339) cachedDetectionTime := []byte(now) err = dc.Set(fmt.Sprintf("%v_detectionTime", errObj.Error()), cachedDetectionTime) - return cachedDetectionTime + if err == nil { + return cachedDetectionTime + } + return nil } // CleanThrottlingCache clean all the diskcache in throttling cache directory From 1038ab5dbe5425985b41d072f84aa18ad55a3165 Mon Sep 17 00:00:00 2001 From: Marchenko Ihor Date: Wed, 7 Feb 2024 17:43:13 +0900 Subject: [PATCH 6/6] adjust grace reset --- throttler.go | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/throttler.go b/throttler.go index 469ebe1..71c1930 100644 --- a/throttler.go +++ b/throttler.go @@ -58,9 +58,16 @@ func (t *Throttler) IsThrottledOrGraced(ocError error) bool { if err != nil { return false } + cachedThrottleTime, throttled := dc.Get(ocError.Error()) + cachedDetectionTime, graced := dc.Get(fmt.Sprintf("%v_detectionTime", ocError.Error())) - cachedDetectionTime, hasCachedDetectionTime := dc.Get(fmt.Sprintf("%v_detectionTime", ocError.Error())) - if !hasCachedDetectionTime { + throttleIsOver := isOverThrottleDuration(string(cachedThrottleTime), t.ThrottleDuration) + if throttled && !throttleIsOver { + // already throttled and not over throttling duration, do nothing + return true + } + + if !graced || isOverGracePlusThrottleDuration(string(cachedDetectionTime), t.GraceDuration, t.ThrottleDuration) { cachedDetectionTime = t.InitGrace(ocError) } if cachedDetectionTime != nil && !isOverGraceDuration(string(cachedDetectionTime), t.GraceDuration) { @@ -68,12 +75,6 @@ func (t *Throttler) IsThrottledOrGraced(ocError error) bool { return true } - cachedTime, throttled := dc.Get(ocError.Error()) - if throttled && !isOverThrottleDuration(string(cachedTime), t.ThrottleDuration) { - // already throttled and not over throttling duration, do nothing - return true - } - // if it has not throttled yet or over throttle duration, throttle it and return false to send notification // Rethrottler will also renew the timestamp in the throttler cache. if err = t.ThrottleError(ocError); err != nil { @@ -82,6 +83,17 @@ func (t *Throttler) IsThrottledOrGraced(ocError error) bool { return false } +func isOverGracePlusThrottleDuration(cachedTime string, graceDurationInSec int, throttleDurationInMin int) bool { + detectionTime, err := time.Parse(time.RFC3339, string(cachedTime)) + if err != nil { + return false + } + now := time.Now() + diff := int(now.Sub(detectionTime).Seconds()) + overallDurationInSec := graceDurationInSec + throttleDurationInMin*60 + return diff >= overallDurationInSec +} + func isOverGraceDuration(cachedTime string, graceDuration int) bool { detectionTime, err := time.Parse(time.RFC3339, string(cachedTime)) if err != nil { @@ -99,7 +111,7 @@ func isOverThrottleDuration(cachedTime string, throttleDuration int) bool { } now := time.Now() diff := int(now.Sub(throttledTime).Minutes()) - return diff > throttleDuration + return diff >= throttleDuration } // ThrottleError throttle the alert within the limited duration @@ -124,10 +136,11 @@ func (t *Throttler) InitGrace(errObj error) []byte { now := time.Now().Format(time.RFC3339) cachedDetectionTime := []byte(now) err = dc.Set(fmt.Sprintf("%v_detectionTime", errObj.Error()), cachedDetectionTime) - if err == nil { - return cachedDetectionTime + if err != nil { + return nil } - return nil + + return cachedDetectionTime } // CleanThrottlingCache clean all the diskcache in throttling cache directory