[
https://issues.apache.org/jira/browse/ARTEMIS-4338?focusedWorklogId=869316&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-869316
]
ASF GitHub Bot logged work on ARTEMIS-4338:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 05/Jul/23 12:48
Start Date: 05/Jul/23 12:48
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4530:
URL: https://github.com/apache/activemq-artemis/pull/4530#discussion_r1252202278
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ProtocolHandler.java:
##########
@@ -243,6 +243,12 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf
in, List<Object> out) t
ctx.flush();
}
+ @Override
+ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+
ActiveMQServerLogger.LOGGER.failureDuringProtocolHandshake(ctx.channel().remoteAddress().toString(),
cause);
Review Comment:
Including the local address may make sense...broker has multiple acceptors,
which can also potentially be multi-homed on top of that.
Could potentially drop the toString and leave it to the logger.
##########
tests/smoke-tests/src/main/resources/servers/audit-logging2/log4j2.properties:
##########
@@ -40,7 +40,7 @@ logger.audit_base = INFO, audit_log_file
logger.audit_base.name = org.apache.activemq.audit.base
logger.audit_base.additivity = false
-logger.audit_resource = OFF, audit_log_file
+logger.audit_resource = INFO, audit_log_file
Review Comment:
Related to other comment, the resource logger is enabled in the
AuditLoggerResourceTest test config already.
##########
tests/smoke-tests/pom.xml:
##########
@@ -188,6 +188,13 @@
<version>${project.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.activemq.tests</groupId>
+ <artifactId>integration-tests</artifactId>
+ <version>${project.version}</version>
+ <type>test-jar</type>
+ <scope>test</scope>
+ </dependency>
Review Comment:
I've slowly been trying to break all of the ugly chained test-jar
dependencies, and related classpath pollutions coming with them, so rather than
adding/restoring this one...
It looks like you added this to get to a STOMP test-client? The AMQP
test-client lives in the _artemis-test-support_ module for sharing supporting
systest helper bits (specifically,
tests/artemis-test-support/src/main/java/org/apache/activemq/transport/amqp/client/),
as does the CFUtil the new tests also use. Seems like moving the STOMP
test-client alongside them would be nicer than this, assuming its simple.
EDIT: looks like the stomp client innards already use the transport bits
from artemis-test-support, making it seem like an even more appropriate move.
I'll put together a PR after lunch.
##########
tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/logging/AuditLoggerTest.java:
##########
@@ -117,6 +121,56 @@ public void testAuditLog() throws Exception {
checkAuditLogRecord(true, "is sending a message");
}
+ @Test
+ public void testCoreConnectionAuditLog() throws Exception {
+ testConnectionAuditLog("CORE");
+ }
+
+ @Test
+ public void testAMQPConnectionAuditLog() throws Exception {
+ testConnectionAuditLog("AMQP");
+ }
+
+ @Test
+ public void testOpenWireConnectionAuditLog() throws Exception {
+ testConnectionAuditLog("OPENWIRE");
+ }
Review Comment:
Since these are considered 'resource' logs, they might be better in
AuditLoggerResourceTest
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ProtocolHandler.java:
##########
@@ -243,6 +243,12 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf
in, List<Object> out) t
ctx.flush();
}
+ @Override
+ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
+
ActiveMQServerLogger.LOGGER.failureDuringProtocolHandshake(ctx.channel().remoteAddress().toString(),
cause);
+ ctx.close();
Review Comment:
Since this is the important bit perhaps whacking it in a try-finally would
make sense.
Issue Time Tracking
-------------------
Worklog Id: (was: 869316)
Time Spent: 40m (was: 0.5h)
> STOMP inoperable w/resource audit logging enabled
> -------------------------------------------------
>
> Key: ARTEMIS-4338
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4338
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Components: STOMP
> Affects Versions: 2.29.0
> Reporter: Otavio Rodolfo Piske
> Assignee: Justin Bertram
> Priority: Blocker
> Attachments: camel-stomp-test-debug-2.28.0.log,
> camel-stomp-test-debug-2.29.0.log
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Recently we tried upgrading to Artemis 2.29, but when doing so our
> integration tests started to fail. No code was changed as part of the upgrade.
> With 2.29, the connection seems to fail due to a connection timeout:
>
> {noformat}
> 2023-06-29 07:51:40,739 [Impl$6@b91d8c4)] WARN server
> - AMQ222067: Connection failure has been detected: AMQ229014: Did not
> receive data from /127.0.0.1:50934 within the 60000ms connection TTL. The
> connection will now be closed. [code=CONNECTION_TIMEDOUT]{noformat}
>
> I wasn't able to determine the problem, so I am attaching the debug logs for
> an execution with 2.28 and another with 2.29.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
