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. > 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. Suggestions to improve this welcome, I blush for the lack of a review request. Cheers, Alan.
