From 34c7e6c9977b94591ae6a7b424e114862e07f95b Mon Sep 17 00:00:00 2001 From: Leonid Bugaev Date: Thu, 16 Jan 2025 13:00:36 +0300 Subject: [PATCH] Merging to release-5-lts: [TT-13819] Benchmark updates, session limiter workaround for test goroutine leak (#6826) (#6832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
TT-13819
Summary Local testing, run and collect go test benchmarks across releases
Type Story Story
Status In Dev
Points N/A
Labels SESAP
--- [TT-13819] Benchmark updates, session limiter workaround for test goroutine leak (#6826) ### **PR Type** Enhancement, Tests ___ ### **Description** - Added `DisableRateLimit` and `DisableQuota` flags in multiple test cases. - Improved benchmark scripts and added skipping for specific benchmarks. - Refactored session limiter to encourage reuse of `bucketStore`. - Updated CI benchmark script for better profiling and output. ___ ### **Changes walkthrough** 📝
Relevant files
Tests
7 files
api_definition_test.go
Added DisableRateLimit and DisableQuota flags in API definition tests
 +2/-0     
event_system_test.go
Added benchmark skipping for generic event handlers           
+2/-0     
mw_basic_auth_test.go
Added `DisableRateLimit` and `DisableQuota` flags in basic auth tests
 +2/-1     
mw_jwt_test.go
Added `DisableRateLimit` and `DisableQuota` flags in JWT tests
 +10/-0   
mw_transform_test.go
Refactored transform middleware benchmarks for better structure
 +9/-7     
oauth_manager_test.go
Added benchmark skipping for OAuth token purging                 
+2/-0     
res_handler_header_injector_test.go
Adjusted header injection benchmarks and test cases           
+7/-6     
Enhancement
2 files
session_manager.go
Refactored session limiter to reuse `bucketStore`               
+4/-1     
ci-benchmark.sh
Enhanced CI benchmark script with profiling and output improvements
 +9/-3     
Configuration changes
1 files
CODEOWNERS
Updated ownership for `/bin/` directory                                   
+1/-0     
___ > 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull request to receive relevant information --------- Co-authored-by: Tit Petric [TT-13819]: https://tyktech.atlassian.net/browse/TT-13819?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Tit Petric Co-authored-by: Tit Petric --- .github/CODEOWNERS | 24 +++++++- bin/ci-benchmark.sh | 16 ++++- gateway/api_definition_test.go | 2 + gateway/event_system_test.go | 2 + gateway/mw_basic_auth_test.go | 3 +- gateway/mw_jwt_test.go | 10 +++ gateway/mw_transform_test.go | 67 +++++++++++---------- gateway/mw_version_check_test.go | 2 + gateway/oauth_manager_test.go | 2 + gateway/res_handler_header_injector_test.go | 13 ++-- 10 files changed, 96 insertions(+), 45 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index b95dd6ad159..c90ecd2384c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,2 +1,22 @@ -/ci/ @TykTechnologies/devops -.github/workflows/release.yml @TykTechnologies/devops +# SysE/DevOps ownership of CI release pipeline, repo policy, automations. + +/ci/ @TykTechnologies/devops +.github/workflows/release.yml @TykTechnologies/devops + +# Core API Squad ownership of CI test pipeline and developer tooling. + +/bin/ @TykTechnologies/core-api-squad +.github/workflows/ci-tests.yml @TykTechnologies/core-api-squad +.github/workflows/release-tests.yml @TykTechnologies/core-api-squad +/.taskfiles/ @TykTechnologies/core-api-squad +/Dockerfile @TykTechnologies/core-api-squad +/docker/ @TykTechnologies/core-api-squad +/Makefile @TykTechnologies/core-api-squad +/Taskfile.yml @TykTechnologies/core-api-squad +/docker-compose.yml @TykTechnologies/core-api-squad + +# Core API Squad ownership of plugin compiler and related tests. + +/smoke-tests/ @TykTechnologies/core-api-squad +/ci/tests/ @TykTechnologies/core-api-squad +/ci/images/plugin-compiler/ @TykTechnologies/core-api-squad diff --git a/bin/ci-benchmark.sh b/bin/ci-benchmark.sh index 93ad8a4d2da..9333213424c 100755 --- a/bin/ci-benchmark.sh +++ b/bin/ci-benchmark.sh @@ -1,7 +1,17 @@ #!/bin/bash - set -e -benchRegex=${1:-.} +# Build gw test binary +go test -o gateway.test -c ./gateway + +BENCHMARKS=$(./gateway.test -test.list=Bench.+) -TYK_LOGLEVEL= go test -run=NONE -bench=$benchRegex || fatal "go test -run=NONE -bench=$benchRegex" +for benchmark in $BENCHMARKS; do + echo $benchmark + benchRegex="^${benchmark}$" + ./gateway.test -test.run=^$ -test.bench=$benchRegex -test.count=1 \ + -test.benchtime 30s -test.benchmem \ + -test.cpuprofile=coverage/$benchmark-cpu.out \ + -test.memprofile=coverage/$benchmark-mem.out \ + -test.trace=coverage/$benchmark-trace.out +done diff --git a/gateway/api_definition_test.go b/gateway/api_definition_test.go index 045a02db18b..833c281c4e3 100644 --- a/gateway/api_definition_test.go +++ b/gateway/api_definition_test.go @@ -1063,6 +1063,8 @@ func (ts *Test) testPrepareDefaultVersion() string { spec.Proxy.ListenPath = "/" spec.UseKeylessAccess = false + spec.DisableRateLimit = true + spec.DisableQuota = true }) return CreateSession(ts.Gw, func(s *user.SessionState) { diff --git a/gateway/event_system_test.go b/gateway/event_system_test.go index 43f850f1ab8..ec603547d98 100644 --- a/gateway/event_system_test.go +++ b/gateway/event_system_test.go @@ -173,6 +173,8 @@ func TestInitGenericEventHandlers(t *testing.T) { } func BenchmarkInitGenericEventHandlers(b *testing.B) { + b.Skip() + ts := StartTest(nil) defer ts.Close() diff --git a/gateway/mw_basic_auth_test.go b/gateway/mw_basic_auth_test.go index f4de92785a8..4c032a6a1db 100644 --- a/gateway/mw_basic_auth_test.go +++ b/gateway/mw_basic_auth_test.go @@ -21,7 +21,6 @@ func genAuthHeader(username, password string) string { } func (ts *Test) testPrepareBasicAuth(cacheDisabled bool) *user.SessionState { - session := CreateStandardSession() session.BasicAuthData.Password = "password" session.AccessRights = map[string]user.AccessDefinition{"test": {APIID: "test", Versions: []string{"v1"}}} @@ -33,6 +32,8 @@ func (ts *Test) testPrepareBasicAuth(cacheDisabled bool) *user.SessionState { spec.UseKeylessAccess = false spec.Proxy.ListenPath = "/" spec.OrgID = "default" + spec.DisableRateLimit = true + spec.DisableQuota = true }) return session diff --git a/gateway/mw_jwt_test.go b/gateway/mw_jwt_test.go index 5a6e4b1e741..21dbe598a42 100644 --- a/gateway/mw_jwt_test.go +++ b/gateway/mw_jwt_test.go @@ -141,6 +141,8 @@ func (ts *Test) prepareGenericJWTSession(testName string, method string, claimNa spec.EnableJWT = true spec.Proxy.ListenPath = "/" spec.JWTSkipKid = ApiSkipKid + spec.DisableRateLimit = true + spec.DisableQuota = true if claimName != KID { spec.JWTIdentityBaseField = claimName @@ -471,6 +473,8 @@ func (ts *Test) prepareJWTSessionRSAWithRawSourceOnWithClientID(isBench bool) st spec.JWTIdentityBaseField = "user_id" spec.JWTClientIDBaseField = "azp" spec.Proxy.ListenPath = "/" + spec.DisableRateLimit = true + spec.DisableQuota = true })[0] policyID := ts.CreatePolicy(func(p *user.Policy) { @@ -551,6 +555,8 @@ func (ts *Test) prepareJWTSessionRSAWithRawSource() string { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/" + spec.DisableRateLimit = true + spec.DisableQuota = true }) pID := ts.CreatePolicy() @@ -1440,6 +1446,8 @@ func (ts *Test) prepareJWTSessionRSAWithJWK() string { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/" + spec.DisableRateLimit = true + spec.DisableQuota = true }) pID := ts.CreatePolicy() @@ -1499,6 +1507,8 @@ func (ts *Test) prepareJWTSessionRSAWithEncodedJWK() (*APISpec, string) { spec.JWTIdentityBaseField = "user_id" spec.JWTPolicyFieldName = "policy_id" spec.Proxy.ListenPath = "/" + spec.DisableRateLimit = true + spec.DisableQuota = true })[0] pID := ts.CreatePolicy() diff --git a/gateway/mw_transform_test.go b/gateway/mw_transform_test.go index b7f6674b18a..4244b609763 100644 --- a/gateway/mw_transform_test.go +++ b/gateway/mw_transform_test.go @@ -37,11 +37,10 @@ func TestTransformNonAscii(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - spec.EnableContextVars = false - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err != nil { @@ -64,15 +63,16 @@ func BenchmarkTransformNonAscii(b *testing.B) { ts := StartTest(nil) defer ts.Close() - spec := APISpec{} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false - transform := TransformMiddleware{base} + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + + transform := &TransformMiddleware{base} for i := 0; i < b.N; i++ { r := TestReq(b, "GET", "/", in) - if err := transformBody(r, tmeta, &transform); err != nil { + if err := transformBody(r, tmeta, transform); err != nil { b.Fatalf("wanted nil error, got %v", err) } } @@ -90,10 +90,10 @@ func TestTransformXMLCrash(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err == nil { @@ -145,10 +145,10 @@ func TestTransformJSONMarshalXMLInput(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err != nil { @@ -172,10 +172,10 @@ func TestTransformJSONMarshalJSONInput(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err != nil { @@ -211,10 +211,10 @@ func TestTransformJSONMarshalJSONArrayInput(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err != nil { @@ -236,9 +236,10 @@ func BenchmarkTransformJSONMarshal(b *testing.B) { ts := StartTest(nil) defer ts.Close() - spec := APISpec{} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} for i := 0; i < b.N; i++ { @@ -257,10 +258,10 @@ func TestTransformXMLMarshal(t *testing.T) { ts := StartTest(nil) defer ts.Close() - ad := apidef.APIDefinition{} - spec := APISpec{APIDefinition: &ad} - base := BaseMiddleware{Spec: &spec, Gw: ts.Gw} - base.Spec.EnableContextVars = false + ad := &apidef.APIDefinition{} + spec := &APISpec{APIDefinition: ad} + base := BaseMiddleware{Spec: spec, Gw: ts.Gw} + transform := TransformMiddleware{base} if err := transformBody(r, tmeta, &transform); err != nil { diff --git a/gateway/mw_version_check_test.go b/gateway/mw_version_check_test.go index fb6a0910b77..ccff654a087 100644 --- a/gateway/mw_version_check_test.go +++ b/gateway/mw_version_check_test.go @@ -20,6 +20,8 @@ func (ts *Test) testPrepareVersioning() (string, string) { spec.VersionDefinition.Location = "header" spec.VersionDefinition.Key = "version" spec.Proxy.ListenPath = "/" + spec.DisableRateLimit = true + spec.DisableQuota = true spec.VersionData.Versions["expired"] = apidef.VersionInfo{ Name: "expired", Expires: "2006-01-02 15:04", diff --git a/gateway/oauth_manager_test.go b/gateway/oauth_manager_test.go index 3fbe7fc97bb..f8c99f190b6 100644 --- a/gateway/oauth_manager_test.go +++ b/gateway/oauth_manager_test.go @@ -1448,6 +1448,8 @@ func TestPurgeOAuthClientTokens(t *testing.T) { } func BenchmarkPurgeLapsedOAuthTokens(b *testing.B) { + b.Skip() + conf := func(globalConf *config.Config) { // set tokens to be expired after 1 second globalConf.OauthTokenExpire = 1 diff --git a/gateway/res_handler_header_injector_test.go b/gateway/res_handler_header_injector_test.go index 24c1b83aa9d..c5984b28b81 100644 --- a/gateway/res_handler_header_injector_test.go +++ b/gateway/res_handler_header_injector_test.go @@ -14,6 +14,8 @@ func testPrepareResponseHeaderInjection(ts *Test) { spec.UseKeylessAccess = true spec.Proxy.ListenPath = "/" spec.OrgID = "default" + spec.DisableRateLimit = true + spec.DisableQuota = true UpdateAPIVersion(spec, "v1", func(v *apidef.VersionInfo) { v.UseExtendedPaths = true json.Unmarshal([]byte(`[ @@ -118,10 +120,6 @@ func BenchmarkResponseHeaderInjection(b *testing.B) { } deleteHeaders := map[string]string{ - "X-Tyk-Test": "1", - cachedResponseHeader: "1", - } - deleteHeadersCached := map[string]string{ "X-Tyk-Test": "1", } @@ -133,9 +131,12 @@ func BenchmarkResponseHeaderInjection(b *testing.B) { {Method: "GET", Path: "/test-with-slash", HeadersMatch: addHeaders, HeadersNotMatch: deleteHeaders}, {Method: "GET", Path: "/test-no-slash", HeadersMatch: addHeaders, HeadersNotMatch: deleteHeaders}, {Method: "GET", Path: "/rewrite-test", HeadersMatch: addHeaders, HeadersNotMatch: deleteHeaders, BodyMatch: `"Url":"/newpath"`}, - {Method: "GET", Path: "/rewrite-test", HeadersMatch: addHeadersCached, HeadersNotMatch: deleteHeadersCached, BodyMatch: `"X-I-Am":"Request"`}, - {Method: "GET", Path: "/rewrite-test", HeadersMatch: addHeadersCached, HeadersNotMatch: deleteHeadersCached, BodyMatch: userAgent}, + {Method: "GET", Path: "/rewrite-test", HeadersMatch: addHeadersCached, HeadersNotMatch: deleteHeaders, BodyMatch: `"X-I-Am":"Request"`}, + {Method: "GET", Path: "/rewrite-test", HeadersMatch: addHeadersCached, HeadersNotMatch: deleteHeaders, BodyMatch: userAgent}, }...) + + // It's a loop, first time won't be cached. + addHeaders[cachedResponseHeader] = "1" } }