fix: leaf admissioncontrol should not interfere with inner node admis…

…sionControl

somehow test errors unexpected with context canceled, so not really an admissionControl error but would be interesintg to understand, maybe use vegeta lib instead

Signed-off-by: Sandor Szücs <[email protected]>

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
 	}
 

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