Merging to release-5.6.0: Revert "[TT-2539] added access/transaction …

…logs" (#6524) Revert "[TT-2539] added access/transaction logs" (#6524) ### **User description** Reverts #6354 with QA failure ___ ### **PR Type** enhancement, bug fix ___ ### **Description** - Removed the `AccessLogsConfig` struct and related access log configurations from the codebase. - Deleted functions and tests related to access logs, including transaction log printing in both error and success handlers. - Moved hashing and token functions from `crypto` to `storage`, consolidating related functionality. - Updated the schema and Taskfile to reflect the removal of access logs and related configurations. ___ ### **Changes walkthrough** 📝 <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><details><summary>8 files</summary><table> <tr> <td> <details> <summary><strong>config.go</strong><dd><code>Remove access logs configuration from analytics settings</code>  </dd></summary> <hr> config/config.go <li>Removed <code>AccessLogsConfig</code> struct and its usage.<br> <li> Deleted configuration for access logs. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-fe44f09c4d5977b5f5eaea29170b6a0748819c9d02271746a20d81a5f3efca17">+0/-10</a>    </td> </tr> <tr> <td> <details> <summary><strong>middleware.go</strong><dd><code>Remove access log recording function from middleware</code>          </dd></summary> <hr> gateway/middleware.go - Removed `recordAccessLog` function. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-703054910891a4db633eca0f42ed779d6b4fa75cd9b3aa4c503e681364201c1b">+0/-16</a>    </td> </tr> <tr> <td> <details> <summary><strong>hash.go</strong><dd><code>Remove crypto hashing functions</code>                                                    </dd></summary> <hr> internal/crypto/hash.go - Deleted the entire file related to hashing functions. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-fd1c33ede81b9c5740cabc411ea8e4ce122cf642382b699114dfddcc49ea84d6">+0/-53</a>    </td> </tr> <tr> <td> <details> <summary><strong>token.go</strong><dd><code>Remove token generation and parsing functions</code>                        </dd></summary> <hr> internal/crypto/token.go - Deleted the entire file related to token generation and parsing. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-25b0099bc38076a27697918a7d82178f3f031a5abb58ae30c70c22d7332ee918">+0/-83</a>    </td> </tr> <tr> <td> <details> <summary><strong>access_log.go</strong><dd><code>Remove access log record functionality</code>                                      </dd></summary> <hr> internal/httputil/accesslog/access_log.go - Deleted the entire file related to access log records. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-39a7ae68c9608f8e05ab9207081cb3be248d00d68a5a2412304b83b2340401b7">+0/-89</a>    </td> </tr> <tr> <td> <details> <summary><strong>alias.go</strong><dd><code>Remove storage alias for crypto functions</code>                                </dd></summary> <hr> storage/alias.go <li>Deleted the entire file related to storage alias for crypto functions. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-824743fa5c64b4ca24743cdcb0c3014513632cb0bcad7b225d162510698c9a82">+0/-24</a>    </td> </tr> <tr> <td> <details> <summary><strong>storage.go</strong><dd><code>Integrate hashing and token functions into storage</code>              </dd></summary> <hr> storage/storage.go - Moved hashing and token functions from `crypto` to `storage`. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+125/-0</a>  </td> </tr> <tr> <td> <details> <summary><strong>schema.json</strong><dd><code>Remove access logs configuration from schema</code>                          </dd></summary> <hr> cli/linter/schema.json - Removed `access_logs` configuration from the schema. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-103cec746d3e61d391c5a67c171963f66fea65d651d704d5540e60aa5d574f46">+0/-9</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Bug fix</strong></td><td><details><summary>2 files</summary><table> <tr> <td> <details> <summary><strong>handler_error.go</strong><dd><code>Remove transaction log printing in error handler</code>                  </dd></summary> <hr> gateway/handler_error.go - Removed transaction log printing for error situations. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-d3b05530ad23401f3b8f33bb1a467cd807a29a6b09c7430d01d069f626b20f77">+0/-7</a>      </td> </tr> <tr> <td> <details> <summary><strong>handler_success.go</strong><dd><code>Remove transaction log printing in success handler</code>              </dd></summary> <hr> gateway/handler_success.go - Removed transaction log printing for successful requests. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-45135957493eca37f2e3e9a81079577777133c53b27cf95ea2ff0329c05bd006">+2/-8</a>      </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>3 files</summary><table> <tr> <td> <details> <summary><strong>handler_error_test.go</strong><dd><code>Remove access logs benchmark tests from error handler tests</code></dd></summary> <hr> gateway/handler_error_test.go - Removed benchmark tests related to access logs. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-530de66339b5f5dbe9e19144fe4215f0aa021c8414c8f3b840d3175b2d6675da">+0/-51</a>    </td> </tr> <tr> <td> <details> <summary><strong>handler_success_test.go</strong><dd><code>Remove access logs benchmark tests from success handler tests</code></dd></summary> <hr> gateway/handler_success_test.go - Removed benchmark tests related to access logs. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-42a565d872ff6c1f380386f4ee2f75bfa87991b52728a1a9a1772452bb0cd1bb">+0/-55</a>    </td> </tr> <tr> <td> <details> <summary><strong>access_log_test.go</strong><dd><code>Remove access log tests</code>                                                                    </dd></summary> <hr> internal/httputil/accesslog/access_log_test.go - Deleted the entire file related to access log tests. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-7e171bde9a23d0039c5dd0c1f0718a1da2dcf93211f997ce8798d4a9a14450f0">+0/-99</a>    </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>1 files</summary><table> <tr> <td> <details> <summary><strong>Taskfile.yml</strong><dd><code>Update test command in Taskfile</code>                                                    </dd></summary> <hr> internal/httputil/Taskfile.yml - Updated test command to remove specific coverage package. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6524/files#diff-31877aaad48d3baf442aa52077c28b873df2f2ac6c70941d409261460d7c3c8e">+1/-2</a>      </td> </tr> </table></details></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: >Comment `/help` on the PR to get a list of all available PR-Agent tools and their descriptions
TykTechnologies · Sep 17, 2024 · d40aca9 · d40aca9
1 parent bc067e7
commit d40aca9
Show file tree

