TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.76k stars 1.09k forks source link

Revert "[TT-2539] added access/transaction logs" #6524

Closed jeffy-mathew closed 2 months ago

jeffy-mathew commented 2 months ago

User description

Reverts TykTechnologies/tyk#6354 with QA failure


PR Type

enhancement, bug fix


Description


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

    github-actions[bot] commented 2 months ago

    API Changes

    --- prev.txt    2024-09-17 08:19:37.439154968 +0000
    +++ current.txt 2024-09-17 08:19:34.303139953 +0000
    @@ -5188,12 +5188,6 @@
    
     TYPES
    
    -type AccessLogsConfig struct {
    -   // Enable the transaction logs. Default: false
    -   Enabled bool `json:"enabled"`
    -}
    -    AccessLogsConfig defines the type of transactions logs printed to stdout
    -
     type AnalyticsConfigConfig struct {
        // Set empty for a Self-Managed installation or `rpc` for multi-cloud.
        Type string `json:"type"`
    @@ -5643,10 +5637,6 @@
        // 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"`
    @@ -11326,12 +11316,6 @@
     CONSTANTS
    
     const (
    -   HashSha256    = crypto.HashSha256
    -   HashMurmur32  = crypto.HashMurmur32
    -   HashMurmur64  = crypto.HashMurmur64
    -   HashMurmur128 = crypto.HashMurmur128
    -)
    -const (
        // DefaultConn is the default connection type. Not analytics and Not cache.
        DefaultConn = "default"
        // CacheConn is the cache connection type
    @@ -11339,26 +11323,26 @@
        // AnalyticsConn is the analytics connection type
        AnalyticsConn = "analytics"
     )
    +const B64JSONPrefix = "ey"
    +    `{"` in base64
    +
    +const MongoBsonIdLength = 24
    
     VARIABLES
    
     var (
    -   HashStr = crypto.HashStr
    -   HashKey = crypto.HashKey
    -)
    -var (
    -   GenerateToken = crypto.GenerateToken
    -   TokenHashAlgo = crypto.TokenHashAlgo
    -   TokenID       = crypto.TokenID
    -   TokenOrg      = crypto.TokenOrg
    -)
    -var (
        // ErrRedisIsDown is returned when we can't communicate with redis
        ErrRedisIsDown = errors.New("storage: Redis is either down or was not configured")
    
        // ErrStorageConn is returned when we can't get a connection from the ConnectionHandler
        ErrStorageConn = fmt.Errorf("Error trying to get singleton instance: %w", ErrRedisIsDown)
     )
    +var (
    +   HashSha256    = "sha256"
    +   HashMurmur32  = "murmur32"
    +   HashMurmur64  = "murmur64"
    +   HashMurmur128 = "murmur128"
    +)
     var ErrKeyNotFound = errors.New("key not found")
         ErrKeyNotFound is a standard error for when a key is not found in the
         storage engine
    @@ -11367,9 +11351,19 @@
    
     FUNCTIONS
    
    +func GenerateToken(orgID, keyID, hashAlgorithm string) (string, error)
    +    If hashing algorithm is empty, use legacy key generation
    +
    +func HashKey(in string, hashKey bool) string
    +func HashStr(in string, withAlg ...string) string
     func NewConnector(connType string, conf config.Config) (model.Connector, error)
         NewConnector creates a new storage connection.
    
    +func TokenHashAlgo(token string) string
    +func TokenID(token string) (id string, err error)
    +    TODO: add checks
    +
    +func TokenOrg(token string) string
    
     TYPES
    
    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

    Error Handling
    The function `GenerateToken` in `storage/storage.go` does not handle the error from `hashFunction` properly. It defaults to `defaultHashAlgorithm` without logging or handling the initial error, which could lead to silent failures or unexpected behavior in token generation. Function Complexity
    The function `TokenOrg` in `storage/storage.go` has multiple nested conditions and returns, making it complex and hard to maintain. Consider refactoring to simplify the logic and improve readability.
    github-actions[bot] commented 2 months ago

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    sonarcloud[bot] commented 2 months ago

    Quality Gate Failed Quality Gate failed

    Failed conditions
    C Reliability Rating on New Code (required β‰₯ A)

    See analysis details on SonarCloud

    Catch issues before they fail your Quality Gate with our IDE extension SonarLint

    jeffy-mathew commented 2 months ago

    /release to release-5.6

    tykbot[bot] commented 2 months ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 2 months ago

    @jeffy-mathew Succesfully merged PR

    jeffy-mathew commented 2 months ago

    /release to release-5.6.0

    tykbot[bot] commented 2 months ago

    Working on it! Note that it can take a few minutes.

    tykbot[bot] commented 2 months ago

    @jeffy-mathew Succesfully merged PR