On Tue, 2014-12-16 at 10:48 -0500, Andrew Stitcher wrote: > 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?
Done: commit a72202d4f4c02221c6555a9e76480d383c7398c8 Author: Alan Conway <[email protected]> Commit: Alan Conway <[email protected]> PROTON-772: New proton logging code Based on list discussion: - moved the log statement api into src/log_private - simplified the public API to 2 functions: pn_log_enable and pn_log_logger The minimal public API is required to let applications using proton control where proton log output goes, which is the point of all this. The present state of the logging achieves the following goals: - allow applications to disable or redirect ALL proton library output, nothing is forced to stderr. - leaves the previous behavior of transport tracers unaffected. - introduces as little disturbance as possible (one new env. var PN_TRACE_LOG and 2 new API functions) This is definitely not the final word on logging for proton, but a more sophisticated, unified logging system requires more discussion in the proton community. > 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. Proton has no "library init" function that is required to be called before using proton objects, and there's no "root" to the proton object tree so I had trouble figuring out where to put such a thing. I agree it would be nicer if we could find a place to attach a logging object. > 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. Agree with the sentiment but again I couldn't figure out where to put it. The transport-attached tracer API allows the app to attach arbitrary function calls to each transport, so without changing that behavior what can you centralize? I did make tracer calls with transport=0 go to the global log so that is centralized sort-of. What I've done so far is *definitely* not the last word but it is a step in that it a) stops the barfing on stderr and b) doesn't break anything else. Thanks for making me clean it up so at least there's minimal public API to change when we do come up with something more comprehensive. Cheers, Alan. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
