Skip to content

Conversation

itsibitzi
Copy link
Contributor

Motivation

While testing the http/2 rollout I noticed that there was a up-tick in HTTP/2 requests coming in but the client send metrics were all saying HTTP/1.1.

It turns out this is because we were manually reconstructing a http::Response in the twirp client, without propagating the version.

Implementation

Copied across the version like we do with the status etc.

@itsibitzi itsibitzi requested a review from a team as a code owner August 14, 2025 14:29
@Copilot Copilot AI review requested due to automatic review settings August 14, 2025 14:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the HTTP version was not being preserved when reconstructing HTTP responses in the Twirp client, causing metrics to incorrectly report all requests as HTTP/1.1 instead of their actual version (e.g., HTTP/2).

  • Extract the HTTP version from the original response before consuming it
  • Set the version on the reconstructed response object alongside existing headers and extensions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@itsibitzi itsibitzi added this pull request to the merge queue Aug 14, 2025
Merged via the queue into main with commit d58d30f Aug 14, 2025
5 checks passed
@itsibitzi itsibitzi deleted the sc-20250814-http-version-response branch August 14, 2025 14:36
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.

2 participants