Hide file tree

Showing 15 changed files with 128 additions and 507 deletions.
diff --git a/.gitignore b/.gitignore
@@ -50,7 +50,6 @@ session_state_gen_test.go
 __pycache__/
 tyk.test
 tyk-gateway.pid
-*.go-e
 
 tyk_linux_*
 /dist/

diff --git a/cli/linter/schema.json b/cli/linter/schema.json
@@ -648,15 +648,6 @@
       "type": "string",
       "enum": ["", "standard", "json"]
     },
-    "access_logs": {
-      "type": ["object", "null"],
-      "additionalProperties": false,
-      "properties": {
-        "enabled": {
-          "type": "boolean"
-        }
-      }
-    },
     "enable_http_profiler": {
       "type": "boolean"
     },

diff --git a/config/config.go b/config/config.go
@@ -246,12 +246,6 @@ type AnalyticsConfigConfig struct {
 	SerializerType string `json:"serializer_type"`
 }
 
-// AccessLogsConfig defines the type of transactions logs printed to stdout
-type AccessLogsConfig struct {
-	// Enable the transaction logs. Default: false
-	Enabled bool `json:"enabled"`
-}
-
 type HealthCheckConfig struct {
 	// Setting this value to `true` will enable the health-check endpoint on /Tyk/health.
 	EnableHealthChecks bool `json:"enable_health_checks"`
@@ -1009,10 +1003,6 @@ type Config struct {
 	// If not set or left empty, it will default to `standard`.
 	LogFormat string `json:"log_format"`
 
-	// You can configure the transaction logs to be turned on
-	// If not set or left empty, it will default to 'false'
-	AccessLogs AccessLogsConfig `json:"access_logs"`
-
 	// Section for configuring OpenTracing support
 	// Deprecated: use OpenTelemetry instead.
 	Tracer Tracer `json:"tracing"`

diff --git a/gateway/handler_error.go b/gateway/handler_error.go
@@ -311,13 +311,6 @@ func (e *ErrorHandler) HandleError(w http.ResponseWriter, r *http.Request, errMs
 			log.WithError(err).Error("could not store analytic record")
 		}
 	}
-
-	// Print the transaction logs for error situations if enabled. Success transaction
-	// logs will be handled by the "handler_success.go"
-	if e.Spec.GlobalConfig.AccessLogs.Enabled {
-		e.recordAccessLog(r, response, nil)
-	}
-
 	// Report in health check
 	reportHealthValue(e.Spec, BlockedRequestLog, "-1")
 }
