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

Add Forwarded header among structured log values. #120

Open
wants to merge 2 commits into
base: feature/d8
Choose a base branch
from

Conversation

Bladedu
Copy link
Member

@Bladedu Bladedu commented Sep 16, 2024

PR Type

Enhancement


Description

  • Enhanced the structured log format in Nginx configuration to include the 'Forwarded' header
  • Added a new field 'forwarded' that captures the value of 'http_forwarded'
  • This addition improves tracking of the original client IP when requests pass through multiple proxies
  • The change allows for better analysis of request origins and proxy chains

Changes walkthrough 📝

Relevant files
Enhancement
001-structured-logs.conf
Add Forwarded header to structured logs                                   

config/conf.d/001-structured-logs.conf

  • Added 'forwarded' field to the structured log format
  • New field captures the 'http_forwarded' header value
  • +1/-1     

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

    @sparkfabrik-ai-bot
    Copy link

    PR Reviewer Guide 🔍

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

    Incorrect Variable
    The new 'forwarded' field is using an incorrect variable name. It should be '$http_forwarded' instead of 'http_forwarded'.

    @sparkfabrik-ai-bot
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct the usage of the Forwarded header variable in the log format

    The forwarded field in the log format is using a string literal "http_forwarded"
    instead of the Nginx variable $http_forwarded. This will log the literal string
    "http_forwarded" for every request instead of the actual value of the Forwarded
    header.

    config/conf.d/001-structured-logs.conf [1]

    -log_format structured '{"requestId": "$request_id", "status": $status, "request": "$request", "remoteAddr": "$remote_addr", "http_x_forwarded_for": "$http_x_forwarded_for", "proxy_add_x_forwarded_for": "$proxy_add_x_forwarded_for", "forwarded": "http_forwarded", "body_bytes_sent": $body_bytes_sent, "http_referer": "$http_referer", "http_user_agent": "$http_user_agent"}';
    +log_format structured '{"requestId": "$request_id", "status": $status, "request": "$request", "remoteAddr": "$remote_addr", "http_x_forwarded_for": "$http_x_forwarded_for", "proxy_add_x_forwarded_for": "$proxy_add_x_forwarded_for", "forwarded": "$http_forwarded", "body_bytes_sent": $body_bytes_sent, "http_referer": "$http_referer", "http_user_agent": "$http_user_agent"}';
     
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a critical error in the log format where the Forwarded header is incorrectly logged as a string literal instead of its actual value, which could lead to significant issues in request tracking and debugging.

    9

    @Bladedu Bladedu requested a review from Monska85 September 16, 2024 13:58
    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.

    1 participant