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 a timing for response body times #3371

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

deweerdt
Copy link
Member

This is useful for comparing HTTP/2 and HTTP/3 performances since headers are subject to client prioritization in the latter but not in the former. That makes the existing response_time not as useful to reflect server performance

This is useful for comparing HTTP/2 and HTTP/3 performances since
headers are subject to client prioritization in the latter but not in
the former. That makes the existing `response_time` not as useful to
reflect server performance
@kazuho
Copy link
Member

kazuho commented Mar 29, 2024

This is useful for comparing HTTP/2 and HTTP/3 performances since headers are subject to client prioritization in the latter but not in the former.

I think the code might fine, but I'm not sure if it matches the rationale being described.

The PR sets response_body_start_at in the do_send callback, which means that response_body_time will be measuring the duration between when the first byte of the response body was provided by the generator. That is a metric representing the backend behavior, and therefore it is unlikely that this metric would reflect the differences between the frontend protocols.

If we want to observe when the headers or the first chunk of body is written to the TCP or QUIC stack, I think we need to add code to different locations.

For the first byte of the body, the moment of start would be when:

  • H2: emit_writereq_of_openref callback is invoked the first time for each stream, or
  • H3: on_send_emit callback that contains something other than headers (we might need to add a flag indicating that to the sendvec) is invoked.

Regarding the monent of ending, I think timestamps.response_end_at correctly reflects when the last chunk is sent (for the first time, in case of QUIC).

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.

None yet

2 participants