From 9988fd5712f784d98b3b98d18629e0bb6c1d65f0 Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Fri, 18 Oct 2024 22:27:35 +0200 Subject: [PATCH 1/7] revert wrappedServeHTTP to use recordDetail --- .gitignore | 2 ++ gateway/reverse_proxy.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c3d3051bdea..4be204743e6 100644 --- a/.gitignore +++ b/.gitignore @@ -61,3 +61,5 @@ tyk_linux_* *.test main + +/coprocess/*.pb.go-e diff --git a/gateway/reverse_proxy.go b/gateway/reverse_proxy.go index 820c4201638..6d2731b1004 100644 --- a/gateway/reverse_proxy.go +++ b/gateway/reverse_proxy.go @@ -515,7 +515,7 @@ func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) Prox startTime := time.Now() p.logger.WithField("ts", startTime.UnixNano()).Debug("Started") - resp := p.WrappedServeHTTP(rw, req, true) + resp := p.WrappedServeHTTP(rw, req, recordDetail(req, p.TykAPISpec)) finishTime := time.Since(startTime) p.logger.WithField("ns", finishTime.Nanoseconds()).Debug("Finished") From 177015ba236568db9447ca35580fd922135cd8a5 Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Mon, 21 Oct 2024 12:25:44 +0200 Subject: [PATCH 2/7] record detail when API is of type graphQL --- gateway/handler_success.go | 2 +- gateway/handler_success_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/gateway/handler_success.go b/gateway/handler_success.go index 0f24aadc3f9..da4e1313830 100644 --- a/gateway/handler_success.go +++ b/gateway/handler_success.go @@ -345,7 +345,7 @@ func recordDetail(r *http.Request, spec *APISpec) bool { // Are we even checking? if !spec.GlobalConfig.EnforceOrgDataDetailLogging { - return spec.GlobalConfig.AnalyticsConfig.EnableDetailedRecording + return spec.GraphQL.Enabled || spec.GlobalConfig.AnalyticsConfig.EnableDetailedRecording } // We are, so get session data diff --git a/gateway/handler_success_test.go b/gateway/handler_success_test.go index 9f1c4a1d9d3..e96cf99eecd 100644 --- a/gateway/handler_success_test.go +++ b/gateway/handler_success_test.go @@ -102,6 +102,13 @@ func TestRecordDetail(t *testing.T) { }, expect: true, }, + { + title: "graphql request", + spec: testAPISpec(func(spec *APISpec) { + spec.GraphQL.Enabled = true + }), + expect: true, + }, } for _, tc := range testcases { From eedbb59d76db51fa3c28a5bae41744f924c59be4 Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Mon, 21 Oct 2024 13:57:22 +0200 Subject: [PATCH 3/7] improve comments --- gateway/handler_success.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/gateway/handler_success.go b/gateway/handler_success.go index da4e1313830..f61281a2b7f 100644 --- a/gateway/handler_success.go +++ b/gateway/handler_success.go @@ -343,19 +343,16 @@ func recordDetail(r *http.Request, spec *APISpec) bool { } } - // Are we even checking? - if !spec.GlobalConfig.EnforceOrgDataDetailLogging { - return spec.GraphQL.Enabled || spec.GlobalConfig.AnalyticsConfig.EnableDetailedRecording - } - - // We are, so get session data - session, ok := r.Context().Value(ctx.OrgSessionContext).(*user.SessionState) - if ok && session != nil { - return session.EnableDetailedRecording || session.EnableDetailRecording // nolint:staticcheck // Deprecated DetailRecording + // decide based on org session. + if spec.GlobalConfig.EnforceOrgDataDetailLogging { + session, ok := r.Context().Value(ctx.OrgSessionContext).(*user.SessionState) + if ok && session != nil { + return session.EnableDetailedRecording || session.EnableDetailRecording // nolint:staticcheck // Deprecated DetailRecording + } } - // no session found, use global config - return spec.GlobalConfig.AnalyticsConfig.EnableDetailedRecording + // no org session found, use global config + return spec.GraphQL.Enabled || spec.GlobalConfig.AnalyticsConfig.EnableDetailedRecording } // ServeHTTP will store the request details in the analytics store if necessary and proxy the request to it's From 9a26d18ac418e388ef58e4f3cbbde7826f29f8dc Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Tue, 22 Oct 2024 08:19:11 +0200 Subject: [PATCH 4/7] add benchmark test --- gateway/reverse_proxy_test.go | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/gateway/reverse_proxy_test.go b/gateway/reverse_proxy_test.go index bfab9ab339c..b5ed8c8cb4e 100644 --- a/gateway/reverse_proxy_test.go +++ b/gateway/reverse_proxy_test.go @@ -14,6 +14,7 @@ import ( "net/http/httptest" "net/url" "reflect" + "strconv" "strings" "sync" "testing" @@ -2023,3 +2024,45 @@ func TestQuotaResponseHeaders(t *testing.T) { }) } + +func BenchmarkLargeResponsePayload(b *testing.B) { + ts := StartTest(func(globalConf *config.Config) {}) + b.Cleanup(func() { + ts.Close() + }) + + // Create a 500 MB payload of zeros + payloadSize := 500 * 1024 * 1024 // 500 MB in bytes + largePayload := bytes.Repeat([]byte("x"), payloadSize) + + largePayloadHandler := func(w http.ResponseWriter, r *http.Request) { + + // Write the payload to the response writer + w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Length", strconv.Itoa(payloadSize)) + w.WriteHeader(http.StatusOK) + w.Write(largePayload) + } + + // Create a test server with the largePayloadHandler + testServer := httptest.NewServer(http.HandlerFunc(largePayloadHandler)) + b.Cleanup(func() { + testServer.Close() + }) + + ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { + spec.UseKeylessAccess = true + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = testServer.URL + }) + + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + ts.Run(b, test.TestCase{ + Method: http.MethodGet, + Path: "/", + Code: http.StatusOK, + }) + } +} From 37dbb4f242f88d2e827f63e711a0d1ed54ab084c Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Tue, 22 Oct 2024 09:33:48 +0200 Subject: [PATCH 5/7] gh warnings --- gateway/reverse_proxy_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gateway/reverse_proxy_test.go b/gateway/reverse_proxy_test.go index b5ed8c8cb4e..1a1f9962519 100644 --- a/gateway/reverse_proxy_test.go +++ b/gateway/reverse_proxy_test.go @@ -2026,7 +2026,7 @@ func TestQuotaResponseHeaders(t *testing.T) { } func BenchmarkLargeResponsePayload(b *testing.B) { - ts := StartTest(func(globalConf *config.Config) {}) + ts := StartTest(func(_ *config.Config) {}) b.Cleanup(func() { ts.Close() }) @@ -2035,13 +2035,13 @@ func BenchmarkLargeResponsePayload(b *testing.B) { payloadSize := 500 * 1024 * 1024 // 500 MB in bytes largePayload := bytes.Repeat([]byte("x"), payloadSize) - largePayloadHandler := func(w http.ResponseWriter, r *http.Request) { + largePayloadHandler := func(w http.ResponseWriter, _ *http.Request) { // Write the payload to the response writer w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Length", strconv.Itoa(payloadSize)) w.WriteHeader(http.StatusOK) - w.Write(largePayload) + _, _ = w.Write(largePayload) } // Create a test server with the largePayloadHandler From 9fedac3d3601042d91e0454b4fea625f77f32908 Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Tue, 22 Oct 2024 11:08:52 +0200 Subject: [PATCH 6/7] address review comments --- gateway/reverse_proxy_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/gateway/reverse_proxy_test.go b/gateway/reverse_proxy_test.go index 1a1f9962519..65a6d0b48c1 100644 --- a/gateway/reverse_proxy_test.go +++ b/gateway/reverse_proxy_test.go @@ -2027,28 +2027,23 @@ func TestQuotaResponseHeaders(t *testing.T) { func BenchmarkLargeResponsePayload(b *testing.B) { ts := StartTest(func(_ *config.Config) {}) - b.Cleanup(func() { - ts.Close() - }) + b.Cleanup(ts.Close) // Create a 500 MB payload of zeros payloadSize := 500 * 1024 * 1024 // 500 MB in bytes largePayload := bytes.Repeat([]byte("x"), payloadSize) largePayloadHandler := func(w http.ResponseWriter, _ *http.Request) { - - // Write the payload to the response writer w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Length", strconv.Itoa(payloadSize)) w.WriteHeader(http.StatusOK) - _, _ = w.Write(largePayload) + _, err := w.Write(largePayload) + assert.NoError(b, err) } // Create a test server with the largePayloadHandler testServer := httptest.NewServer(http.HandlerFunc(largePayloadHandler)) - b.Cleanup(func() { - testServer.Close() - }) + b.Cleanup(testServer.Close) ts.Gw.BuildAndLoadAPI(func(spec *APISpec) { spec.UseKeylessAccess = true From b679b766f86b9f30089ba945c146093fffe2d380 Mon Sep 17 00:00:00 2001 From: Jeffy Mathew Date: Tue, 22 Oct 2024 11:22:33 +0200 Subject: [PATCH 7/7] use TykTechnologies/tyk-pro instead of tyklabs/tyk-pro --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f39edc6bd5c..8cfc29ff971 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -334,7 +334,7 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | - gh release download --repo github.com/tyklabs/tyk-pro --archive tar.gz -O env.tgz + gh release download --repo github.com/TykTechnologies/tyk-pro --archive tar.gz -O env.tgz mkdir auto && tar --strip-components=1 -C auto -xzvf env.tgz - name: env up shell: bash