Skip to content

Fix response size sum in metrics#99

Merged
riandyrn merged 2 commits intoriandyrn:masterfrom
bullet4791:master
Mar 30, 2026
Merged

Fix response size sum in metrics#99
riandyrn merged 2 commits intoriandyrn:masterfrom
bullet4791:master

Conversation

@bullet4791
Copy link
Copy Markdown

Fix response size metric aggregation

The http_server_response_size_bytes_sum metric was incorrectly reporting zero because writtenBytes was only incremented on the first Write call, when !rrw.written. This ignored all subsequent writes, leading to inaccurate or zero size reporting, especially for responses written in multiple chunks.

Updated the Write hook to always increment writtenBytes by the actual number of written bytes (n), not len(b), and to track the first non-zero write separately. This ensures accurate response size measurement regardless of write pattern.

Changes:

  • Fix incorrect writtenBytes accumulation
  • Use n (actual written bytes) instead of len(b)
  • Preserve written flag logic for WriteHeader tracking

@bullet4791
Copy link
Copy Markdown
Author

@riandyrn can you check this PR?

@ilhamsyahids
Copy link
Copy Markdown
Collaborator

Hello @bullet4791

Thanks for fixing this, I was able to confirm the issue as well and the change is makes sense

Before merging this, could you also add a test case that covers this scenario? Specifically, it would be good to have a test where the response is written in multiple Write calls to ensure metric correctly accumulates the total bytes written which is should be FAIL before this patch.

@bullet4791
Copy link
Copy Markdown
Author

Hello @ilhamsyahids
I've added the necessary tests. Can you check it?

Copy link
Copy Markdown
Collaborator

@ilhamsyahids ilhamsyahids left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@riandyrn riandyrn merged commit f90315d into riandyrn:master Mar 30, 2026
11 checks passed
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.

3 participants