[
https://issues.apache.org/jira/browse/AMQ-8601?focusedWorklogId=769238&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-769238
]
ASF GitHub Bot logged work on AMQ-8601:
---------------------------------------
Author: ASF GitHub Bot
Created on: 11/May/22 18:52
Start Date: 11/May/22 18:52
Worklog Time Spent: 10m
Work Description: zisding opened a new pull request, #840:
URL: https://github.com/apache/activemq/pull/840
Hello,
While viewing the
[MAPREDUCE-4262](https://issues.apache.org/jira/browse/MAPREDUCE-4262), I found
that the logging statements might give inaccurate messages.
I also found that the in the line 93 of the file
[UpdateVirtualDestinationsTask](https://github.com/apache/activemq/blob/b3c2c49f96bebee14b74c6456feb233e0312724b/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/UpdateVirtualDestinationsTask.java#L93),
the log messages says "Removing virtual destination ". However, the removing
action should already completed in previous code (line 92).
```
plugin.virtualDestinationRemoved(connectionContext, removedVirtualDest);
LOG.info("Removing virtual destination: {}", removedVirtualDest);
```
Would it be better if we change the verb "Removing" to "Removed" to indicate
the action is completed, which is similar to the previous [logging
statement?](https://github.com/apache/activemq/blob/b3c2c49f96bebee14b74c6456feb233e0312724b/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/UpdateVirtualDestinationsTask.java#L82)
```
virtualDestinationInterceptor
.setVirtualDestinations(getVirtualDestinations());
plugin.info("applied updates to: "
+ virtualDestinationInterceptor);
```
Or can we move the logging statement to the line before 92? Since when there
was an exception in previous lines, the logging message would not be printed,
which may be not good for debugging.
A similar issue is also found in the file
[StatisticsBrokerPlugin](https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-broker/src/main/java/org/apache/activemq/plugin/StatisticsBrokerPlugin.java#L47)
where it seems that the logging statement it is describing the whole method.
Would it be better to move this logging statement to the beginning of the
method?
Especially considering the following logging practices:
-
https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-broker/src/main/java/org/apache/activemq/plugin/DiscardingDLQBrokerPlugin.java#L56
-
https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/RuntimeConfigurationPlugin.java#L37
Thanks.
Issue Time Tracking
-------------------
Worklog Id: (was: 769238)
Remaining Estimate: 0h
Time Spent: 10m
> UpdateVirtualDestinationsTask gives inaccurate log message saying "Removing
> virtual destination ... " after already applied the removal
> ---------------------------------------------------------------------------------------------------------------------------------------
>
> Key: AMQ-8601
> URL: https://issues.apache.org/jira/browse/AMQ-8601
> Project: ActiveMQ
> Issue Type: Bug
> Reporter: Ding Ding
> Priority: Trivial
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Hello,
> While viewing the MAPREDUCE-4262, I found that the logging statements might
> give inaccurate messages.
> I also found that the in the line *93* of the file
> [UpdateVirtualDestinationsTask|https://github.com/apache/activemq/blob/b3c2c49f96bebee14b74c6456feb233e0312724b/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/UpdateVirtualDestinationsTask.java#L93],
> the log messages says "{_}Removing virtual destination "{_}. However, the
> removing action should already completed in previous code ({*}line 92{*}).
> {code:java}
> plugin.virtualDestinationRemoved(connectionContext, removedVirtualDest);
> LOG.info("Removing virtual destination: {}",
> removedVirtualDest); {code}
> Would it be better if we change the verb "Removing" to "Removed" to indicate
> the action is completed, which is similar to the previous logging statement?
> {code:java}
> virtualDestinationInterceptor
> .setVirtualDestinations(getVirtualDestinations());
> plugin.info("applied updates to: "
> + virtualDestinationInterceptor); {code}
> Or can we move the logging statement to the line before {*}92{*}? Since when
> there was an exception in previous lines, the logging message would not be
> printed, which may be not good for debugging.
>
> A similar issue is also found in the file
> [StatisticsBrokerPlugin|https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-broker/src/main/java/org/apache/activemq/plugin/StatisticsBrokerPlugin.java#L47]
> where it seems that the logging statement it is describing the whole method.
> Would it be better to move this logging statement to the beginning of the
> method?
> Especially considering the following logging practices:
> #
> [https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-broker/src/main/java/org/apache/activemq/plugin/DiscardingDLQBrokerPlugin.java#L56]
> #
> [https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/RuntimeConfigurationPlugin.java#L37]
>
> Thanks.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
