ARTEMIS-5434 Use full exception name in exceptions

[ViliusS]

I hope that I'm not breaking any backward compatibility here but it would be great to have a full package name in the exception header. Currently Artemis exceptions only include the short name of the exception. On logging systems where exceptions are caught and tracked if they are repeating errors (like for example Google Error Reporting), this groups different Artemis exceptions into single error group.

See apache/logging-log4j2#3586 (comment) for more information and screenshots.

[jbertram]

What do you mean by "exception header" in this context? The change in the PR is only touching the toString of org.apache.activemq.artemis.api.core.ActiveMQException.

Are you saying that because the toString doesn't contain the package name then some log analyzers can't disambiguate log messages that contain exception stack-traces?

[ViliusS]

Some context how Google Error Reporting works. It analyzes all log messages and tries to find exceptions in them. If exception is found monitoring agent is notified about new problem within an environment. If the same exception is found it is grouped and produces histogram, so monitoring agent can clearly see when the problem started to appear. See the following screenshot as an example:

If the error is marked as resolved and later reoccurs, monitoring agent is again notified, that the same error resurfaced.

All of this is great except the part how Google Error Reporting differentiates between what is the same error and what is not. I'm not sure if it takes into account what is written after exception name (in Artemis case the part between []), but it definitely looks into full package name in the first line of the exception. If org.apache.activemq.artemis.api.core.ActiveMQIllegalStateException and com.third.party.package.ActiveMQIllegalStateException are found but exception structure just returns ActiveMQIllegalStateException in the first line it is treated as the same error case, which is not.

This PR also would make exception format more inline with what how exceptions are returned from standard Java libraries. java.lang example:

[ppkarwasz]

LGTM

For Artemis users with a standard logging configuration this will not change anything, since the %ex pattern converter does not call the toString() method of the exception.

A change in the formatting of stacktraces in logs will occur if apache/logging-log4j2#3623 is implemented.

[jbertram]

Just to clarify...In your environment you have exceptions with the same class names as those in ActiveMQ Artemis?

[ViliusS]

Just to clarify...In your environment you have exceptions with the same class names as those in ActiveMQ Artemis?

For ActiveMQ Artemis not really. I just was going through all our environments and application packages, implementing JSON logging for proper integration with Google Cloud Logging. During this process I noticed the same error grouping issue with other Java software, not related to Artemis, which uses more generic exception names and also doesn't provide FQCN in the exception. I have already fixed this in those packages.
Not sure if this change could breaks anything, but I thought it is good idea to change it everywhere, including the broker itself which we monitor constantly in our environment.

[jbertram]

@ViliusS, thanks for the clarification. Set this to "Ready for review" once you've made all your changes, and I'll take another look.

[jbertram]

Please make sure you squash all your commits.

[ViliusS]

I usually try to avoid squashing because then review comments get disconnected from the changes, and it should be possible to squash during merge. But if that's really needed it is done now.

[jbertram]

I usually try to avoid squashing because then review comments get disconnected from the changes...

In what way do review comments get disconnected from the changes?

[ViliusS]

If you go here https://github.com/apache/activemq-artemis/pull/5639/files you will see that there is no comment from Piotr. After force-pushing comments remain only visible in Conversation thread here, but if you want to see them in full file context, or if you want them to be shown for the next reviewer, they are now gone in Files Changed tab.

[ppkarwasz]

I usually try to avoid squashing because then review comments get disconnected from the changes...

In what way do review comments get disconnected from the changes?

If the PR is longer you also lose the possibility of reviewing just the recent changes to the PR and you need to read it from the beginning.