Skip to content

[TT-4023] Track 404 logs not showing in listening port when changing control api port#7976

Open
MaciekMis wants to merge 3 commits intomasterfrom
TT-4023-track-404-logs-not-showing-in-listening-port-when-changing-control-api-port
Open

[TT-4023] Track 404 logs not showing in listening port when changing control api port#7976
MaciekMis wants to merge 3 commits intomasterfrom
TT-4023-track-404-logs-not-showing-in-listening-port-when-changing-control-api-port

Conversation

@MaciekMis
Copy link
Copy Markdown
Contributor

Description

Related Issue

Motivation and Context

How This Has Been Tested

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

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 8, 2026

This PR addresses a bug where 404 errors were not logged on the main gateway listening port if the control API was configured to run on a separate, dedicated port. The issue stemmed from the NotFoundHandler being set only on the router for the control API, leaving other listeners without this crucial error logging mechanism.

The fix centralizes the 404 handling logic. The responsibility for setting the NotFoundHandler has been moved from the specific loadApps function to the generic proxyMux.setRouter method. This ensures that any router configured through this method—including the main listener and the control API listener—will consistently have the 404 handler attached. To support this, the proxyMux struct is now initialized with the track404Logs configuration setting, allowing the handler to respect this flag when deciding whether to log a 404 event.

Files Changed Analysis

  • gateway/api_loader.go: The line that previously set the NotFoundHandler for the control API's router has been removed, delegating this responsibility to a more centralized function.
  • gateway/proxy_muxer.go: The setRouter function now assigns the handle404 method to the NotFoundHandler of every router it configures, ensuring uniform behavior.
  • gateway/server.go: The proxyMux struct is now initialized with the track404Logs configuration value in both NewGateway and startServer, ensuring the handler has access to this setting wherever it's used.

Architecture & Impact Assessment

  • What this PR accomplishes: It guarantees consistent 404 logging behavior across all gateway listeners, regardless of whether the control API runs on the main port or a separate one.
  • Key technical changes introduced: The logic for setting the 404 handler has been centralized into the proxyMux.setRouter function. The proxyMux struct now holds the track404Logs state, which is passed during its initialization.
  • Affected system components: This change affects the gateway's core HTTP server setup, request routing, and error handling logic, primarily within the gateway package.
sequenceDiagram
    participant APILoader
    participant ProxyMuxer

    rect rgb(240, 240, 255)
        note over APILoader, ProxyMuxer: Old Behavior
        APILoader->>APILoader: Create router for Control API
        APILoader->>APILoader: Set NotFoundHandler on this router ONLY
        note right of APILoader: Main listener's router has no 404 handler.
    end

    rect rgb(240, 240, 255)
        note over APILoader, ProxyMuxer: New Behavior
        ProxyMuxer->>ProxyMuxer: setRouter(router, ...)
        ProxyMuxer->>ProxyMuxer: Set NotFoundHandler on ANY given router
        note right of ProxyMuxer: All listeners (Control API, Main Port, etc.)<br/>get the same 404 handler consistently.
    end
Loading

Scope Discovery & Context Expansion

This change impacts the setup of all HTTP listeners managed by the proxyMux. By modifying setRouter, the PR ensures that not only the control API and the main gateway listener but also any other listeners configured through this central function will have consistent 404 logging. The proxyMux.handle404 method, now universally applied, contains the logic to check the track404Logs flag and write to the log file, which directly resolves the inconsistent behavior reported in the ticket.

Metadata
  • Review Effort: 2 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-09T17:11:49.279Z | Triggered by: pr_updated | Commit: 300e2d6

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs bot commented Apr 8, 2026

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

Architecture Issues (1)

Severity Location Issue
🟠 Error gateway/server.go:2293-2295
A new local `proxyMux` instance is created here. This is architecturally inconsistent as the `Gateway` has a main instance (`gw.proxyMux`) that should be used. This local instance is not fully initialized (e.g., the `again` field is missing compared to the instance in `NewGateway`) and any state set on it (like the `proxies` map populated by `setRouter`) is discarded when the function exits. Adding the `track404Logs` configuration here fixes the immediate bug but reinforces this flawed pattern of using temporary, partially-configured service objects.
💡 SuggestionRefactor this function to use the gateway's shared `proxyMux` instance (`gw.proxyMux`) instead of creating a new local one. This would eliminate the need for the change in this function, as `gw.proxyMux` is already correctly configured in `NewGateway` by this pull request.

✅ Performance Check Passed

No performance issues found – changes LGTM.

Quality Issues (1)

Severity Location Issue
🟡 Warning gateway/server.go:2292
The `proxyMux` initialization logic is duplicated. The `DefaultProxyMux` is initialized in `NewGateway` but appears to be unused, as a new `muxer` is created and configured in `startServer`. This new `muxer` is then used to set up the control API router, but it is a local variable and does not replace `gw.DefaultProxyMux`. This suggests either redundant initialization or a potential logic issue where the `DefaultProxyMux` should have been used or updated instead.
💡 SuggestionConsolidate the initialization of `proxyMux`. Either use the `gw.DefaultProxyMux` instance within `startServer` or remove the initialization from `NewGateway` if it's truly not used. If `startServer` is meant to configure the default muxer, it should operate on `gw.DefaultProxyMux` directly.

Powered by Visor from Probelabs

Last updated: 2026-04-09T17:11:45.545Z | Triggered by: pr_updated | Commit: 300e2d6

💡 TIP: You can chat with Visor using /visor ask <your question>

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

API Changes

no api changes detected

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

🚨 Jira Linter Failed

Commit: 300e2d6
Failed at: 2026-04-09 17:09:58 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details
failed to get Jira issue: failed to fetch Jira issue TT-4023: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

Next Steps

  • Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
  • Verify your PR title matches the branch's Jira ticket ID
  • Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 9, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant