You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Oleg Kalnichevski <o....@dplanet.ch> on 2003/02/24 14:02:53 UTC
[PATCH] Wire logging revised (attempt 2)
I hope it works this time around
Oleg
On Thu, 2003-02-13 at 07:30, Jeffrey Dever wrote:
> Haven't had time to review all of this, but I am a bit concerned over
> any performance issues and buffering. The wirelog is a good thing, but
> there are other ways of getting a request/response log. I wouldnot like
> to see it excessively effect performance or resident set size, or add
> too much complexity.
>
> Also I find that the wirelog output conforms to the 10%/90% rule where
> 90% of its troubleshooting value comes from 10% of its output, namely
> the headers and request/response status lines. The applications I have
> written based on HttpClient would never enable the wirelog because the
> output was too large (and could contain offensive binary data), so I
> would just never enable it and use the applications logging system and
> my own methods to write out the headers. This should be a better
> service provided by HttpClient.
>
> We could just read/buffer/process/log headers in one shot in one logger,
> and handle the body logging seperately. The header logs might be part
> of the normal operation of some applications where the body log would
> only be used for headscratching debugging. That might help a little bit
> with the seperation of concerns that we are grappling here.
>
> Jandalf.
>
>
> Michael Becke wrote:
>
> > Well after looking at this further it seems one problem with this is
> > that bytes get written one at a time to the input WireLog. I made a
> > few changes in HttpConnection and WireLogInputStream that pose a
> > possible solution. This version of the InputStream buffers content
> > until a "\r\n" is read or until a certain number of bytes are read.
> > This way wire log will always work even if content is read using the
> > socket input stream. Take a look. These are just some other ideas.
> >
> > Mike
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
Re: [PATCH] Wire logging revised (attempt 2)
Posted by Jeffrey Dever <js...@sympatico.ca>.
>
>
>>- certainly would prefer Wire, WireLogInputStream, WireLogOutputStream
>>to be package access, not public
>>
>>
>
>So do I. I just find it illogical that HttpMethodBase and all classes
>derived from it reside in different packages. That causes the problem in
>the first place. Visibility of Wire, WireLogInputStream,
>WireLogOutputStream classes should have been restricted to
>org.apache.commons.httpclient.methods package in my opinion, which
>should have included HttpMethodBase.
>
Sounds like a 3.0 feature ...
Re: [PATCH] Wire logging revised (attempt 3)
Posted by Jeffrey Dever <js...@sympatico.ca>.
I think its justified too. The copywrite date in any new files should
just be 2003. Odi's name might have been munged again. Other than
that, its cool. Might need come updates to the xdocs/logging.xml
logging guide?
Oleg Kalnichevski wrote:
>Jandalf
>
>I have tried to incorporate your suggestions into the new patch. I left
>ugly cast unchanged, though. I believe it's justified in this specific
>case
>
>Any other comments, suggestions, objections?
>
>
>
[PATCH] Wire logging revised (attempt 3)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
Jandalf
I have tried to incorporate your suggestions into the new patch. I left
ugly cast unchanged, though. I believe it's justified in this specific
case
Any other comments, suggestions, objections?
Cheers
Oleg
On Thu, 2003-02-27 at 14:37, Oleg Kalnichevski wrote:
> On Thu, 2003-02-27 at 06:24, Jeffrey Dever wrote:
> > Hey Odi,
> > A few comments:
> >
> > - certainly would prefer Wire, WireLogInputStream, WireLogOutputStream
> > to be package access, not public
>
> So do I. I just find it illogical that HttpMethodBase and all classes
> derived from it reside in different packages. That causes the problem in
> the first place. Visibility of Wire, WireLogInputStream,
> WireLogOutputStream classes should have been restricted to
> org.apache.commons.httpclient.methods package in my opinion, which
> should have included HttpMethodBase.
>
>
> > - like the idea of the log stream working like tee to send the ouput to
> > destination and to log
> > - Wire.wire has a cast to (char). Could cause unicode problems?
>
> I know it broke good programming practice here, which I had been
> preaching myself not that long ago. I found it justified, though, in
> this very specific case as I deem more important ability to escape CR &
> LF and other nasty characters in the wire log. I would personally rather
> prefer a hex dump of all characters outside 32-127 ASCII range
>
>
>
> > - can MultipartPost and EntityEnclosing drop reliance on log logic?
> > conn.getRequestOutputStream() could return a WireLogOutputStream which
> > would help with the first point above.
> >
>
> I believe this can be done. I regret having overlooked this possibility.
> Cheers
>
> Oleg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
Re: [PATCH] Wire logging revised (attempt 2)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
On Thu, 2003-02-27 at 06:24, Jeffrey Dever wrote:
> Hey Odi,
> A few comments:
>
> - certainly would prefer Wire, WireLogInputStream, WireLogOutputStream
> to be package access, not public
So do I. I just find it illogical that HttpMethodBase and all classes
derived from it reside in different packages. That causes the problem in
the first place. Visibility of Wire, WireLogInputStream,
WireLogOutputStream classes should have been restricted to
org.apache.commons.httpclient.methods package in my opinion, which
should have included HttpMethodBase.
> - like the idea of the log stream working like tee to send the ouput to
> destination and to log
> - Wire.wire has a cast to (char). Could cause unicode problems?
I know it broke good programming practice here, which I had been
preaching myself not that long ago. I found it justified, though, in
this very specific case as I deem more important ability to escape CR &
LF and other nasty characters in the wire log. I would personally rather
prefer a hex dump of all characters outside 32-127 ASCII range
> - can MultipartPost and EntityEnclosing drop reliance on log logic?
> conn.getRequestOutputStream() could return a WireLogOutputStream which
> would help with the first point above.
>
I believe this can be done. I regret having overlooked this possibility.
Cheers
Oleg
Re: [PATCH] Wire logging revised (attempt 2)
Posted by Jeffrey Dever <js...@sympatico.ca>.
Hey Odi,
A few comments:
- certainly would prefer Wire, WireLogInputStream, WireLogOutputStream
to be package access, not public
- like the idea of the log stream working like tee to send the ouput to
destination and to log
- Wire.wire has a cast to (char). Could cause unicode problems?
- can MultipartPost and EntityEnclosing drop reliance on log logic?
conn.getRequestOutputStream() could return a WireLogOutputStream which
would help with the first point above.
Oleg Kalnichevski wrote:
>Folks
>Any feedback on this one. Can interpret absence of responses as silent
>approval ;-)
>Cheers
>Oleg
>
>
>On Mon, 2003-02-24 at 14:02, Oleg Kalnichevski wrote:
>
>
>>I hope it works this time around
>>
>>Oleg
>>
>>
>>On Thu, 2003-02-13 at 07:30, Jeffrey Dever wrote:
>>
>>
>>>Haven't had time to review all of this, but I am a bit concerned over
>>>any performance issues and buffering. The wirelog is a good thing, but
>>>there are other ways of getting a request/response log. I wouldnot like
>>>to see it excessively effect performance or resident set size, or add
>>>too much complexity.
>>>
>>>Also I find that the wirelog output conforms to the 10%/90% rule where
>>>90% of its troubleshooting value comes from 10% of its output, namely
>>>the headers and request/response status lines. The applications I have
>>>written based on HttpClient would never enable the wirelog because the
>>>output was too large (and could contain offensive binary data), so I
>>>would just never enable it and use the applications logging system and
>>>my own methods to write out the headers. This should be a better
>>>service provided by HttpClient.
>>>
>>>We could just read/buffer/process/log headers in one shot in one logger,
>>>and handle the body logging seperately. The header logs might be part
>>>of the normal operation of some applications where the body log would
>>>only be used for headscratching debugging. That might help a little bit
>>>with the seperation of concerns that we are grappling here.
>>>
>>>Jandalf.
>>>
>>>
>>>Michael Becke wrote:
>>>
>>>
>>>
>>>>Well after looking at this further it seems one problem with this is
>>>>that bytes get written one at a time to the input WireLog. I made a
>>>>few changes in HttpConnection and WireLogInputStream that pose a
>>>>possible solution. This version of the InputStream buffers content
>>>>until a "\r\n" is read or until a certain number of bytes are read.
>>>>This way wire log will always work even if content is read using the
>>>>socket input stream. Take a look. These are just some other ideas.
>>>>
>>>>Mike
>>>>
>>>>
>>>>
>>>
>>>---------------------------------------------------------------------
>>>To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
>>>For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>>>
>>>
>>>
>>______________________________________________________________________
>>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
>>For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>>
>>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
>
>
>
>
Re: [PATCH] Wire logging revised (attempt 2)
Posted by Oleg Kalnichevski <o....@dplanet.ch>.
Folks
Any feedback on this one. Can interpret absence of responses as silent
approval ;-)
Cheers
Oleg
On Mon, 2003-02-24 at 14:02, Oleg Kalnichevski wrote:
> I hope it works this time around
>
> Oleg
>
>
> On Thu, 2003-02-13 at 07:30, Jeffrey Dever wrote:
> > Haven't had time to review all of this, but I am a bit concerned over
> > any performance issues and buffering. The wirelog is a good thing, but
> > there are other ways of getting a request/response log. I wouldnot like
> > to see it excessively effect performance or resident set size, or add
> > too much complexity.
> >
> > Also I find that the wirelog output conforms to the 10%/90% rule where
> > 90% of its troubleshooting value comes from 10% of its output, namely
> > the headers and request/response status lines. The applications I have
> > written based on HttpClient would never enable the wirelog because the
> > output was too large (and could contain offensive binary data), so I
> > would just never enable it and use the applications logging system and
> > my own methods to write out the headers. This should be a better
> > service provided by HttpClient.
> >
> > We could just read/buffer/process/log headers in one shot in one logger,
> > and handle the body logging seperately. The header logs might be part
> > of the normal operation of some applications where the body log would
> > only be used for headscratching debugging. That might help a little bit
> > with the seperation of concerns that we are grappling here.
> >
> > Jandalf.
> >
> >
> > Michael Becke wrote:
> >
> > > Well after looking at this further it seems one problem with this is
> > > that bytes get written one at a time to the input WireLog. I made a
> > > few changes in HttpConnection and WireLogInputStream that pose a
> > > possible solution. This version of the InputStream buffers content
> > > until a "\r\n" is read or until a certain number of bytes are read.
> > > This way wire log will always work even if content is read using the
> > > socket input stream. Take a look. These are just some other ideas.
> > >
> > > Mike
> > >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org
> >
>
> ______________________________________________________________________
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-httpclient-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-httpclient-dev-help@jakarta.apache.org