Merging to release-5.6.0: Revert "[TT-2539] added access/transaction logs" (#6524)

buger commented 2 months ago

User description

Revert "TT-2539 added access/transaction logs" (#6524)

User description

Reverts TykTechnologies/tyk#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 📝

Relevant files

Enhancement

8 files

config.go `Remove access logs configuration from analytics settings` config/config.go Removed `AccessLogsConfig` struct and its usage. Deleted configuration for access logs.	+0/-10
middleware.go `Remove access log recording function from middleware` gateway/middleware.go - Removed `recordAccessLog` function.	+0/-16
hash.go `Remove crypto hashing functions` internal/crypto/hash.go - Deleted the entire file related to hashing functions.	+0/-53
token.go `Remove token generation and parsing functions` internal/crypto/token.go - Deleted the entire file related to token generation and parsing.	+0/-83
access_log.go `Remove access log record functionality` internal/httputil/accesslog/access_log.go - Deleted the entire file related to access log records.	+0/-89
alias.go `Remove storage alias for crypto functions` storage/alias.go Deleted the entire file related to storage alias for crypto functions.	+0/-24
storage.go `Integrate hashing and token functions into storage` storage/storage.go - Moved hashing and token functions from `crypto` to `storage`.	+125/-0
schema.json `Remove access logs configuration from schema` cli/linter/schema.json - Removed `access_logs` configuration from the schema.	+0/-9

Bug fix

2 files

handler_error.go `Remove transaction log printing in error handler` gateway/handler_error.go - Removed transaction log printing for error situations.	+0/-7
handler_success.go `Remove transaction log printing in success handler` gateway/handler_success.go - Removed transaction log printing for successful requests.	+2/-8

Tests

3 files

handler_error_test.go `Remove access logs benchmark tests from error handler tests` gateway/handler_error_test.go - Removed benchmark tests related to access logs.	+0/-51
handler_success_test.go `Remove access logs benchmark tests from success handler tests` gateway/handler_success_test.go - Removed benchmark tests related to access logs.	+0/-55
access_log_test.go `Remove access log tests` internal/httputil/accesslog/access_log_test.go - Deleted the entire file related to access log tests.	+0/-99

Configuration changes

1 files

Taskfile.yml

Update test command in
Taskfile

internal/httputil/Taskfile.yml - Updated test command to remove specific coverage package.

+1/-2

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

PR Type

Bug fix, Enhancement

Description

Removed AccessLogsConfig struct and related 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 📝

Relevant files

Enhancement

9 files

config.go `Remove access logs configuration from config` config/config.go Removed `AccessLogsConfig` struct and its usage. Deleted configuration for access logs.	+0/-10
handler_error.go `Remove transaction log printing in error handler` gateway/handler_error.go - Removed transaction log printing for error situations.	+0/-7
handler_success.go `Remove transaction log printing in success handler` gateway/handler_success.go - Removed transaction log printing for success situations.	+2/-8
middleware.go `Remove access log recording function from middleware` gateway/middleware.go - Removed `recordAccessLog` function.	+0/-16
hash.go `Remove hash utility functions` internal/crypto/hash.go - Deleted hash utility functions.	+0/-53
token.go `Remove token utility functions` internal/crypto/token.go - Deleted token utility functions.	+0/-83
access_log.go `Remove access log record implementation` internal/httputil/accesslog/access_log.go - Deleted access log record implementation.	+0/-89
alias.go `Remove crypto function aliases` storage/alias.go - Removed alias definitions for crypto functions.	+0/-24
storage.go `Consolidate hashing and token functions in storage` storage/storage.go - Moved hashing and token functions from `crypto` to `storage`.	+125/-0

Tests

3 files

handler_error_test.go `Remove access log benchmark tests from error handler tests` gateway/handler_error_test.go - Removed benchmark tests related to access logs.	+0/-51
handler_success_test.go `Remove access log benchmark tests from success handler tests` gateway/handler_success_test.go - Removed benchmark tests related to access logs.	+0/-55
access_log_test.go `Remove tests for access log records` internal/httputil/accesslog/access_log_test.go - Deleted tests for access log records.	+0/-99

Configuration changes

2 files

schema.json `Remove access logs configuration from schema` cli/linter/schema.json - Removed access logs configuration from schema.	+0/-9
Taskfile.yml `Update test coverage package path in Taskfile` internal/httputil/Taskfile.yml - Updated test coverage package path.	+1/-2

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 2 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

Function Complexity
The function `hashFunction` in `storage/storage.go` uses a default case that logs an error but returns a valid hash function (`murmur32`). This could lead to silent errors in the system where an unsupported hash algorithm is specified but the system continues to work incorrectly. Consider throwing an error or handling this case more explicitly. Error Handling
The function `GenerateToken` in `storage/storage.go` does not handle the error from `hashFunction` properly. It proceeds with encoding the token even if there is an error in determining the hash function, which might lead to generating incorrect tokens.

github-actions[bot] commented 2 months ago