[UNDERTOW-2372] Add request size and response size exchange attribute…#1579
[UNDERTOW-2372] Add request size and response size exchange attribute…#1579xjusko wants to merge 1 commit intoundertow-io:mainfrom
Conversation
|
This seems like a bit of duplicate of #1536 |
baranowb
added
duplicate
| } | ||
| requestSize += calculateRequestLineSize(exchange); | ||
| requestSize += exchange.getRequestHeaders().getHeadersBytes(); | ||
| // add 2 bytes for CRLF, and 2 bytes for ": " between header name and value |
There was a problem hiding this comment.
Ive already talked with Marek. This type of code needs detailed exaplanation.
|
Ive removed duplicate, as its vaguely connected to other PR, which has tad different way of getting size. |
ropalka
left a comment
ropalka
left a comment
There was a problem hiding this comment.
I'm not sure this is correct. The underlying protocol can be either AJP or HTTP 1.1 or HTTP2.
| public static final ExchangeAttribute INSTANCE = new RequestSizeAttribute(); | ||
|
|
||
| @Override | ||
| public String readAttribute(HttpServerExchange exchange) { |
There was a problem hiding this comment.
What if HTTP 2 protocol is in use? The calculations here expect HTTP 1.1 is being used.
| public static final ExchangeAttribute INSTANCE = new ResponseSizeAttribute(); | ||
|
|
||
| @Override | ||
| public String readAttribute(HttpServerExchange exchange) { |
There was a problem hiding this comment.
What if HTTP 2 protocol is in use? The calculations here expect HTTP 1.1 is being used.
There was a problem hiding this comment.
good point, but AFAIR we cant account for, for instance, header compression? Also, even for H1 with compression this will be off?
@xjusko ^^ ?
…s(sizes include headers).
baranowb
left a comment
baranowb
left a comment
There was a problem hiding this comment.
Im going to approve, lets see what community bake feedback will be as to accuracy.
ropalka
left a comment
ropalka
left a comment
There was a problem hiding this comment.
My concerns still apply :(
fl4via
added
the
pending question
fl4via
added
waiting PR update
5 participants
…s(sizes include headers).
https://issues.redhat.com/browse/UNDERTOW-2372