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

[TT-2539] added access/transaction logs #6616

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

LLe27
Copy link
Contributor

@LLe27 LLe27 commented Oct 8, 2024

TT-2539
Summary Transaction/Access Logs
Type Story Story
Status In Dev
Points N/A
Labels A, America's, CSE, Gold, customer_request, innersource, jira_escalated, QA_Fail

Reverts #6524

FR Jira Ticket

https://tyktech.atlassian.net/browse/TT-2539

Description

  • Added the TYK_GW_ACCESSLOGS_ENABLED Gateway config option
  • The Tyk Gateway will determine to print access logs to STDOUT for both success and error handling situations
    • If the TYK_GW_ACCESSLOGS_ENABLED is set to true then the Gateway will print access logs to STDOUT
    • If the TYK_GW_ACCESSLOGS_ENABLED is set to false then the Gateway will not print access logs to STDOUT

Note that this feature is off by default and that the AccessLog struct only contains the more common elements. Below are some examples of an access log

time="Sep 04 08:04:18" level=info APIID=c062396cb62d4e9a5ee37adaf85b9e4c APIKey=00000000 ClientIP=127.0.0.1 ClientRemoteAddr="127.0.0.1:53506" Host="localhost:8080" Method=GET OrgID=66d07f00247d80811d5199c3 Proto=HTTP/1.1 RequestURI=/httpbin/get StatusCode=200 TotalLatency=381 UpstreamAddress="http://httpbin.org/get" UpstreamLatency=381 UpstreamPath=/get UpstreamURI=/get UserAgent=curl/8.1.2 prefix=access-log
time="Sep 04 08:08:20" level=info APIID=c062396cb62d4e9a5ee37adaf85b9e4c APIKey=00000000 ClientIP=127.0.0.1 ClientRemoteAddr="127.0.0.1:53566" Host="localhost:8080" Method=GET OrgID=66d07f00247d80811d5199c3 Proto=HTTP/1.1 RequestURI=/httpbin/get StatusCode=401 TotalLatency=0 UpstreamAddress=":///httpbin/get" UpstreamLatency=0 UpstreamPath=/httpbin/get UpstreamURI=/httpbin/get UserAgent=curl/8.1.2 prefix=access-log

Related Issue

Motivation and Context

Today the Tyk Gateway does not print access logs for success API calls but instead only for error API calls. Providing access logs for both scenarios within the Tyk Gateway is extremely valuable especially if you are monitoring logs, capturing analytics or even debugging. Providing the option to turn on or off the Tyk Gateway access logs will provide clients more insights in for API calls in regards to success and error situations.

How This Has Been Tested

  • Manual testing
  • Unit testing
  • Performance testing/benchmarks

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@buger
Copy link
Member

buger commented Oct 8, 2024

I'm a bot and I 👍 this PR title. 🤖

Copy link
Contributor

github-actions bot commented Oct 8, 2024

API Changes

--- prev.txt	2025-01-20 16:17:51.143769342 +0000
+++ current.txt	2025-01-20 16:17:46.642682387 +0000
@@ -4904,6 +4904,17 @@
 
 TYPES
 
+type AccessLogsConfig struct {
+	// Enabled controls enabling the transaction logs. Default: false.
+	Enabled bool `json:"enabled"`
+
+	// Template defaults to empty which prints the default log.
+	// Set this value to determine which fields will be printed in the access log.
+	// Example: ["..."].
+	Template []string `json:"template"`
+}
+    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"`
@@ -5356,6 +5367,10 @@
 	// If not set or left empty, it will default to `standard`.
 	LogFormat string `json:"log_format"`
 
+	// AccessLogs configures the output for access logs.
+	// If not configured, the access log is disabled.
+	AccessLogs AccessLogsConfig `json:"access_logs"`
+
 	// Section for configuring OpenTracing support
 	// Deprecated: use OpenTelemetry instead.
 	Tracer Tracer `json:"tracing"`

Copy link
Contributor

github-actions bot commented Oct 8, 2024

Failed to generate code suggestions for PR

hashAlgorithm = DefaultHashAlgorithm
}

jsonToken := fmt.Sprintf(`{"org":"%s","id":"%s","h":"%s"}`, orgID, keyID, hashAlgorithm)

Check failure

Code scanning / CodeQL

Potentially unsafe quoting Critical

If this
JSON value
contains a double quote, it could break out of the enclosing quotes.
If this
JSON value
contains a double quote, it could break out of the enclosing quotes.
If this
JSON value
contains a double quote, it could break out of the enclosing quotes.

Copilot Autofix AI 13 days ago

To fix the problem, we need to ensure that any user-provided data embedded in the JSON string is properly escaped. This can be achieved by using a JSON library to construct the JSON string instead of manually formatting it. This approach ensures that all special characters are correctly escaped.

  • Replace the manual JSON string construction with a call to json.Marshal to safely encode the data.
  • Update the GenerateToken function to use json.Marshal for creating the JSON token.
Suggested changeset 1
internal/crypto/token.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/internal/crypto/token.go b/internal/crypto/token.go
--- a/internal/crypto/token.go
+++ b/internal/crypto/token.go
@@ -35,4 +35,12 @@
 
-		jsonToken := fmt.Sprintf(`{"org":"%s","id":"%s","h":"%s"}`, orgID, keyID, hashAlgorithm)
-		return base64.StdEncoding.EncodeToString([]byte(jsonToken)), err
+		tokenData := map[string]string{
+			"org": orgID,
+			"id":  keyID,
+			"h":   hashAlgorithm,
+		}
+		jsonToken, err := json.Marshal(tokenData)
+		if err != nil {
+			return "", err
+		}
+		return base64.StdEncoding.EncodeToString(jsonToken), nil
 	}
EOF
@@ -35,4 +35,12 @@

jsonToken := fmt.Sprintf(`{"org":"%s","id":"%s","h":"%s"}`, orgID, keyID, hashAlgorithm)
return base64.StdEncoding.EncodeToString([]byte(jsonToken)), err
tokenData := map[string]string{
"org": orgID,
"id": keyID,
"h": hashAlgorithm,
}
jsonToken, err := json.Marshal(tokenData)
if err != nil {
return "", err
}
return base64.StdEncoding.EncodeToString(jsonToken), nil
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)
7.1% Duplication on New Code (required ≤ 3%)
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

config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
config/config.go Outdated Show resolved Hide resolved
@LLe27 LLe27 requested review from a team as code owners January 7, 2025 15:11
Copy link
Contributor

github-actions bot commented Jan 7, 2025

Swagger Changes

     _        __  __
   _| |_   _ / _|/ _|  between swagger-prev.yml
 / _' | | | | |_| |_       and swagger-current.yml
 \__,_|\__, |_| |_|   returned no differences
| (_| | |_| |  _|  _|

@LLe27 LLe27 force-pushed the revert-6524-revert-6354-TT-2539/transaction_logs branch from b29445d to 763dcc0 Compare January 10, 2025 15:40
gateway/middleware.go Outdated Show resolved Hide resolved
@titpetric titpetric force-pushed the revert-6524-revert-6354-TT-2539/transaction_logs branch from 647de05 to 9497947 Compare January 20, 2025 11:04
titpetric added a commit that referenced this pull request Jan 20, 2025
…liases (#6838)

### **User description**
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-2539"
title="TT-2539" target="_blank">TT-2539</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
      <td>Transaction/Access Logs</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Story"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10315?size=medium"
/>
        Story
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Code Review</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
<td><a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20A%20ORDER%20BY%20created%20DESC"
title="A">A</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20America's%20ORDER%20BY%20created%20DESC"
title="America's">America's</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20CSE%20ORDER%20BY%20created%20DESC"
title="CSE">CSE</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20Gold%20ORDER%20BY%20created%20DESC"
title="Gold">Gold</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC"
title="QA_Fail">QA_Fail</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20customer_request%20ORDER%20BY%20created%20DESC"
title="customer_request">customer_request</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20innersource%20ORDER%20BY%20created%20DESC"
title="innersource">innersource</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC"
title="jira_escalated">jira_escalated</a></td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

This is a prerequisite to implementing access logs, rebasing #6616


___

### **PR Type**
Enhancement


___

### **Description**
- Refactored hash and token-related APIs into a new `internal/crypto`
package.

- Introduced modularized functions for hashing and token generation.

- Removed redundant code from `storage` package and replaced with
references to `crypto`.

- Improved maintainability and modularity of cryptographic operations.


___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>hash.go</strong><dd><code>Introduced hash-related
utilities in `internal/crypto`</code>&nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

internal/crypto/hash.go

<li>Added a new file for hash-related functions.<br> <li> Implemented
<code>hashFunction</code> to support multiple algorithms.<br> <li>
Created <code>HashStr</code> and <code>HashKey</code> for string
hashing.<br> <li> Added constants for hash algorithm identifiers.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-fd1c33ede81b9c5740cabc411ea8e4ce122cf642382b699114dfddcc49ea84d6">+60/-0</a>&nbsp;
&nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>token.go</strong><dd><code>Introduced token-related
utilities in `internal/crypto`</code>&nbsp; &nbsp; </dd></summary>
<hr>

internal/crypto/token.go

<li>Added a new file for token-related functions.<br> <li> Implemented
<code>GenerateToken</code> for token creation with optional hashing.<br>
<li> Added functions to extract token metadata (e.g.,
<code>TokenHashAlgo</code>, <br><code>TokenID</code>,
<code>TokenOrg</code>).<br> <li> Improved handling of legacy and
JSON-based tokens.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-25b0099bc38076a27697918a7d82178f3f031a5abb58ae30c70c22d7332ee918">+91/-0</a>&nbsp;
&nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Refactored `storage` to
use `internal/crypto`</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/storage.go

<li>Removed hash and token-related functions from
<code>storage</code>.<br> <li> Delegated cryptographic operations to
<code>internal/crypto</code>.<br> <li> Cleaned up unused imports and
constants.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+0/-125</a>&nbsp;
</td>

</tr>
</table></td></tr><tr><td><strong>Miscellaneous</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Updated mock storage
handler file</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; </dd></summary>
<hr>

storage/mock/storage.go

<li>Regenerated mock file for storage handler.<br> <li> Removed
unnecessary blank line.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-0e75f439d0385d9272ea3afa9fc465dcae08554f19ff821e0743ad096325df40">+0/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

Co-authored-by: Tit Petric <[email protected]>
titpetric and others added 3 commits January 20, 2025 10:34
…liases (#6838)

### **User description**
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-2539"
title="TT-2539" target="_blank">TT-2539</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
      <td>Transaction/Access Logs</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Story"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10315?size=medium"
/>
        Story
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Code Review</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
<td><a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20A%20ORDER%20BY%20created%20DESC"
title="A">A</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20America's%20ORDER%20BY%20created%20DESC"
title="America's">America's</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20CSE%20ORDER%20BY%20created%20DESC"
title="CSE">CSE</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20Gold%20ORDER%20BY%20created%20DESC"
title="Gold">Gold</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC"
title="QA_Fail">QA_Fail</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20customer_request%20ORDER%20BY%20created%20DESC"
title="customer_request">customer_request</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20innersource%20ORDER%20BY%20created%20DESC"
title="innersource">innersource</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC"
title="jira_escalated">jira_escalated</a></td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

This is a prerequisite to implementing access logs, rebasing #6616


___

### **PR Type**
Enhancement


___

### **Description**
- Refactored hash and token-related APIs into a new `internal/crypto`
package.

- Introduced modularized functions for hashing and token generation.

- Removed redundant code from `storage` package and replaced with
references to `crypto`.

- Improved maintainability and modularity of cryptographic operations.


___



### **Changes walkthrough** 📝
<table><thead><tr><th></th><th align="left">Relevant
files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>hash.go</strong><dd><code>Introduced hash-related
utilities in `internal/crypto`</code>&nbsp; &nbsp; &nbsp;
</dd></summary>
<hr>

internal/crypto/hash.go

<li>Added a new file for hash-related functions.<br> <li> Implemented
<code>hashFunction</code> to support multiple algorithms.<br> <li>
Created <code>HashStr</code> and <code>HashKey</code> for string
hashing.<br> <li> Added constants for hash algorithm identifiers.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-fd1c33ede81b9c5740cabc411ea8e4ce122cf642382b699114dfddcc49ea84d6">+60/-0</a>&nbsp;
&nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>token.go</strong><dd><code>Introduced token-related
utilities in `internal/crypto`</code>&nbsp; &nbsp; </dd></summary>
<hr>

internal/crypto/token.go

<li>Added a new file for token-related functions.<br> <li> Implemented
<code>GenerateToken</code> for token creation with optional hashing.<br>
<li> Added functions to extract token metadata (e.g.,
<code>TokenHashAlgo</code>, <br><code>TokenID</code>,
<code>TokenOrg</code>).<br> <li> Improved handling of legacy and
JSON-based tokens.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-25b0099bc38076a27697918a7d82178f3f031a5abb58ae30c70c22d7332ee918">+91/-0</a>&nbsp;
&nbsp; </td>

</tr>

<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Refactored `storage` to
use `internal/crypto`</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;
&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary>
<hr>

storage/storage.go

<li>Removed hash and token-related functions from
<code>storage</code>.<br> <li> Delegated cryptographic operations to
<code>internal/crypto</code>.<br> <li> Cleaned up unused imports and
constants.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+0/-125</a>&nbsp;
</td>

</tr>
</table></td></tr><tr><td><strong>Miscellaneous</strong></td><td><table>
<tr>
  <td>
    <details>
<summary><strong>storage.go</strong><dd><code>Updated mock storage
handler file</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; </dd></summary>
<hr>

storage/mock/storage.go

<li>Regenerated mock file for storage handler.<br> <li> Removed
unnecessary blank line.


</details>


  </td>
<td><a
href="https://github.com/TykTechnologies/tyk/pull/6838/files#diff-0e75f439d0385d9272ea3afa9fc465dcae08554f19ff821e0743ad096325df40">+0/-1</a>&nbsp;
&nbsp; &nbsp; </td>

</tr>
</table></td></tr></tr></tbody></table>

___

> 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull
request to receive relevant information

Co-authored-by: Tit Petric <[email protected]>

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}} ./...
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- go test {{.testArgs}} -count=1 -cover -coverprofile=rate.cov {{.coverpkg}} ./...
- go test {{.testArgs}} -count=1 -cover -coverprofile=rate.cov -coverpkg={{.coverpkg}} ./...

run it

@@ -9,6 +9,8 @@ import (
"strings"
"testing"

"github.com/TykTechnologies/tyk/config"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove whitespace.


// Template defaults to empty which prints the default log.
// Set this value to determine which fields will be printed in the access log.
// Example: ["..."].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand examples with possible values to set. See example:
https://github.com/TykTechnologies/tyk/blob/master/apidef/oas/authentication.go#L77-L84

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.

3 participants