On Mon, 2014-12-15 at 11:51 -0500, Alan Conway wrote: > On Fri, 2014-12-12 at 15:09 -0500, Andrew Stitcher wrote: > > Alan recently landed a change which stops the proton library from > > writing directly to stderr/stdout - this is a good thing (IMO). > > > > However I question a couple of things: > > > > 1. This change adds to the external API and so should have been reviewed > > (again IMO). reviews.apache.org now does work with the proton git repo. > > Agreed, my bad definitely should have been reviewed. > > > > > 2. I don't think this change should be an external API at all - at least > > not initially, perhaps with some user pressure. > > Agreed again. We may want to expose some form of logging as API but this > is probably to rushed. The purpose of this work was to allow users of > the proton library to control where protons logging goes, not to provide > a general purpose logging API for developers using proton.
Woudl you like to move it or shall I? > > > 3. There is already logging code triggered by PB_TRACE_FRM, > > PN_TRACE_RAW, PN_TRACE_DRV it would make sense for this to be integrated > > with the new work. Especially as this allows you to specify the tracer > > used (using pn_transport_set_tracer()). > > It is sort-of supposed to be as far as it can be. The issue here is that > we already had a tracing mechanism *attached to the transport*. So code > that has access to a transport instance is already using it and didn't > need to be changed. Code that does _not_ have a transport instance was > just dumping on stderr (even code that was pretending to use the > transport tracing mechanism but was passing 0 for the transport pointer, > which just became a dump-to-stderr.) The new env var (PN_TRACE_LOG) was > intended to be "in the spirt" of the existing configurtion, but apply to > code points that don't have a transport and therefore need some "global > log" to dump to that does not reqiure a transport-attached tracer. The > global tracer is pretty much identical to the transport tracer except it > doesn't take a transport pointer. I think the issue of a new logging level (not that I'm against new levels) and code without access to the transport object is unconnected. I agree that we need some way to log from all parts of the code - I would tend to introduce a log object though rather than make the facility global. Also I really want there to be only one logging service in the library so the previous transport attached logger should share the tracer with the new facility. > > Suggestions to improve this welcome, I blush for the lack of a review > request. > Andrew --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
