You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Chug Rolke <cr...@redhat.com> on 2012/07/02 23:11:19 UTC
Re: Review Request: Add log entries for correlatable broker object life
cycles
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/
-----------------------------------------------------------
(Updated July 2, 2012, 9:11 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
Changes
-------
SessionId get a connection name that can be shown when the Session is created.
The name of the new category is Model (and not Configuration).
New function in templated management Class to return map of stats.
resourceDestroy() for all management classes prints the log message. This method takes a class name to print in the Model log. [1]
Model logging more efficient when logging is disabled.
Description
-------
This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
[Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
This addresses bug QPID-4079.
https://issues.apache.org/jira/browse/QPID-4079
Diffs (updated)
-----
trunk/qpid/cpp/include/qpid/SessionId.h 1354515
trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
Diff: https://reviews.apache.org/r/5616/diff/
Testing
-------
Passes cmake test and automake make check.
Thanks,
Chug Rolke
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Exchange.cpp, line 251
> > <https://reviews.apache.org/r/5616/diff/3/?file=118895#file118895line251>
> >
> > Why does the name need to be passed in here? The management object was given the name when it was created.
This strategy needs to be backed out. A better approach is to move all the logging into the Management Event code where the proper data and context are present for a concise log generation.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Exchange.cpp, line 381
> > <https://reviews.apache.org/r/5616/diff/3/?file=118895#file118895line381>
> >
> > I notice we are not recording connection id here...
This is the constructor for Binding. Binding has exchangeRef, queueRef, and key and no reference to connection.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp, line 379
> > <https://reviews.apache.org/r/5616/diff/3/?file=118914#file118914line379>
> >
> > I don't understand this change; could you clarify?
true error
- Chug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8831
-----------------------------------------------------------
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Gordon Sim <gs...@redhat.com>.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/include/qpid/SessionId.h, line 45
> > <https://reviews.apache.org/r/5616/diff/3/?file=118881#file118881line45>
> >
> > I think adding this here is wrong. A session id does not logically include the connection id. See suggestion in SessionState changes for simple alternative.
>
> Chug Rolke wrote:
> From the XML:
> <class name="Session">
> ...
> <property name="connectionRef" type="objId" references="Connection" access="RO"/>
>
> Session stats must be aggregated to the connection over which it was created.
Right, that is a sesion though, which is different from a session id.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8831
-----------------------------------------------------------
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/include/qpid/SessionId.h, line 45
> > <https://reviews.apache.org/r/5616/diff/3/?file=118881#file118881line45>
> >
> > I think adding this here is wrong. A session id does not logically include the connection id. See suggestion in SessionState changes for simple alternative.
>From the XML:
<class name="Session">
...
<property name="connectionRef" type="objId" references="Connection" access="RO"/>
Session stats must be aggregated to the connection over which it was created.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/acl/Acl.cpp, line 297
> > <https://reviews.apache.org/r/5616/diff/3/?file=118887#file118887line297>
> >
> > Use getStatsAsMap()? (For consistency more than real performance concerns)
correct
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Broker.cpp, line 436
> > <https://reviews.apache.org/r/5616/diff/3/?file=118890#file118890line436>
> >
> > Use getStatsAsMap()?
correct
- Chug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8831
-----------------------------------------------------------
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Gordon Sim <gs...@redhat.com>.
> On July 3, 2012, 11:04 a.m., Gordon Sim wrote:
> > trunk/qpid/cpp/src/qpid/broker/Exchange.cpp, line 381
> > <https://reviews.apache.org/r/5616/diff/3/?file=118895#file118895line381>
> >
> > I notice we are not recording connection id here...
>
> Chug Rolke wrote:
> This is the constructor for Binding. Binding has exchangeRef, queueRef, and key and no reference to connection.
My question is what is the rationale for logging the connection id against the queue and the exchange being created, but not the binding (and logging the session id against subscribe/unsubscribe)?
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8831
-----------------------------------------------------------
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8831
-----------------------------------------------------------
I feel this could be cleaner if contained within management code. There are already events raised for creation/deletion of queues and exchanges (creation admittedly is muddled by the event being a declare event, but I would rather fix that anyway), subscribe/unsubscribe, bind/unbind. If for some reason those events can't be used to also issue log statements, perhaps the management object creation acan be used in a generic way as for deletion here.
I'm not sure I get the philosophy of when connection ids are desired. Queue and Exchange creation seems to want that; deletion does not(?). Bindings don't contain any information. Subscriptions log the session id. The connection ids are lost on a restart anyway. However if they must be logged against queue and exchange creation I think that can be done more simply (i.e. less changes). (One option is using the existing qmf events; another is suggested in comments on exchange registry below, i.e. do it there since all that is logged is name and connection id).
trunk/qpid/cpp/include/qpid/SessionId.h
<https://reviews.apache.org/r/5616/#comment18743>
I think adding this here is wrong. A session id does not logically include the connection id. See suggestion in SessionState changes for simple alternative.
trunk/qpid/cpp/src/qpid/acl/Acl.cpp
<https://reviews.apache.org/r/5616/#comment18732>
Use getStatsAsMap()? (For consistency more than real performance concerns)
trunk/qpid/cpp/src/qpid/broker/Broker.cpp
<https://reviews.apache.org/r/5616/#comment18733>
Use getStatsAsMap()?
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp
<https://reviews.apache.org/r/5616/#comment18734>
Why does the name need to be passed in here? The management object was given the name when it was created.
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp
<https://reviews.apache.org/r/5616/#comment18735>
Assuming the '[Protocol]' string indicates the exchange was declared via an AMQP 0-10 command:
(a) how useful is that anyway? is this as opposed to creating it via the Broker::create() QMF command? What is the significance of that dinstinction? (It will no longer apply for AMQP 1.0).
(b) in this case it is actually potentially inaccurate. If the durable record does not include the manner in which it was created, then the distinction is lost on recovery anyway. (Which brings us back to (a) :-)
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp
<https://reviews.apache.org/r/5616/#comment18736>
I notice we are not recording connection id here...
trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp
<https://reviews.apache.org/r/5616/#comment18737>
Since the purpose of passing through the conection id to each exchange types constructor is simply to have a log statement with the exchange name and connection id, why no simply log that here and avoid having to pass it into the exchanges themselves?
trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/5616/#comment18738>
I am of the opinion that the '[restore]' renders this added field rather meaningless in the general case (and can't see that it is of great significance anyway).
trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp
<https://reviews.apache.org/r/5616/#comment18739>
Again, why does the name need to be passed in here?
trunk/qpid/cpp/src/qpid/broker/SessionState.cpp
<https://reviews.apache.org/r/5616/#comment18740>
Why add the connection id to the session id rather than just retrieving the connection id at this point via SessionState::getConnection() and ConnectionState::getUrl()?
trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp
<https://reviews.apache.org/r/5616/#comment18741>
I don't understand this change; could you clarify?
trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp
<https://reviews.apache.org/r/5616/#comment18742>
Why not have the Created log entry done in the management object as well? And why not just always use the management key?
- Gordon Sim
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review9134
-----------------------------------------------------------
Ship it!
I don't particularly like the term 'Model' in this context. Especially for the generated code, 'Management' seems much more appropriate (it won't be logged unless management is enabled and it is logging the stats the management agent records). For the events I would also prefer using Management or Broker. I assume the desire is to have a unique category that only selects these entity lifecycle related statements. Perhaps 'Lifecycle' would be an alternative?
That said, this is subjective and not the most important aspect. Your patience and persistence in accomodating my incessant whining has been much appreciated! I wouldn't object to inclusion as is if my points above do not convince you. This is a nice, clean addition. Good job!
- Gordon Sim
On July 13, 2012, 1:47 a.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 13, 2012, 1:47 a.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
> trunk/qpid/cpp/managementgen/qmf-gen 1360512
> trunk/qpid/cpp/managementgen/qmfgen/generate.py 1360512
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
> trunk/qpid/cpp/src/CMakeLists.txt 1360512
> trunk/qpid/cpp/src/Makefile.am 1360512
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review9140
-----------------------------------------------------------
Ship it!
trunk/qpid/cpp/managementgen/qmf-gen
<https://reviews.apache.org/r/5616/#comment19433>
Nit: this can be written
vargs["genLogs"] = bool(opts.qpidlogs)
- Alan Conway
On July 13, 2012, 1:47 a.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 13, 2012, 1:47 a.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
> trunk/qpid/cpp/managementgen/qmf-gen 1360512
> trunk/qpid/cpp/managementgen/qmfgen/generate.py 1360512
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
> trunk/qpid/cpp/src/CMakeLists.txt 1360512
> trunk/qpid/cpp/src/Makefile.am 1360512
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/
-----------------------------------------------------------
(Updated July 13, 2012, 1:47 a.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
Changes
-------
This is same as submission 5 but with two instances of showing User/Rhost (instead of RHost/User) and
two instances of not creating some strings unless they will be used.
This is ready for approval.
Description
-------
This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
[Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
This addresses bug QPID-4079.
https://issues.apache.org/jira/browse/QPID-4079
Diffs (updated)
-----
trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
trunk/qpid/cpp/managementgen/qmf-gen 1360512
trunk/qpid/cpp/managementgen/qmfgen/generate.py 1360512
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
trunk/qpid/cpp/src/CMakeLists.txt 1360512
trunk/qpid/cpp/src/Makefile.am 1360512
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
Diff: https://reviews.apache.org/r/5616/diff/
Testing
-------
Passes cmake test and automake make check.
Thanks,
Chug Rolke
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/
-----------------------------------------------------------
(Updated July 12, 2012, 6:29 p.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
Changes
-------
Logging in the generated management object classes include management id on creation and management id and stats on destruction. This facility is implemented in the generated code:
1. A new switch "-l" is added to qmf-gen to add the logging code.
2. Automake and cmake builds specify the -l to get the code, other builds do not.
3. Object creation and destruction emit the log data without any hooks or calls from broker calling classes.
Logging in user-specified terms is implemented by calling QPID_LOG near where raiseEvent is called.
Description
-------
This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
[Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
This addresses bug QPID-4079.
https://issues.apache.org/jira/browse/QPID-4079
Diffs (updated)
-----
trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
trunk/qpid/cpp/managementgen/qmf-gen 1360512
trunk/qpid/cpp/managementgen/qmfgen/generate.py 1360512
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
trunk/qpid/cpp/src/CMakeLists.txt 1360512
trunk/qpid/cpp/src/Makefile.am 1360512
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
Diff: https://reviews.apache.org/r/5616/diff/
Testing
-------
Passes cmake test and automake make check.
Thanks,
Chug Rolke
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review9105
-----------------------------------------------------------
Please don't review this version: changes in progress
- Chug Rolke
On July 12, 2012, 2:38 a.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 12, 2012, 2:38 a.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1360512
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1360512
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1360512
> trunk/qpid/cpp/src/qpid/broker/System.cpp 1360512
> trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1360512
> trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1360512
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1360512
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/
-----------------------------------------------------------
(Updated July 12, 2012, 2:38 a.m.)
Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
Changes
-------
This uses the new strategy described in QPID-4079:
1. It adds a new log category [Model]
2. INFO level logs for create/delete objects in the same user-specified names as management Events.
3. DEBUG level logs for create/delete objects using management keys.
The #2 info logs are created by using QPID_LOG_CAT() statements near existing raiseEvent() calls.
The #3 debug logs are new functions:
* creates are logged by a new member function and is called near where management objects are created.
* deletes are logged in resourceDestroy() with a common logging framework and a virtual call into the managed object to get the statistics.
Much of the complication of previous reviews came from trying to correlate the user-specified names with management keys at inappropriate places in the source code.
This diff is much simpler yet provides more correlation info.
Note this prints debug info for creation/deletion of Session objects but sessions are not logged with raiseEvent.
Description
-------
This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
[Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
This addresses bug QPID-4079.
https://issues.apache.org/jira/browse/QPID-4079
Diffs (updated)
-----
trunk/qpid/cpp/include/qpid/log/Statement.h 1360512
trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1360512
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1360512
trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1360512
trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/ConnectionHandler.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Link.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1360512
trunk/qpid/cpp/src/qpid/broker/System.cpp 1360512
trunk/qpid/cpp/src/qpid/cluster/Cluster.cpp 1360512
trunk/qpid/cpp/src/qpid/ha/HaBroker.cpp 1360512
trunk/qpid/cpp/src/qpid/log/Statement.cpp 1360512
trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1360512
Diff: https://reviews.apache.org/r/5616/diff/
Testing
-------
Passes cmake test and automake make check.
Thanks,
Chug Rolke
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Chug Rolke <cr...@redhat.com>.
> On July 3, 2012, 3:35 p.m., Alan Conway wrote:
> > trunk/qpid/cpp/src/qpid/acl/Acl.cpp, line 296
> > <https://reviews.apache.org/r/5616/diff/3/?file=118887#file118887line296>
> >
> > Performance problem: mapEncodeValues is called even if the log statememnt is disabled.
> >
> > Sugget adding op<<(const ManagementObject&)
> > or struct PrintableManagementObject { const ManagementObject& }; with op<<(PrintableManagementObject), do all the work in the message specification.
> >
> > This is repeated in many other places.
This instance is in ~Acl(). I contend that this and the one in ~Broker() are not performance issues.
The only instance that is performance critical is in ManagementObject::resourceDestroy(). There the creation of a map and its population with stats is qualified with the QPID_LOG_TEST(logEnabled) macro.
> On July 3, 2012, 3:35 p.m., Alan Conway wrote:
> > trunk/qpid/cpp/src/qpid/broker/Exchange.cpp, line 222
> > <https://reviews.apache.org/r/5616/diff/3/?file=118895#file118895line222>
> >
> > Suggest moving this logic to a struct and defining op<< for the struct. Then all can be in-line in a single message:
> >
> > struct Foo {
> > Foo(string n, string i) : name(n), id(i){}
> > };
> > op<<(const Foo&) { ... }
> > QPID_LOG_CAT(debug, model, "xxx:" << Foo(name,connectionId))
Great idea. Thank you.
- Chug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8833
-----------------------------------------------------------
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>
Re: Review Request: Add log entries for correlatable broker object life
cycles
Posted by Alan Conway <ac...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5616/#review8833
-----------------------------------------------------------
trunk/qpid/cpp/src/qpid/acl/Acl.cpp
<https://reviews.apache.org/r/5616/#comment18747>
Performance problem: mapEncodeValues is called even if the log statememnt is disabled.
Sugget adding op<<(const ManagementObject&)
or struct PrintableManagementObject { const ManagementObject& }; with op<<(PrintableManagementObject), do all the work in the message specification.
This is repeated in many other places.
trunk/qpid/cpp/src/qpid/broker/Connection.cpp
<https://reviews.apache.org/r/5616/#comment18749>
why passing mgmtid to the mgmtObject? Doesn't it already know?
trunk/qpid/cpp/src/qpid/broker/Exchange.cpp
<https://reviews.apache.org/r/5616/#comment18750>
Suggest moving this logic to a struct and defining op<< for the struct. Then all can be in-line in a single message:
struct Foo {
Foo(string n, string i) : name(n), id(i){}
};
op<<(const Foo&) { ... }
QPID_LOG_CAT(debug, model, "xxx:" << Foo(name,connectionId))
- Alan Conway
On July 2, 2012, 9:11 p.m., Chug Rolke wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5616/
> -----------------------------------------------------------
>
> (Updated July 2, 2012, 9:11 p.m.)
>
>
> Review request for qpid, Alan Conway, Gordon Sim, and Ted Ross.
>
>
> Description
> -------
>
> This patch adds a new log category [Configuration] and publishes a bunch of information at info level in the format:
> [Configuration] <object> <event>, where <object> is one of Connection, Session, Queue, Subscription, Exchange or Binding, and <event> is one of created or closed with connection also getting a setUser.
> The bulk of the patch involves passing the necessary strings down to the object creators so that they can emit the log.
>
>
> This addresses bug QPID-4079.
> https://issues.apache.org/jira/browse/QPID-4079
>
>
> Diffs
> -----
>
> trunk/qpid/cpp/include/qpid/SessionId.h 1354515
> trunk/qpid/cpp/include/qpid/log/Statement.h 1354515
> trunk/qpid/cpp/include/qpid/management/ManagementObject.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.h 1354515
> trunk/qpid/cpp/managementgen/qmfgen/templates/Class.cpp 1354515
> trunk/qpid/cpp/src/qpid/SessionId.cpp 1354515
> trunk/qpid/cpp/src/qpid/acl/Acl.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Bridge.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Connection.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/DirectExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Exchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/ExchangeRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/FanOutExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Link.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.h 1354515
> trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.h 1354515
> trunk/qpid/cpp/src/qpid/broker/QueueRegistry.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SaslAuthenticator.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionAdapter.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionHandler.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/SessionState.cpp 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.h 1354515
> trunk/qpid/cpp/src/qpid/broker/TopicExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/ha/BrokerReplicator.cpp 1354515
> trunk/qpid/cpp/src/qpid/log/Statement.cpp 1354515
> trunk/qpid/cpp/src/qpid/management/ManagementObject.cpp 1354515
> trunk/qpid/cpp/src/qpid/replication/ReplicatingEventListener.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.h 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchange.cpp 1354515
> trunk/qpid/cpp/src/qpid/xml/XmlExchangePlugin.cpp 1354515
> trunk/qpid/cpp/src/tests/ExchangeTest.cpp 1354515
>
> Diff: https://reviews.apache.org/r/5616/diff/
>
>
> Testing
> -------
>
> Passes cmake test and automake make check.
>
>
> Thanks,
>
> Chug Rolke
>
>