You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2017/06/07 08:52:12 UTC
[Bug 61164] New: Add %X option to access log for connection status
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
Bug ID: 61164
Summary: Add %X option to access log for connection status
Product: Tomcat 9
Version: unspecified
Hardware: PC
OS: Linux
Status: NEW
Severity: enhancement
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: markt@apache.org
Target Milestone: -----
In Apache HTTPD there is an option in their access log format to log
connection status: "%X"
http://httpd.apache.org/docs/2.4/mod/mod_log_config.html#formats
"%X":
Connection status when response is completed:
X = Connection aborted before the response completed.
+ = Connection may be kept alive after the response is sent.
- = Connection will be closed after the response is sent.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #8 from Zemian Deng <ze...@gmail.com> ---
I have added the isIoAllowed with action code check to the PR now. Please
review.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #1 from Zemian Deng <ze...@gmail.com> ---
Hi there,
I have created a PR here (https://github.com/apache/tomcat/pull/70) for this
enhancement. Let me know what you think.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #5 from Zemian Deng <ze...@gmail.com> ---
Mark, I see org.apache.coyote.AbstractProcessor#dispatch() starting line 205
contains error handling code that says "occurred on 'non-container' thread". Is
this where what you want to start tracking with a new req attribute?
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #3 from Zemian Deng <ze...@gmail.com> ---
Hi Mark, yes hence the PR wasn't that big. :)
I saw the comment you made on GitHub, I will take a further look later and let
you know.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #9 from Mark Thomas <ma...@apache.org> ---
Thanks for the patch. It has been applied to trunk (for 9.0.0.M26 onwards) and
8.5.x (for 8.5.20 onwards)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #2 from Mark Thomas <ma...@apache.org> ---
This is a lot simpler than I imagined. My expectation was that the various
places where errors can occur in in Http11Processor.service() would make this
tricky to implement. It looks like I was wrong. Because those errors set the
RequestDispatcher.ERROR_EXCEPTION, this actually makes implementation fairly
simple.
One aspect that I think needs a little more thought is container initiated
(rather than client initiated) aborts. The easiest way to detect these would be
to add another request attribute. Whether both get logged as 'X' or whether one
is logged as 'X' and the other as 'x' is open to discussion.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #4 from Zemian Deng <ze...@gmail.com> ---
I have pushed a new commit to the PR. Please review.
As far as adding "container initiated (rather than client initiated) aborts", I
will need to study some more before I can determine where to add the new req
attr. (do give me a hint if you already know.)
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
The updated patch looks good. Thanks. I noticed that the import order has
changed. If you could undo that it would be good but it isn't a big deal to fix
when the patch is applied.
Regarding tracking container vs client connection abort, I was looking at
AbstractProcessor#setErrorState() around line 90. Having thought about this
some more, a request attribute is a bit of a hack and I think I can see a
batter way.
What I was thinking was to also log 'X' if
AbstractProcessor#getErrorState()#isIoAllowed() returns false. The correct way
to access the Processor via the Request or Response is by defining an
ActionCode and calling Request#acton() or Response#action(). Take a look at
ActionCode#IS_ERROR and how it is used. I am thinking ActionCode#IS_IO_ALLOWED
and then call Request#action() from the access log.
I'm not sure if there is value in differentiating client and container aborts.
I'm leaning towards not differentiating for now. If we add support for '%X' now
that does not differentiate, we can always add '%x' at a later date that does
differentiate if there is demand.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 61164] Add %X option to access log for connection status
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=61164
--- Comment #7 from Zemian Deng <ze...@gmail.com> ---
Mark, thanks for all the tips. No problem, I can re-fix the imports for better
merge experience.
Yes, I agree that we should support just '%X' for now until users demands the
finer abort types, then we can add the extra '%x' pattern.
As for the extra '%X' check with ErrorState()#isIoAllowed() == false, I assume
you wanted for this patch. I can give it a try and see. Will update later.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org