[
https://issues.apache.org/jira/browse/QPID-7799?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16198721#comment-16198721
]
Alex Rudyy commented on QPID-7799:
----------------------------------
Here my review comments
* The usage of context variables ( there names and values used in implemented
functionality to dump of statistics into logs) are inconsistent with the rest
of the broker
** {{qpid.<categoryclass>.statisticsReportPattern}} - the name should indicate
that it is for logging of the statistics into the logs.
** the above includes the category class in the name. Would it be preferable to
specify the category in the value and serialize the categories and their
patterns as map? IMHO, using map would be consistent with other broker code.
Though, editing of the map serialized as json string would be difficult for the
end user, thus, introduction of a separate managed attribute having type either
List or Map would be more advantageous. Another argument for introduction of
the separate attribute would be a state of our context editor - locating
statistics settings there could be a challenging task :) A third argument
against using context variables is that context variables represent the
settings which should not be really used by the end user or used in exceptional
situations. Statistics logging settings do not belong to this category.
** segregated formatting looks unusual. It also overlaps with the statistics
units. It seems that formatting should be simply represented as boolean flag;
if set, the output is provided in human readable format and units to display
should be taken from the statistics declaration.
* StatisticsReportingTask looks for the settings in the context every time.
IMHO, the settings can be detected in the constructor and re-used in the
invocation of the task. When settings are changed the task can be recreated.
Using dedicated attributes for statistics logging would simplify the task
creation.
* Statistics logging requires pre-created logging rule for logger
qpid.statistics.*". Potentially, rule can be added automatically and removed
automatically on adding and removing statistics settings.
* Additionally, the implementation only allows to enable reporting per category
basis. If only statistics for certain queues is required, more changes would be
needed for implementation. IMHO, the more flexible solution could be
implemented using
** sperate statistic settings attribute
** the value can include multiple statistic criteria allowing to specify the
statistic names to report, whether human readable formatting is required,
category or a name of the object to report statistics for. For example,
{code:javascript}
"statisticsLogging" : [
{
category: "Connection",
statistics: ["lastIoTime", "bytesIn", "bytesOut", "messagesIn",
"messagesOut"],
humanReadeble: true
},
{
name: "myQueue",
category: "Queue",
statistics: ["queueDepthBytes", "queueDepthMessages"],
humanReadeble: true
}
]
{code}
or
{code:javascript}
"statisticsLogging" : {
{
"Connection": { humanReadable: true, statistics: ["lastIoTime",
"bytesIn", "bytesOut", "messagesIn", "messagesOut"], name="*"},
"Queue": { humanReadable: true, statistics: ["queueDepthBytes",
"queueDepthMessages"], name: "myQueue"}
}
{code}
Alternately, where conditions similar for query where conditions can be
introduced to report statistics.
* I am not sure about replacing StatisticsCounter with AtomicLongs as that
might create a problem with updating statistics from multiple threads. For
example, updating of virtual host inbound and outbound statistics would need to
be synchronized somehow.
> Broker should be able to write a periodic dump of statistics
> ------------------------------------------------------------
>
> Key: QPID-7799
> URL: https://issues.apache.org/jira/browse/QPID-7799
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Keith Wall
> Assignee: Keith Wall
> Fix For: qpid-java-broker-7.0.0
>
> Attachments:
> 0001-QPID-7799-Java-Broker-Improve-statistics-reporting.patch
>
>
> To assist those running a service, the Broker should have the ability to
> write a periodic dump of ConiguredObject statistics to the log file. It
> should be possible to configure the statistics dumped at runtime and be
> configured on a per-category or per-object instance basis. Within a HA
> group, the configuration applied to objects belonging to a virtualhost must
> survive a mastership change.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