diff --git a/gateway/handler_error_test.go b/gateway/handler_error_test.go
@@ -9,8 +9,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/TykTechnologies/tyk/config"
-
 	"github.com/TykTechnologies/tyk/header"
 	"github.com/TykTechnologies/tyk/test"
 )
@@ -124,52 +122,3 @@ func TestHandleDefaultErrorJSON(t *testing.T) {
 	})
 
 }
-
-func BenchmarkErrorLogTransaction(b *testing.B) {
-	b.Run("AccessLogs enabled with Hashkeys set to true", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = true
-			globalConf.AccessLogs.Enabled = true
-		}
-		benchmarkErrorLogTransaction(b, conf)
-
-	})
-	b.Run("AccessLogs enabled with Hashkeys set to false", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = false
-			globalConf.AccessLogs.Enabled = true
-		}
-		benchmarkErrorLogTransaction(b, conf)
-	})
-
-	b.Run("AccessLogs disabled with Hashkeys set to true", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = true
-			globalConf.AccessLogs.Enabled = false
-		}
-		benchmarkErrorLogTransaction(b, conf)
-	})
-
-	b.Run("AccessLogs disabled with Hashkeys set to false", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = false
-			globalConf.AccessLogs.Enabled = false
-		}
-		benchmarkErrorLogTransaction(b, conf)
-	})
-}
-
-func benchmarkErrorLogTransaction(b *testing.B, conf func(globalConf *config.Config)) {
-	b.ReportAllocs()
-	b.Helper()
-	b.ResetTimer()
-
-	ts := StartTest(conf)
-	defer ts.Close()
-
-	for i := 0; i < b.N; i++ {
-		ts.Run(b, test.TestCase{
-			Code: http.StatusNotFound,
-		})
-	}
-}
diff --git a/gateway/handler_success.go b/gateway/handler_success.go
@@ -10,8 +10,9 @@ import (
 	"strings"
 	"time"
 
-	"github.com/TykTechnologies/tyk/apidef"
 	graphqlinternal "github.com/TykTechnologies/tyk/internal/graphql"
+
+	"github.com/TykTechnologies/tyk/apidef"
 	"github.com/TykTechnologies/tyk/internal/httputil"
 
 	"github.com/TykTechnologies/tyk-pump/analytics"
@@ -381,15 +382,8 @@ func (s *SuccessHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) *http
 			Upstream: int64(DurationToMillisecond(resp.UpstreamLatency)),
 		}
 		s.RecordHit(r, latency, resp.Response.StatusCode, resp.Response, false)
-
-		// Don't print a transaction log there is no "resp", that indicates an error.
-		// In error situations, transaction log is already printed by "handler_error.go"
-		if s.Spec.GlobalConfig.AccessLogs.Enabled {
-			s.recordAccessLog(r, resp.Response, &latency)
-		}
 	}
 	log.Debug("Done proxy")
-
 	return nil
 }
 

diff --git a/gateway/handler_success_test.go b/gateway/handler_success_test.go
@@ -326,58 +326,3 @@ func TestAnalyticsIgnoreSubgraph(t *testing.T) {
 	)
 	assert.NoError(t, err)
 }
