[AMQ-8601] UpdateVirtualDestinationsTask gives inaccurate log message saying "Removing virtual destination ... " after already applied the removal

• Type: Bug
• Status: Resolved
• Priority: Trivial
• Resolution: Fixed
• Fix Version/s: 5.18.0, 5.17.2
• Assignee: Jean-Baptiste Onofré
• Reporter: Ding Ding
• Created: 11/May/22 18:21
• Updated: 17/Aug/22 05:49
• Resolved: 17/Aug/22 05:49

Description

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, 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?

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 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:

1. https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-broker/src/main/java/org/apache/activemq/plugin/DiscardingDLQBrokerPlugin.java#L56
2. https://github.com/apache/activemq/blob/9b1eb96b838957cd60541ca5e057567be3f11990/activemq-runtime-config/src/main/java/org/apache/activemq/plugin/RuntimeConfigurationPlugin.java#L37 

Thanks.