From 4fdfe1cdeb27100bf26c404cf3e8028bec625d60 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Thu, 11 Dec 2014 12:09:47 -0500 Subject: [PATCH] AMQP-446 Reduce Logs for Temporary Declarations JIRA: https://jira.spring.io/browse/AMQP-446 Previously, 3 WARN log entries were generated when declaring a non-durable, auto-delete, exclusive queue. 2 logs were generated for a non-durable, auto-delete exchange. Reduce log level to INFO and only emit 1 log per queue/exchange. --- .../amqp/rabbit/core/RabbitAdmin.java | 32 +++----- .../amqp/rabbit/core/RabbitAdminTests.java | 75 +++++++++++++++++++ 2 files changed, 86 insertions(+), 21 deletions(-) diff --git a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java index 15dbdd03..a91571e7 100644 --- a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java +++ b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java @@ -358,33 +358,23 @@ public class RabbitAdmin implements AmqpAdmin, ApplicationContextAware, Initiali final Collection bindings = filterDeclarables(applicationContext.getBeansOfType(Binding.class).values()); for (Exchange exchange : exchanges) { - if (!exchange.isDurable()) { - logger.warn("Auto-declaring a non-durable Exchange (" + if (!exchange.isDurable() || exchange.isAutoDelete()) { + logger.info("Auto-declaring a non-durable or auto-delete Exchange (" + exchange.getName() - + "). It will be deleted by the broker if it shuts down, and can be redeclared by closing and reopening the connection."); - } - if (exchange.isAutoDelete()) { - logger.warn("Auto-declaring an auto-delete Exchange (" - + exchange.getName() - + "). It will be deleted by the broker if not in use (if all bindings are deleted), but will only be redeclared if the connection is closed and reopened."); + + ") durable:" + exchange.isDurable() + ", auto-delete:" + exchange.isAutoDelete() + ". " + + "It will be deleted by the broker if it shuts down, and can be redeclared by closing and " + + "reopening the connection."); } } for (Queue queue : queues) { - if (!queue.isDurable()) { - logger.warn("Auto-declaring a non-durable Queue (" + if (!queue.isDurable() || queue.isAutoDelete() || queue.isExclusive()) { + logger.info("Auto-declaring a non-durable, auto-delete, or exclusive Queue (" + queue.getName() - + "). It will be redeclared if the broker stops and is restarted while the connection factory is alive, but all messages will be lost."); - } - if (queue.isAutoDelete()) { - logger.warn("Auto-declaring an auto-delete Queue (" - + queue.getName() - + "). It will be deleted by the broker if not in use, and all messages will be lost. Redeclared when the connection is closed and reopened."); - } - if (queue.isExclusive()) { - logger.warn("Auto-declaring an exclusive Queue (" - + queue.getName() - + "). It cannot be accessed by consumers on another connection, and will be redeclared if the connection is reopened."); + + ") durable:" + queue.isDurable() + ", auto-delete:" + queue.isAutoDelete() + ", exclusive:" + + queue.isExclusive() + ". " + + "It will be redeclared if the broker stops and is restarted while the connection factory is " + + "alive, but all messages will be lost."); } } diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java index 6c32b4f1..3604f71a 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java @@ -14,19 +14,40 @@ package org.springframework.amqp.rabbit.core; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Properties; +import org.apache.commons.logging.Log; +import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.ArgumentCaptor; +import org.mockito.internal.stubbing.answers.DoesNothing; +import org.springframework.amqp.core.DirectExchange; +import org.springframework.amqp.core.Exchange; import org.springframework.amqp.core.Queue; import org.springframework.amqp.rabbit.connection.ConnectionFactory; import org.springframework.amqp.rabbit.connection.SingleConnectionFactory; import org.springframework.amqp.rabbit.test.BrokerRunning; +import org.springframework.amqp.utils.test.TestUtils; +import org.springframework.beans.DirectFieldAccessor; +import org.springframework.context.ApplicationContext; import org.springframework.context.support.GenericApplicationContext; import com.rabbitmq.client.Channel; @@ -119,4 +140,58 @@ public class RabbitAdminTests { assertNotNull(props.get(RabbitAdmin.QUEUE_MESSAGE_COUNT)); return Integer.valueOf((Integer) props.get(RabbitAdmin.QUEUE_MESSAGE_COUNT)); } + + @Test + public void testTemporaryLogs() throws Exception { + SingleConnectionFactory connectionFactory = new SingleConnectionFactory(); + connectionFactory.setHost("localhost"); + RabbitAdmin rabbitAdmin = new RabbitAdmin(connectionFactory); + try { + ApplicationContext ctx = mock(ApplicationContext.class); + Map queues = new HashMap(); + queues.put("nonDurQ", new Queue("testq.nonDur", false, false, false)); + queues.put("adQ", new Queue("testq.ad", true, false, true)); + queues.put("exclQ", new Queue("testq.excl", true, true, false)); + queues.put("allQ", new Queue("testq.all", false, true, true)); + when(ctx.getBeansOfType(Queue.class)).thenReturn(queues); + Map exchanges = new HashMap(); + exchanges.put("nonDurEx", new DirectExchange("testex.nonDur", false, false)); + exchanges.put("adEx", new DirectExchange("testex.ad", true, true)); + exchanges.put("allEx", new DirectExchange("testex.all", false, true)); + when(ctx.getBeansOfType(Exchange.class)).thenReturn(exchanges); + rabbitAdmin.setApplicationContext(ctx); + rabbitAdmin.afterPropertiesSet(); + Log logger = spy(TestUtils.getPropertyValue(rabbitAdmin, "logger", Log.class)); + when(logger.isInfoEnabled()).thenReturn(true); + doAnswer(new DoesNothing()).when(logger).info(anyString()); + new DirectFieldAccessor(rabbitAdmin).setPropertyValue("logger", logger); + connectionFactory.createConnection().close(); // force declarations + ArgumentCaptor log = ArgumentCaptor.forClass(String.class); + verify(logger, times(7)).info(log.capture()); + List logs = log.getAllValues(); + Collections.sort(logs); + assertThat(logs.get(0), Matchers.containsString("(testex.ad) durable:true, auto-delete:true")); + assertThat(logs.get(1), Matchers.containsString("(testex.all) durable:false, auto-delete:true")); + assertThat(logs.get(2), Matchers.containsString("(testex.nonDur) durable:false, auto-delete:false")); + assertThat(logs.get(3), Matchers.containsString("(testq.ad) durable:true, auto-delete:true, exclusive:false")); + assertThat(logs.get(4), Matchers.containsString("(testq.all) durable:false, auto-delete:true, exclusive:true")); + assertThat(logs.get(5), Matchers.containsString("(testq.excl) durable:true, auto-delete:false, exclusive:true")); + assertThat(logs.get(6), Matchers.containsString("(testq.nonDur) durable:false, auto-delete:false, exclusive:false")); + } + finally { + cleanQueuesAndExchanges(rabbitAdmin); + connectionFactory.destroy(); + } + } + + private void cleanQueuesAndExchanges(RabbitAdmin rabbitAdmin) { + rabbitAdmin.deleteQueue("testq.nonDur"); + rabbitAdmin.deleteQueue("testq.ad"); + rabbitAdmin.deleteQueue("testq.excl"); + rabbitAdmin.deleteQueue("testq.all"); + rabbitAdmin.deleteExchange("testex.nonDur"); + rabbitAdmin.deleteExchange("testex.ad"); + rabbitAdmin.deleteExchange("testex.all"); + } + }