Hi Martin, Thanks very much for your feedback. The main problem I had with the current framework is the fact that the LogSubject objects seem to have knowledge about broker hierarchy (eg the SubscriptionLogSubject instantiates a QueueLogSubject, also the ConnectionLogSubject takes a AMQProtocolSession which is pre 0-10, etc) - this makes those LogSubject(s) hard to re-use and things can get even harder if 1-0 comes with a different Model Object hierarchy. Given the LogSubject role is to provide a subject (ultimately a label) to the business message pertaining to a broker specific Object state I thought that the object itself is in the best position to have all the information needed to generate the log subject and therefore the logging framework can consist of a GenericLogActor which would take something like: (LogSubject) Object and format it using the selected method from a class generated from o.a.q.server.logging.messages.*_logmessages.properties (where the formatting takes place). So our Model Object encapsulates all the info needed to log itself up. This allows decoupling of the logging framework from the need to know anything about the objects it logs, it only needs to call the toLogString method defined in the LogSubject interface. It could also log anything implementing a LogSubject interface via the same GenericActor. Otherwise we might need to create new Actors and LogSubject's to fit any new Model Object which doesn't seem right to me. As of now, with the proposed patches, the 0-10 operational logging works independent of the pre 0-10 one and I think it is simpler and more flexible. This is why Qpid-2835 seems inconsistent with the 0-8/0-9, as it is designed to sit alongside the current framework. If agreed and within the planned October release scope, we can refactor the current implementation also, effectively reducing the logging framework to a LogSubject interface, a GenericActor and o.a.q.server.logging.messages.* classes, still obeying the schema you highlighted:
LogActors <-> Logging Framework LogActors <-> Model Objects but we might as well leave it as it is. The move to slf4j is something very good to have, but it requires major changes as you know. Once we have it, we can hook it at the o.a.q.server.logging.rawloggers level. But to achieve that, we need to give up the log4j.xml file watching, the org.apache.log4j packages (eg the QpidCompositeRollingAppender), we have jmx console issues to take care of, couple of hardcoded places to look at and about 94 straight changes, not to mention the testing framework as well. All in all, not a trivial move but doable nevertheless. I would be very interested to hear thoughts on the above from anyone on the list. Thank you, Sorin On Wed, Sep 1, 2010 at 7:12 PM, Martin Ritchie <ritch...@apache.org> wrote: > Hi Sorin, > I've been watching your changes wrt the Operational Logging apologies > for not replying to your thread over the weekend but I wasn't at a > computer. > > I think the addition of the toLogString() is the wrong approach here. > The responsibility for the logging format should not lie with the > individual model objects. > > From our discussion on QPID-2780 this doesn't seem like the same > approach you are now taking, what changed? > > The change suggested by the patch on QPID-2835 will introduce > inconsistency in the operation logging. Something that its > introduction was designed to reduce. > > The LogActors are that point where the consistency is gained. > By locating the LogFormat and the string builder in one location the > LogActor has full responsibility of formatting log messages. > > The approach that I would have taken here is to expose the required > common methods for logging on our Session and Connection models. This > would then mean that we would have two clean interfaces boundaries: > LogActors <-> Logging Framework > LogActors <-> Model Objects > > This would mean that the desire to move to slf4j would be a matter of > adjusting the Logging Framework and the adding consistent 1-0 logging > would be simply a matter of implementing the model interfaces. > > The only code that I would expect to see added to the 0-10 Connection > code paths is the introduction of Logging commands. The 0-10 Model > Objects should have nothing to do with formatting logging statements. > > Cheers > Martin > > On 31 August 2010 12:18, Sorin Suciu (JIRA) > <qpid-...@incubator.apache.org> wrote: >> >> [ >> https://issues.apache.org/jira/browse/QPID-2835?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel >> ] >> >> Sorin Suciu updated QPID-2835: >> ------------------------------ >> >> Attachment: qpid-2835.patch >> >>> Implement connections (CON) operational logging on 0-10 >>> ------------------------------------------------------- >>> >>> Key: QPID-2835 >>> URL: https://issues.apache.org/jira/browse/QPID-2835 >>> Project: Qpid >>> Issue Type: Improvement >>> Components: Java Broker >>> Affects Versions: 0.7 >>> Reporter: Sorin Suciu >>> Priority: Minor >>> Fix For: 0.7 >>> >>> Attachments: qpid-2835.patch >>> >>> >>> This is part of the Qpid-2801, dealing with connection operational logging >>> on 0-10 code path. >> >> -- >> This message is automatically generated by JIRA. >> - >> You can reply to this email to add a comment to the issue online. >> >> >> --------------------------------------------------------------------- >> Apache Qpid - AMQP Messaging Implementation >> Project: http://qpid.apache.org >> Use/Interact: mailto:dev-subscr...@qpid.apache.org >> >> > > > > -- > Martin Ritchie > > --------------------------------------------------------------------- > Apache Qpid - AMQP Messaging Implementation > Project: http://qpid.apache.org > Use/Interact: mailto:dev-subscr...@qpid.apache.org > > -- Sorin S --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
