[
https://issues.apache.org/jira/browse/ARTEMIS-2919?focusedWorklogId=519784&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-519784
]
ASF GitHub Bot logged work on ARTEMIS-2919:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 03/Dec/20 17:24
Start Date: 03/Dec/20 17:24
Worklog Time Spent: 10m
Work Description: gemmellr commented on a change in pull request #3279:
URL: https://github.com/apache/activemq-artemis/pull/3279#discussion_r535422701
##########
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AMQPMessageSupport.java
##########
@@ -198,6 +200,8 @@
public static final Symbol JMS_DEST_TYPE_MSG_ANNOTATION =
getSymbol("x-opt-jms-dest");
public static final Symbol JMS_REPLY_TO_TYPE_MSG_ANNOTATION =
getSymbol("x-opt-jms-reply-to");
+ public static final Symbol INGRESS_TIME = getSymbol(HDR_INGRESS_TIME);
Review comment:
Might be good to group and align the name with the other _MSG_ANNOTATION
constants above?
##########
File path:
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/AmqpCoreConverter.java
##########
@@ -283,6 +284,8 @@ protected static ServerJMSMessage
processMessageAnnotations(ServerJMSMessage jms
if (delay > 0) {
jms.setLongProperty(HDR_SCHEDULED_DELIVERY_TIME.toString(),
System.currentTimeMillis() + delay);
}
+ } else if (AMQPMessageSupport.HDR_INGRESS_TIME.equals(key) &&
entry.getValue() != null) {
+ jms.setLongProperty(HDR_INGRESS_TIME.toString(), ((Number)
entry.getValue()).longValue());
Review comment:
Might be good to rename AMQPMessageSupport.HDR_INGRESS_TIME to avoid the
qualification and confusing it can cause, and be more consistent with the
symbol name created from it?
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/IngressTimeTest.java
##########
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.client;
+
+import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
+import javax.jms.DeliveryMode;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
+import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class IngressTimeTest extends ActiveMQTestBase {
+ private ActiveMQServer server;
+
+ private final SimpleString QUEUE = new SimpleString("ConsumerTestQueue");
+
+ @Before
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ server = createServer(true, true);
+ server.start();
+ server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setEnableIngressTime(true));
+ server.createQueue(new
QueueConfiguration(QUEUE).setRoutingType(RoutingType.ANYCAST));
+ }
+
+ @Test
+ public void testSendCoreReceiveCore() throws Throwable {
+ internalSendReceive(Protocol.CORE, Protocol.CORE);
+ }
+
+ @Test
+ public void testSendAMQPReceiveCore() throws Throwable {
+ internalSendReceive(Protocol.AMQP, Protocol.CORE);
+ }
+
+ @Test
+ public void testSendOpenWireReceiveCore() throws Throwable {
+ internalSendReceive(Protocol.OPENWIRE, Protocol.CORE);
+ }
+
+ @Test
+ public void testSendCoreReceiveOpenwire() throws Throwable {
+ internalSendReceive(Protocol.CORE, Protocol.OPENWIRE);
+ }
+
+ @Test
+ public void testSendAMQPReceiveOpenWire() throws Throwable {
+ internalSendReceive(Protocol.AMQP, Protocol.OPENWIRE);
+ }
+
+ @Test
+ public void testSendOpenWireReceiveOpenWire() throws Throwable {
+ internalSendReceive(Protocol.OPENWIRE, Protocol.OPENWIRE);
+ }
+
+ /**
+ * AMQP receiver tests are in
org.apache.activemq.artemis.tests.integration.amqp.AmqpIngressTimeTest
+ */
+
+ private void internalSendReceive(Protocol protocolSender, Protocol
protocolConsumer) throws Throwable {
+ ConnectionFactory factorySend = createFactory(protocolSender);
+ ConnectionFactory factoryConsume = protocolConsumer == protocolSender ?
factorySend : createFactory(protocolConsumer);
+
+ long beforeSend, afterSend;
+ try (Connection connection = factorySend.createConnection()) {
+ try (Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE)) {
+ javax.jms.Queue queue = session.createQueue(QUEUE.toString());
+ try (MessageProducer producer = session.createProducer(queue)) {
+ producer.setDeliveryMode(DeliveryMode.PERSISTENT);
+ TextMessage msg = session.createTextMessage("hello");
+ beforeSend = System.currentTimeMillis();
+ producer.send(msg);
+ afterSend = System.currentTimeMillis();
+ }
+ }
+ }
+
+ server.stop();
+ server.start();
+ assertTrue(server.waitForActivation(3, TimeUnit.SECONDS));
+
+ try (Connection connection = factoryConsume.createConnection()) {
+ connection.start();
+ try (Session session = connection.createSession(false,
Session.AUTO_ACKNOWLEDGE)) {
+ javax.jms.Queue queue = session.createQueue(QUEUE.toString());
+ try (MessageConsumer consumer = session.createConsumer(queue)) {
+ TextMessage message = (TextMessage) consumer.receive(1000);
+ Assert.assertNotNull(message);
+ Object ingressTime =
message.getObjectProperty(Message.HDR_INGRESS_TIME.toString());
+ assertNotNull(ingressTime);
+ assertTrue(ingressTime instanceof Long);
+ long ingress = (Long) ingressTime;
+ assertTrue("Ingress time should be set in expected
range",beforeSend <= ingress && afterSend >= ingress);
Review comment:
Same point as last time about the unhelpful failure this will produce
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpIngressTimeTest.java
##########
@@ -0,0 +1,98 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.apache.activemq.artemis.protocol.amqp.converter.AMQPMessageSupport;
+import org.apache.activemq.transport.amqp.client.AmqpClient;
+import org.apache.activemq.transport.amqp.client.AmqpConnection;
+import org.apache.activemq.transport.amqp.client.AmqpMessage;
+import org.apache.activemq.transport.amqp.client.AmqpReceiver;
+import org.apache.activemq.transport.amqp.client.AmqpSession;
+import org.junit.Test;
+
+public class AmqpIngressTimeTest extends AmqpClientTestSupport {
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendCore() throws Exception {
+ internalTestIngressTimestamp(Protocol.CORE);
+ }
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendAMQP() throws Exception {
+ internalTestIngressTimestamp(Protocol.AMQP);
+ }
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendOpenWire() throws Exception {
+ internalTestIngressTimestamp(Protocol.OPENWIRE);
+ }
+
+ private void internalTestIngressTimestamp(Protocol protocol) throws
Exception {
+ server.getAddressSettingsRepository().addMatch(getQueueName(), new
AddressSettings().setEnableIngressTime(true));
+ long beforeSend = System.currentTimeMillis();
+ if (protocol == Protocol.CORE) {
+ sendMessagesCore(getQueueName(), 1, true);
+ } else if (protocol == Protocol.OPENWIRE) {
+ sendMessagesOpenWire(getQueueName(), 1, true);
+ } else {
+ sendMessages(getQueueName(), 1, true);
+ }
+ long afterSend = System.currentTimeMillis();
+
+ server.stop();
+ server.start();
+ assertTrue(server.waitForActivation(3, TimeUnit.SECONDS));
Review comment:
Its good there is now a test that verifies it survives restart...but now
it seems we dont have any that verifies it works without the restart except
STOMP? With my earlier comment I was more thinking of at least a single added
test that checked it survived restart, rather than swapping the existing ones.
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/SimpleTest.java
##########
@@ -83,6 +83,7 @@ public void setUp() throws Exception {
@Test
public void simpleTest() throws Exception {
+ System.out.println((String) null);
Review comment:
Leftover debug?
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpIngressTimeTest.java
##########
@@ -0,0 +1,98 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.core.server.Queue;
+import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
+import org.apache.activemq.artemis.protocol.amqp.converter.AMQPMessageSupport;
+import org.apache.activemq.transport.amqp.client.AmqpClient;
+import org.apache.activemq.transport.amqp.client.AmqpConnection;
+import org.apache.activemq.transport.amqp.client.AmqpMessage;
+import org.apache.activemq.transport.amqp.client.AmqpReceiver;
+import org.apache.activemq.transport.amqp.client.AmqpSession;
+import org.junit.Test;
+
+public class AmqpIngressTimeTest extends AmqpClientTestSupport {
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendCore() throws Exception {
+ internalTestIngressTimestamp(Protocol.CORE);
+ }
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendAMQP() throws Exception {
+ internalTestIngressTimestamp(Protocol.AMQP);
+ }
+
+ @Test(timeout = 60000)
+ public void testIngressTimestampSendOpenWire() throws Exception {
+ internalTestIngressTimestamp(Protocol.OPENWIRE);
+ }
+
+ private void internalTestIngressTimestamp(Protocol protocol) throws
Exception {
+ server.getAddressSettingsRepository().addMatch(getQueueName(), new
AddressSettings().setEnableIngressTime(true));
+ long beforeSend = System.currentTimeMillis();
+ if (protocol == Protocol.CORE) {
+ sendMessagesCore(getQueueName(), 1, true);
+ } else if (protocol == Protocol.OPENWIRE) {
+ sendMessagesOpenWire(getQueueName(), 1, true);
+ } else {
+ sendMessages(getQueueName(), 1, true);
+ }
+ long afterSend = System.currentTimeMillis();
+
+ server.stop();
+ server.start();
+ assertTrue(server.waitForActivation(3, TimeUnit.SECONDS));
+
+ AmqpClient client = createAmqpClient();
+ AmqpConnection connection = addConnection(client.connect());
+ AmqpSession session = connection.createSession();
+
+ AmqpReceiver receiver = session.createReceiver(getQueueName());
+
+ Queue queueView = getProxyToQueue(getQueueName());
+ assertEquals(1, queueView.getMessageCount());
+
+ receiver.flow(1);
+ AmqpMessage receive = receiver.receive(5, TimeUnit.SECONDS);
+ assertNotNull(receive);
+ instanceLog.info(receive);
+ Object ingressTimestamp =
receive.getMessageAnnotation(AMQPMessageSupport.HDR_INGRESS_TIME);
+ assertNotNull(ingressTimestamp);
+ assertTrue(ingressTimestamp instanceof Long);
+ long ingressTs = (Long) ingressTimestamp;
+ assertTrue("Ingress time should be set in expected range",beforeSend <=
ingressTs && afterSend >= ingressTs);
Review comment:
Same point as last time about the unhelpful failure this will produce
##########
File path:
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java
##########
@@ -709,6 +709,27 @@ public void testSubscribeWithAutoAck() throws Exception {
}
+ @Test
+ public void testIngressTimestamp() throws Exception {
+ server.getAddressSettingsRepository().addMatch("#", new
AddressSettings().setEnableIngressTime(true));
+ conn.connect(defUser, defPass);
+
+ subscribe(conn, null, Stomp.Headers.Subscribe.AckModeValues.AUTO);
+ long before = System.currentTimeMillis();
+ sendJmsMessage(getName());
+ long after = System.currentTimeMillis();
+
+ ClientStompFrame frame = conn.receiveFrame(10000);
+ Assert.assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+ Assert.assertEquals(getQueuePrefix() + getQueueName(),
frame.getHeader(Stomp.Headers.Send.DESTINATION));
+ String ingressTimeHeaderValue =
frame.getHeader(Stomp.Headers.Message.INGRESS_TIME);
+ Assert.assertNotNull(ingressTimeHeaderValue);
+ long ingressTime = Long.parseLong(ingressTimeHeaderValue);
+ assertTrue("Ingress time should be set in expected range", before <=
ingressTime && after >= ingressTime);
Review comment:
Same point as last time about the unhelpful failure this will produce
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 519784)
Time Spent: 5h (was: 4h 50m)
> Support timestamping incoming messages
> --------------------------------------
>
> Key: ARTEMIS-2919
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2919
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