-
-func BenchmarkSuccessLogTransaction(b *testing.B) {
-	b.Run("AccessLogs enabled with Hashkeys set to true", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = true
-			globalConf.AccessLogs.Enabled = true
-		}
-		benchmarkSuccessLogTransaction(b, conf)
-
-	})
-	b.Run("AccessLogs enabled with Hashkeys set to false", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = false
-			globalConf.AccessLogs.Enabled = true
-		}
-		benchmarkSuccessLogTransaction(b, conf)
-	})
-	b.Run("AccessLogs disabled with Hashkeys set to true", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = true
-			globalConf.AccessLogs.Enabled = false
-		}
-		benchmarkSuccessLogTransaction(b, conf)
-	})
-	b.Run("AccessLogs disabled with Hashkeys set to false", func(b *testing.B) {
-		conf := func(globalConf *config.Config) {
-			globalConf.HashKeys = false
-			globalConf.AccessLogs.Enabled = false
-		}
-		benchmarkSuccessLogTransaction(b, conf)
-	})
-}
-
-func benchmarkSuccessLogTransaction(b *testing.B, conf func(globalConf *config.Config)) {
-	b.ReportAllocs()
-	b.Helper()
-	b.ResetTimer()
-
-	ts := StartTest(conf)
-	defer ts.Close()
-
-	API := BuildAPI(func(spec *APISpec) {
-		spec.Name = "test-api"
-		spec.APIID = "test-api-id"
-		spec.Proxy.ListenPath = "/"
-	})[0]
-
-	ts.Gw.LoadAPI(API)
-
-	for i := 0; i < b.N; i++ {
-		ts.Run(b, test.TestCase{
-			Code: http.StatusOK,
-		})
-	}
-}
diff --git a/gateway/middleware.go b/gateway/middleware.go
@@ -12,11 +12,8 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/TykTechnologies/tyk-pump/analytics"
-
 	"github.com/TykTechnologies/tyk/internal/cache"
 	"github.com/TykTechnologies/tyk/internal/event"
-	"github.com/TykTechnologies/tyk/internal/httputil/accesslog"
 	"github.com/TykTechnologies/tyk/internal/otel"
 	"github.com/TykTechnologies/tyk/internal/policy"
 	"github.com/TykTechnologies/tyk/rpc"
@@ -361,19 +358,6 @@ func (t *BaseMiddleware) ApplyPolicies(session *user.SessionState) error {
 	return store.Apply(session)
 }
 
-// recordAccessLog is only used for Success/Error handler
-func (t *BaseMiddleware) recordAccessLog(req *http.Request, resp *http.Response, latency *analytics.Latency) {
-	hashKeys := t.Gw.GetConfig().HashKeys
-
-	accessLog := accesslog.NewRecord(t.Spec.APIID, t.Spec.OrgID)
-	accessLog.WithAuthToken(req, hashKeys, t.Gw.obfuscateKey)
-	accessLog.WithLatency(latency)
-	accessLog.WithRequest(req)
-	accessLog.WithResponse(resp)
-
-	t.Logger().WithFields(accessLog.Fields()).Info()
-}
-
 func copyAllowedURLs(input []user.AccessSpec) []user.AccessSpec {
 	if input == nil {
 		return nil

diff --git a/internal/crypto/hash.go b/internal/crypto/hash.go
diff --git a/internal/crypto/token.go b/internal/crypto/token.go
diff --git a/internal/httputil/Taskfile.yml b/internal/httputil/Taskfile.yml
@@ -3,14 +3,13 @@ version: "3"
 
 vars:
   testArgs: -v
-  coverpkg: ./...,github.com/TykTechnologies/tyk/internal/httputil/...
 
 tasks:
   test:
     desc: "Run tests (requires redis)"
     cmds:
       - task: fmt
-      - go test {{.testArgs}} -count=1 -cover -coverprofile=rate.cov {{.coverpkg}} ./...
+      - go test {{.testArgs}} -count=1 -cover -coverprofile=rate.cov -coverpkg=./... ./...
 
   bench:
     desc: "Run benchmarks"