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

Simplify ability to get Byte Counts about requests #4954

Closed
joakime opened this issue Jun 9, 2020 · 5 comments
Closed

Simplify ability to get Byte Counts about requests #4954

joakime opened this issue Jun 9, 2020 · 5 comments
Assignees
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement Stale For auto-closed stale issues and pull requests

Comments

@joakime
Copy link
Contributor

joakime commented Jun 9, 2020

Jetty version
9.4.29

Description
There are several techniques used by web projects to collect information about byte counts.

Most are based on using the Servlet API.
The most common technique being ...

  1. Create a Filter
  2. Have Filter create a HttpServletRequestWrapper and HttpServletResponseWrapper.
  3. Any stream access is wrapped (HttpServletRequest.getInputStream(), HttpServletRequest.getReader(), HttpServletResponse.getOutputStream(), HttpServletResponse.getWriter())
  4. When dispatch returns to the Filter the number of bytes that have gone through the stream is the "byte count" used for whatever purpose the developer needs.

This has a few flaw.

  • Unable to get HTTP Request Header size
  • Unable to get HTTP Response Header size
  • GzipHandler changes the "API Bytes" to "Network Bytes", which there's no Servlet API for
  • Chunked Transfer-Encoding uses more bytes on the network then the API shows
  • Complicates using the Servlet 3.1 Async I/O featureset.

Internally, the Jetty implementation is keeping track of these numbers already in the various HttpInput, HttpOutput, and HttpConnection classes.
Writing a HttpChannel.Listener for the ServerConnector is one way to utilize these numbers at the appropriate times.
However, some of the numbers have no accessors in the Jetty API.
And the nuances of the HttpChannel.Listener events and Request / Response lifecycle (recycling for example) can complicate writing such a layer against Jetty.

I propose we add a simple API to allow developers to hook into the "Request Complete" event to get the list of byte counts for every request (even those that don't go to a context).
This simple API can also be used as a demonstration on using the HttpChannel.Listener properly.

@joakime joakime added Enhancement Sponsored This issue affects a user with a commercial support agreement labels Jun 9, 2020
@joakime joakime self-assigned this Jun 9, 2020
@joakime joakime added this to Review in progress in Jetty 9.4.30 Jun 9, 2020
@joakime joakime moved this from Review in progress to In progress in Jetty 9.4.30 Jun 9, 2020
joakime added a commit that referenced this issue Jun 9, 2020
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime linked a pull request Jun 9, 2020 that will close this issue
joakime added a commit that referenced this issue Jun 9, 2020
+ More updates from quick review of concept with simone

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime moved this from In progress to To do in Jetty 9.4.30 Jun 12, 2020
@joakime joakime removed this from To do in Jetty 9.4.30 Jun 12, 2020
@joakime joakime added this to To do in Jetty 9.4.31 via automation Jun 12, 2020
@joakime joakime moved this from To do to In progress in Jetty 9.4.31 Jun 12, 2020
@joakime joakime added this to In progress in Jetty 9.4.32 Jul 22, 2020
@joakime joakime removed this from In progress in Jetty 9.4.31 Jul 22, 2020
joakime added a commit that referenced this issue Sep 14, 2020
- Minimal new API for exposing byte counts to HttpChannel.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Sep 15, 2020
- Minimal new API for exposing byte counts to HttpChannel.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Sep 15, 2020
…de-exposure

Issue #4954 - Expose header size/length from HttpParser
@joakime joakime added this to To do in Jetty 9.4.33 via automation Sep 17, 2020
@joakime joakime removed this from In progress in Jetty 9.4.32 Sep 17, 2020
@joakime joakime added this to To do in Jetty 9.4.34 via automation Oct 14, 2020
@joakime joakime removed this from To do in Jetty 9.4.33 Oct 14, 2020
@gregw gregw removed this from To do in Jetty 9.4.34 Nov 2, 2020
@stale
Copy link

stale bot commented Jun 20, 2021

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale For auto-closed stale issues and pull requests label Jun 20, 2021
@snago
Copy link

snago commented Jun 21, 2021

I would still be interested in some API for this.
Now we estimate request/response size in our application by adding up lengths of method, uri, headers, etc.
Would be nice to be able to get the correct sizes.

@stale stale bot removed the Stale For auto-closed stale issues and pull requests label Jun 21, 2021
@gregw
Copy link
Contributor

gregw commented Jun 21, 2021

@joakime I don't understand what is being suggested here? If applications have access to the jetty APIs, then we already provide listeners and API to get bytes written by connection and by request/response.
If applications don't have access to the jetty APIs, then how is adding a new listener going to help?

@github-actions
Copy link

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Jun 22, 2022
@github-actions
Copy link

This issue has been closed due to it having no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement Stale For auto-closed stale issues and pull requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants