You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "ViliusS (via GitHub)" <gi...@apache.org> on 2025/04/16 11:05:51 UTC
[PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
ViliusS opened a new pull request, #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639
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 https://github.com/apache/logging-log4j2/pull/3586#discussion_r2044006120 for more information and screenshots.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2815819788
@ViliusS, thanks for the clarification. Set this to "Ready for review" once you've made all your changes, and I'll take another look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2813381050
Just to clarify...In your environment you have exceptions with the same class names as those in ActiveMQ Artemis?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ViliusS (via GitHub)" <gi...@apache.org>.
ViliusS commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2814839166
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on code in PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#discussion_r2049100971
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQException.java:
##########
@@ -75,6 +75,6 @@ public ActiveMQExceptionType getType() {
@Override
public String toString() {
- return this.getClass().getSimpleName() + "[errorType=" + type + " message=" + getMessage() + "]";
+ return this.getClass().getName() + "[errorType=" + type + " message=" + getMessage() + "]";
Review Comment:
```suggestion
return this.getClass().getName() + ": [errorType=" + type + " message=" + getMessage() + "]";
```
It is probably better to keep the format `<fully_qualified_class_name>: <custom_message>`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2810681152
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ViliusS (via GitHub)" <gi...@apache.org>.
ViliusS commented on code in PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#discussion_r2050286413
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQException.java:
##########
@@ -75,6 +75,6 @@ public ActiveMQExceptionType getType() {
@Override
public String toString() {
- return this.getClass().getSimpleName() + "[errorType=" + type + " message=" + getMessage() + "]";
+ return this.getClass().getName() + "[errorType=" + type + " message=" + getMessage() + "]";
Review Comment:
I agree. If Justin thinks this is useful I will commit suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2816696612
> > 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram merged PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2816109561
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on code in PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#discussion_r2050850994
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ActiveMQException.java:
##########
@@ -75,6 +75,6 @@ public ActiveMQExceptionType getType() {
@Override
public String toString() {
- return this.getClass().getSimpleName() + "[errorType=" + type + " message=" + getMessage() + "]";
+ return this.getClass().getName() + "[errorType=" + type + " message=" + getMessage() + "]";
Review Comment:
I think that's a useful change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2815988671
Please make sure you squash all your commits.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ViliusS (via GitHub)" <gi...@apache.org>.
ViliusS commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2816549211
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ViliusS (via GitHub)" <gi...@apache.org>.
ViliusS commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2812021316
Some context [how Google Error Reporting works](https://cloud.google.com/error-reporting/docs/grouping-errors#how_errors_are_grouped). 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:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] ARTEMIS-5434 Use full exception name in exceptions [activemq-artemis]
Posted by "ViliusS (via GitHub)" <gi...@apache.org>.
ViliusS commented on PR #5639:
URL: https://github.com/apache/activemq-artemis/pull/5639#issuecomment-2816084895
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact