So, my main comment would be that I think the Factories should not be depending on the MessageLoggerSpi as you've defined it, but instead purely on EngineLogger. The MessageLogger stuff is a convenience but I don't think it should be mandatory to use it.
My other comment would be that I don't think ProtonCategory should be defining the "qualified name" I think that would the specific to the implementation of the logging. -- Rob On 25 June 2013 13:28, Phil Harvey <[email protected]> wrote: > I've created a skeleton Java implementation of the Proton logging design > and attached it as a patch to > https://issues.apache.org/jira/browse/PROTON-343. > > I think the next steps are: > - Gather comments from folks about the design. > - Sketch out the corresponding proton-c and proton-jni code. I'd > appreciate assistance from someone with more proton-c familiarity for this. > > Please let me know your thoughts. > > Thanks, > Phil > > > On 5 June 2013 15:27, Phil Harvey <[email protected]> wrote: > > > An interesting discussion about logging has emerged from the mailing > > thread "AMQP 1.0 JMS client - supplementary coding standards". I'm > > starting a new thread for this specific topic and am including the proton > > list. > > > > To recap, Rob, Rajith, Rafi and Gordon have expressed a desire for Proton > > and the new JMS client to use a custom logging facade, rather than > directly > > calling log4j, slf4j etc. The Proton logging facade would work > > consistently across proton-c and proton-j. > > > > I think the case for adopting this approach is overwhelming, but am > > interested in views on the best implementation. > > > > > > *=== Proton ===* > > * > > * > > I added a diagram to the wiki illustrating how this might work for > > proton-j. It's not finished, but I thought it useful to share it early > to > > stimulate discussion. Hopefully the implied proton-c equivalent is > fairly > > obvious. > > > > https://cwiki.apache.org/confluence/display/qpid/Proton+Logging > > > > I'm not sure what would go into ProtonOperationalLogger at the moment > > (Rob/Rafi may know), but want to leave the door open to separating > > Proton-specific methods from general purpose log(Level, String) kind of > > stuff. It does at least give us a place to define the behaviour of the > > "public logging API" that Rob referred to, and which would behave the > same > > as its proton-c counterpart. > > > > To me, the Logger interface in the diagram looks very similar to the Qpid > > Java Broker's RootMessageLogger. Proton *may* use it directly for debug > > logging. > > > > > > *=== JMS Client ===* > > * > > * > > Turning to the JMS client, my initial preference would be to create > > interfaces JmsOperationalLogger and JmsLogger corresponding to the Proton > > ones. The JMS Client would pass to Proton a ProtonLogger implementation > > that simply wraps its JmsLogger. > > > > Alternatively we could create a Logger interface in a central sub-project > > and use it in both Proton and the JMS Client, but I suspect that will > > involve more re-jigging of our project structure than we currently have > > appetite for. > > > > Comments/criticisms etc welcomed. I'm especially interested in whether > > there are proton-c-specific factors that would significantly affect our > > implementation. > > > > > > Phil > > >
