Skip to content

Fixing bug of missing access logs#561

Merged
ameowlia merged 2 commits into
cloudfoundry:developfrom
sap-contributions:CFN-7640-investigating-missing-logs
May 27, 2026
Merged

Fixing bug of missing access logs#561
ameowlia merged 2 commits into
cloudfoundry:developfrom
sap-contributions:CFN-7640-investigating-missing-logs

Conversation

@brmlad
Copy link
Copy Markdown
Contributor

@brmlad brmlad commented May 4, 2026

Summary

We propose introducing new logic in Gorouter so every request now produces an access log entry, regardless of whether the client disconnects mid-stream. The fix ensures the logging step always runs, even when a client abort occurs during response streaming. Requests affected by a client disconnect are recorded with the state captured at the time of the abort, including status code, bytes sent, and timings, and are marked with an error indicating the client closed the connection during streaming.

Resolves: #564

Backward Compatibility

Breaking Change? No

Note on AI usage

Parts of this code and tests were developed with assistance from Claude Code (claude-sonnet-4-6).

@brmlad brmlad requested a review from a team as a code owner May 4, 2026 09:28
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 4, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

Co-authored-by: Tamara Boehm <tamara.boehm@sap.com>
@brmlad brmlad force-pushed the CFN-7640-investigating-missing-logs branch from 706ad4d to cb4853f Compare May 26, 2026 11:04
})
})

Context("when the client disconnects during response streaming", func() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new test only covers one branch of this condition:

if alr.RouterError == "" && proxyWriter.WriteError() != nil {
    alr.RouterError = utils.ConnectionCloseDuringStreamingErrMsg
}

Could you also add a test for the case where both an existing RouterError is set on the response and a write error occurred?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implemented a new test when RouterError exists on the response header, and we do not want to overwrite it.

@ameowlia ameowlia self-requested a review May 27, 2026 14:06
@github-project-automation github-project-automation Bot moved this from Inbox to Pending Merge | Prioritized in Application Runtime Platform Working Group May 27, 2026
@ameowlia ameowlia merged commit 28b7284 into cloudfoundry:develop May 27, 2026
8 checks passed
@github-project-automation github-project-automation Bot moved this from Pending Merge | Prioritized to Done in Application Runtime Platform Working Group May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

Fix missing Gorouter access logs when a client disconnects mid-stream.

3 participants