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

buger · 2024-09-17T10:54:43Z

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

### **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 (cherry picked from commit 3e435cc)

github-actions · 2024-09-17T10:55:48Z

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 · 2024-09-17T10:55:50Z

PR Code Suggestions ✨

No code suggestions found for the PR.

sonarqubecloud · 2024-09-17T10:56:02Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

buger merged commit d40aca9 into release-5.6.0 Sep 17, 2024

buger deleted the merge/release-5.6.0/3e435cc9abff13d2cfafe36acd4b500009daf8e9 branch September 17, 2024 10:54

github-actions bot added enhancement Bug fix Review effort [1-5]: 3 labels Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

buger commented Sep 17, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Sep 17, 2024

github-actions bot commented Sep 17, 2024 •

edited

Loading

sonarqubecloud bot commented Sep 17, 2024

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

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

Conversation

buger commented Sep 17, 2024 • edited by github-actions bot Loading

User description

User description

PR Type

Description

Changes walkthrough 📝

PR Type

Description

Changes walkthrough 📝

github-actions bot commented Sep 17, 2024

PR Reviewer Guide 🔍

github-actions bot commented Sep 17, 2024 • edited Loading

PR Code Suggestions ✨

sonarqubecloud bot commented Sep 17, 2024

Quality Gate passed

buger commented Sep 17, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading