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

[DX-1803] /transaction-logs-docs #5932

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

[DX-1803] /transaction-logs-docs #5932

wants to merge 16 commits into from

Conversation

LLe27
Copy link
Contributor

@LLe27 LLe27 commented Jan 28, 2025

User description

For internal users - Please add a Jira DX PR ticket to the subject!


DX-1803

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


Preview Link


Description


Screenshots (if appropriate)


Checklist

  • I have added a preview link to the PR description.
  • I have reviewed the suggestions made by our AI (PR Agent) and updated them accordingly (spelling errors, rephrasing, etc.)
  • I have reviewed the guidelines for contributing to this repository.
  • I have read the technical guidelines for contributing to this repository.
  • Make sure you have started your change off our latest master.
  • I labeled the PR

PR Type

Documentation


Description

  • Added documentation for enabling API request access logs in Tyk Gateway.

  • Detailed configuration options for TYK_ACCESSLOGS_TEMPLATE explained.

  • Included performance considerations for enabling access logs.

  • Updated headings format for improved readability.


Changes walkthrough 📝

Relevant files
Documentation
log-data.md
Document API request access logs and configuration             

tyk-docs/content/log-data.md

  • Added section on enabling API request access logs.
  • Detailed configuration for TYK_ACCESSLOGS_TEMPLATE.
  • Explained performance impacts of access logs.
  • Improved heading structure for clarity.
  • +33/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Performance Impact

    The addition of access logs introduces performance overhead (latency, memory usage, and allocations). Ensure that these impacts are acceptable for the intended use case and that they are clearly documented for users.

    #### Access Logs Performance Considerations
    Enabling access logs introduces some performance overhead:
    
    - **Latency:** Increases consistently by approximately 4%–13%, depending on CPU allocation and configuration.
    - **Memory Usage:** Memory consumption increases by approximately 6%–7%.
    - **Allocations:** The number of memory allocations increases by approximately 5%–6%.
    
    {{< note >}}
    **Note**  
    While the overhead of enabling access logs is noticeable, the impact is relatively modest. These findings suggest the performance trade-off may be acceptable depending on the criticality of logging to your application.
    Configuration Clarity

    The configuration for TYK_ACCESSLOGS_TEMPLATE should be validated for clarity and completeness, ensuring users understand how to properly format and use the values.

    #### Configuring Access Logs Data
    You can specify the type of data to be logged by configuring the `TYK_ACCESSLOGS_TEMPLATE` in the Tyk Gateway configuration. Below are the available values you can include:
    
    - `api_key`: Logs the obfuscated or hashed API key.
    - `client_ip`: Logs the IP address of the request.
    - `host`: Logs the host of the request.
    - `method`: Logs the HTTP method of the request.
    - `path`: Logs the path of the request.
    - `protocol`: Logs the protocol used in the request.
    - `remote_addr`: Logs the remote address of the request.
    - `upstream_address`: Logs the upstream address, including scheme, host, and path.
    - `upstream_latency`: Logs the upstream latency of the request.
    - `latency_total`: Logs the total latency of the request.
    - `user_agent`: Logs the user agent of the request.
    - `status`: Logs the response status code.
    
    To configure, set `TYK_ACCESSLOGS_TEMPLATE` with the desired values in the format: ["value1", "value2", ...].

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Emphasize secure handling of sensitive data

    Clarify that sensitive data like API keys should be obfuscated or hashed when using
    the TYK_ACCESSLOGS_TEMPLATE to avoid potential security risks.

    tyk-docs/content/log-data.md [63]

    -- `api_key`: Logs the obfuscated or hashed API key.
    +- `api_key`: Logs the obfuscated or hashed API key. Ensure sensitive data like API keys are always obfuscated or hashed to prevent exposure in logs.
    Suggestion importance[1-10]: 8

    Why: The suggestion enhances security by explicitly warning about the importance of obfuscating or hashing sensitive data like API keys, which is critical to prevent potential data exposure in logs.

    8
    General
    Warn against excessive logging configurations

    Highlight that excessive logging configurations in TYK_ACCESSLOGS_TEMPLATE can lead
    to significant performance degradation and should be used judiciously.

    tyk-docs/content/log-data.md [76]

    -To configure, set `TYK_ACCESSLOGS_TEMPLATE` with the desired values in the format: ["value1", "value2", ...].
    +To configure, set `TYK_ACCESSLOGS_TEMPLATE` with the desired values in the format: ["value1", "value2", ...]. Avoid excessive logging configurations to minimize performance degradation.
    Suggestion importance[1-10]: 7

    Why: The suggestion adds a cautionary note about the performance impact of excessive logging configurations, which is relevant and useful for users to make informed decisions about their logging setup.

    7

    Copy link

    netlify bot commented Jan 28, 2025

    PS. Pls add /docs/nightly to the end of url

    Name Link
    🔨 Latest commit 340b538
    🔍 Latest deploy log https://app.netlify.com/sites/tyk-docs/deploys/67a20d5e801bbb00081d1749
    😎 Deploy Preview https://deploy-preview-5932--tyk-docs.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @sharadregoti sharadregoti changed the title [DX-1803]/transaction-logs-docs [DX-1803] /transaction-logs-docs Jan 30, 2025
    @andyo-tyk
    Copy link
    Contributor

    In this PR we are adding a whole new capability to Tyk logging. We can’t just hide it partway down the system log page. These aren’t system logs, but are effectively a duplication of the traffic logs (“analytics”) for a different purpose.

    So, the documentation for this feature must explain:

    • what it is
    • why and when a user might want to use it
    • how to configure it
    • the implications of its use (e.g. performance, memory and storage)

    We aim to use the principle of Progressive Disclosure in the docs, so I recommend restructuring this page slightly so that it refers to the native logger functionality - the fact it can be configured to write to stderr or stdout (or whatever the options/limitations are), that it recognises different levels of verbosity and how to configure all this.

    Then we can have a section on System events - and the different messages that are generated - and a separate one on this new access log function.

    Most of the content is there, but if we want to have useful docs, we need to consider the structure and consistency of the pages, not just add new content to an existing page without checking the flow and usability of that page.

    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.

    4 participants