Adds NPE checks for JMS topics and queues

fixes gh-4505
micrometer-metrics · Dec 22, 2023 · 26afbc2 · 26afbc2
1 parent 2ebfbcf
commit 26afbc2
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 8 deletions.
diff --git a/...in/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java b/...in/java/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConvention.java
@@ -116,19 +116,31 @@ protected KeyValue destinationName(JmsPublishObservationContext context) {
             Destination jmsDestination = message.getJMSDestination();
             if (jmsDestination instanceof Queue) {
                 Queue queue = (Queue) jmsDestination;
-                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, queue.getQueueName());
-            }
-            if (jmsDestination instanceof Topic) {
-                Topic topic = (Topic) jmsDestination;
-                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, topic.getTopicName());
+                String queueName = queue.getQueueName();
+                if (queueName == null) {
+                    return getKeyValueTopic(jmsDestination);
+                }
+                return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, queueName);
             }
-            return DESTINATION_NAME_UNKNOWN;
+            return getKeyValueTopic(jmsDestination);
         }
         catch (JMSException e) {
             return DESTINATION_NAME_UNKNOWN;
         }
     }
 
+    private static KeyValue getKeyValueTopic(Destination jmsDestination) throws JMSException {
+        if (jmsDestination instanceof Topic) {
+            Topic topic = (Topic) jmsDestination;
+            String topicName = topic.getTopicName();
+            if (topicName == null) {
+                return DESTINATION_NAME_UNKNOWN;
+            }
+            return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, topicName);
+        }
+        return DESTINATION_NAME_UNKNOWN;
+    }
+
     protected KeyValue messageId(JmsPublishObservationContext context) {
         try {
             Message message = context.getCarrier();

diff --git a/...va/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java b/...va/io/micrometer/jakarta9/instrument/jms/DefaultJmsPublishObservationConventionTests.java
@@ -20,8 +20,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 /**
  * Tests for {@link DefaultJmsPublishObservationConvention}.
@@ -96,6 +95,13 @@ void shouldHaveTopicDestinationName() throws Exception {
             .contains(KeyValue.of("messaging.destination.name", "micrometer.test.topic"));
     }
 
+    @Test
+    void shouldHaveTopicNullDestinationName() throws Exception {
+        JmsPublishObservationContext context = new JmsPublishObservationContext(createMessageWithNullTopic());
+        assertThat(convention.getHighCardinalityKeyValues(context))
+            .contains(KeyValue.of("messaging.destination.name", "unknown"));
+    }
+
     @Test
     void shouldHaveUnknownDestinationNameWhenException() throws Exception {
         Message message = createMessageWithQueue();
@@ -163,6 +169,23 @@ void shouldHaveTempDestinationForTemporaryTopic() throws Exception {
             .contains(KeyValue.of("messaging.destination.temporary", "true"));
     }
 
+    @Test
+    void shouldHaveTopicDestinationNameEvenWhenTheTopicAlsoImplementsTheQueueInterface() throws Exception {
+        JmsPublishObservationContext context = new JmsPublishObservationContext(
+                createMessageWithTopicThatAlsoImplementsTheQueueInterface());
+
+        assertThat(convention.getHighCardinalityKeyValues(context))
+            .contains(KeyValue.of("messaging.destination.name", "micrometer.test.topic"));
+    }
+
+    private Message createMessageWithTopicThatAlsoImplementsTheQueueInterface() throws Exception {
+        Topic topic = mock(Topic.class, withSettings().extraInterfaces(Queue.class));
+        when(topic.getTopicName()).thenReturn("micrometer.test.topic");
+        Message message = mock(Message.class);
+        when(message.getJMSDestination()).thenReturn(topic);
+        return message;
+    }
+
     private Message createMessageWithQueue() throws Exception {
         Queue queue = mock(Queue.class);
         when(queue.getQueueName()).thenReturn("micrometer.test.queue");
@@ -187,6 +210,14 @@ private Message createMessageWithTopic() throws Exception {
         return message;
     }
 
+    private Message createMessageWithNullTopic() throws Exception {
+        Topic topic = mock(Topic.class);
+        when(topic.getTopicName()).thenReturn(null);
+        Message message = mock(Message.class);
+        when(message.getJMSDestination()).thenReturn(topic);
+        return message;
+    }
+
     private Message createMessageWithTempTopic() throws Exception {
         TemporaryTopic topic = mock(TemporaryTopic.class);
         when(topic.getTopicName()).thenReturn("micrometer.test.topic");