Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

buger
Copy link
Member

@buger buger commented Sep 17, 2024

User description

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 📝

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 the 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 analytics settings 

    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                                         

    gateway/middleware.go

    • Removed recordAccessLog function.
    +0/-16   
    hash.go
    Remove hash functions from crypto package                               

    internal/crypto/hash.go

    • Removed hash-related functions.
    +0/-53   
    token.go
    Remove token functions from crypto package                             

    internal/crypto/token.go

    • Removed token-related functions.
    +0/-83   
    access_log.go
    Remove access log record struct and methods                           

    internal/httputil/accesslog/access_log.go

    • Removed access log record struct and related methods.
    +0/-89   
    alias.go
    Remove crypto alias references in storage                               

    storage/alias.go

    • Removed alias references to crypto package.
    +0/-24   
    storage.go
    Move hashing and token functions to storage package           

    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 record struct                               

    internal/httputil/accesslog/access_log_test.go

    • Removed tests for access log record struct.
    +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 configuration in Taskfile                     

    internal/httputil/Taskfile.yml

    • Updated test coverage configuration.
    +1/-2     

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

    ### **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>&nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>middleware.go</strong><dd><code>Remove access log
    recording function from middleware</code>&nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>hash.go</strong><dd><code>Remove crypto hashing
    functions</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>token.go</strong><dd><code>Remove token generation and
    parsing functions</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>access_log.go</strong><dd><code>Remove access log
    record functionality</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>alias.go</strong><dd><code>Remove storage alias for
    crypto functions</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>storage.go</strong><dd><code>Integrate hashing and
    token functions into storage</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; </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>&nbsp;
    </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>schema.json</strong><dd><code>Remove access logs
    configuration from schema</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; &nbsp; </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>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>handler_success.go</strong><dd><code>Remove transaction
    log printing in success handler</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; </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>&nbsp;
    &nbsp; &nbsp; </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>&nbsp;
    &nbsp; </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>&nbsp;
    &nbsp; </td>
    
    </tr>                    
    
    <tr>
      <td>
        <details>
    <summary><strong>access_log_test.go</strong><dd><code>Remove access log
    tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; </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>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
    &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </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>&nbsp;
    &nbsp; &nbsp; </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
    
    (cherry picked from commit 3e435cc)
    @buger buger merged commit 0c97c01 into release-5.6 Sep 17, 2024
    @buger buger deleted the merge/release-5.6/3e435cc9abff13d2cfafe36acd4b500009daf8e9 branch September 17, 2024 08:45
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Code Quality
    The new hashing and token functions added to storage.go introduce significant complexity and potential for errors. It's recommended to add unit tests to ensure the correctness of these functions, especially since they handle sensitive operations like token generation and hashing.

    Copy link
    Contributor

    github-actions bot commented Sep 17, 2024

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Copy link

    sonarcloud bot commented Sep 17, 2024

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants