diff --git a/filters/shedder/admission.go b/filters/shedder/admission.go index dcd7add75e..280df01993 100644 --- a/filters/shedder/admission.go +++ b/filters/shedder/admission.go @@ -435,11 +435,14 @@ func (ac *admissionControl) Request(ctx filters.FilterContext) { } func (ac *admissionControl) Response(ctx filters.FilterContext) { - // we don't want to count ourselves + // we don't want to count our short cutted responses as errors if ctx.StateBag()[admissionControlKey] == admissionControlValue { return } - if ctx.Response().Header.Get(admissionControlKey) != "" { + + // we don't want to count other shedders in the call path as errors + if ctx.Response().Header.Get(admissionSignalHeaderKey) == admissionSignalHeaderValue { + ac.successCounter.Add(1) return } diff --git a/filters/shedder/admission_test.go b/filters/shedder/admission_test.go index 7a543bfc0f..4ccd1d56c4 100644 --- a/filters/shedder/admission_test.go +++ b/filters/shedder/admission_test.go @@ -2,17 +2,21 @@ package shedder import ( "fmt" + "io" "math/rand" "net/http" "net/http/httptest" "net/url" "sort" + "sync" "testing" "time" "github.com/zalando/skipper/eskip" "github.com/zalando/skipper/filters" "github.com/zalando/skipper/filters/builtin" + "github.com/zalando/skipper/metrics" + "github.com/zalando/skipper/metrics/metricstest" "github.com/zalando/skipper/net" "github.com/zalando/skipper/proxy/proxytest" "github.com/zalando/skipper/routing" @@ -202,6 +206,162 @@ func TestAdmissionControl(t *testing.T) { } } +func TestAdmissionControlChainOnlyBackendErrorPass(t *testing.T) { + // fixed rand to have non flaky tests + rand := rand.New(rand.NewSource(5)) + + for _, ti := range []struct { + msg string + mode string + d time.Duration + windowsize int + minRequests int + successThreshold float64 + maxrejectprobability float64 + exponent float64 + N int // iterations + pBackendErr float64 // [0,1] + pExpectedAdmissionShedding float64 // [0,1] + }{{ + msg: "backend errors create no shedding on outermost proxy", + mode: "active", + d: 20 * time.Millisecond, + windowsize: 5, + minRequests: 10, + successThreshold: 0.9, + maxrejectprobability: 0.95, + exponent: 1.0, + N: 20, + pBackendErr: 1.1, + pExpectedAdmissionShedding: 0.0, + }} { + t.Run(ti.msg, func(t *testing.T) { + dm := metrics.Default + t.Cleanup(func() { metrics.Default = dm }) + m := &metricstest.MockMetrics{} + metrics.Default = m + + var mu sync.Mutex + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + p := rand.Float64() + mu.Unlock() + if p < ti.pBackendErr { + w.WriteHeader(http.StatusInternalServerError) + } else { + w.WriteHeader(http.StatusOK) + } + })) + + spec := NewAdmissionControl(Options{}) + argsLeaf := make([]interface{}, 0, 6) + argsLeaf = append(argsLeaf, "testmetric-leaf", ti.mode, ti.d.String(), ti.windowsize, ti.minRequests, ti.successThreshold, ti.maxrejectprobability, ti.exponent) + args := make([]interface{}, 0, 6) + args = append(args, "testmetric", ti.mode, ti.d.String(), ti.windowsize, ti.minRequests, ti.successThreshold, ti.maxrejectprobability, ti.exponent) + _, err := spec.CreateFilter(argsLeaf) + if err != nil { + t.Logf("argsLeaf: %+v", argsLeaf...) + t.Fatalf("error creating filter: %v", err) + return + } + _, err = spec.CreateFilter(args) + if err != nil { + t.Logf("args: %+v", args...) + t.Fatalf("error creating filter: %v", err) + return + } + + fr := make(filters.Registry) + fr.Register(spec) + rLeaf := &eskip.Route{Filters: []*eskip.Filter{{Name: spec.Name(), Args: argsLeaf}}, Backend: backend.URL} + + proxyLeaf := proxytest.New(fr, rLeaf) + + r := &eskip.Route{Filters: []*eskip.Filter{{Name: spec.Name(), Args: args}}, Backend: proxyLeaf.URL} + proxy := proxytest.New(fr, r) + + reqURL, err := url.Parse(proxy.URL) + if err != nil { + t.Fatalf("Failed to parse url %s: %v", proxy.URL, err) + } + + rt := net.NewTransport(net.Options{ + Timeout: time.Second, + ResponseHeaderTimeout: time.Second, + }) + + req, err := http.NewRequest("GET", reqURL.String(), nil) + if err != nil { + t.Error(err) + return + } + + N := 1000 + // sem := semaphore.NewWeighted(10) + + var wg sync.WaitGroup + wg.Add(N) + for i := 0; i < N; i++ { + // for { + // if !sem.TryAcquire(1) { + // time.Sleep(10 * time.Microsecond) + // continue + // } + // break + // } + + go func(i int) { + defer wg.Done() + //defer sem.Release(1) + rsp, err := rt.RoundTrip(req) + if err != nil { + t.Logf("roundtrip %d: %v", i, err) + } else { + io.Copy(io.Discard, rsp.Body) + rsp.Body.Close() + } + }(i) + } + wg.Wait() + + var total, totalLeaf, rejects, rejectsLeaf int64 + m.WithCounters(func(counters map[string]int64) { + t.Logf("counters: %+v", counters) + var ok bool + rejects, ok = counters["shedder.admission_control.reject.testmetric"] + if ok { + t.Errorf("Should have no rejects, but have: %d", rejects) + } + + total, ok = counters["shedder.admission_control.total.testmetric"] + if !ok { + t.Error("Failed to find total shedder data") + } + + rejectsLeaf, ok = counters["shedder.admission_control.reject.testmetric-leaf"] + if !ok { + t.Error("Failed to find rejectsLeaf shedder data") + } + + totalLeaf, ok = counters["shedder.admission_control.total.testmetric-leaf"] + if !ok { + t.Error("Failed to find totalLeaf shedder data") + } + }) + + t.Logf("N: %d", N) + t.Logf("totalLeaf: %d, rejectLeaf: %d", totalLeaf, rejectsLeaf) + t.Logf("total: %d, reject: %d", total, rejects) + + epsilon := 0.05 + maxExpectedFails := int64(epsilon * float64(N)) // maybe 5% instead of 0.1% + if rejects > maxExpectedFails { + t.Fatalf("Failed to get less rejects as wanted: %d > %d", rejects, maxExpectedFails) + } + }) + } +} + func TestAdmissionControlCleanupModes(t *testing.T) { for _, ti := range []struct { msg string