java.lang.NullPointerException in DefaultJmsPublishObservationConvention

[toellrich]

Since upgrading to Spring Boot 3.2.0 we get a NullPointerException.

Environment

• Micrometer version: 1.12.0
• OS: Windows
• Java version:
  openjdk version "21" 2023-09-19 LTS
  OpenJDK Runtime Environment Temurin-21+35 (build 21+35-LTS)
  OpenJDK 64-Bit Server VM Temurin-21+35 (build 21+35-LTS, mixed mode, sharing)

To Reproduce
Our JMS provider is called SwiftMQ. They have a class com.swiftmq.jms.TopicImpl that not only implements the Topic interface, but also the Queue interface (don't ask me why). See the following Maven coordinate for their code: com.swiftmq:swiftmq-client-jakarta:12.5.4.
This leads to the follwoing NullPointerException in your code:

java.lang.NullPointerException: null
	at java.base/java.util.Objects.requireNonNull(Objects.java:233)
	at io.micrometer.common.ImmutableKeyValue.(ImmutableKeyValue.java:38)
	at io.micrometer.common.KeyValue.of(KeyValue.java:48)
	at io.micrometer.common.KeyValue.of(KeyValue.java:58)
	at io.micrometer.jakarta9.instrument.jms.DefaultJmsPublishObservationConvention.destinationName(DefaultJmsPublishObservationConvention.java:119)
	at io.micrometer.jakarta9.instrument.jms.DefaultJmsPublishObservationConvention.getHighCardinalityKeyValues(DefaultJmsPublishObservationConvention.java:94)
	at io.micrometer.jakarta9.instrument.jms.DefaultJmsPublishObservationConvention.getHighCardinalityKeyValues(DefaultJmsPublishObservationConvention.java:31)
	at io.micrometer.observation.SimpleObservation.stop(SimpleObservation.java:175)
	at io.micrometer.jakarta9.instrument.jms.MessageProducerInvocationHandler.invoke(MessageProducerInvocationHandler.java:70)
	at jdk.proxy2/jdk.proxy2.$Proxy136.send(Unknown Source)
	at org.springframework.jms.core.JmsTemplate.doSend(JmsTemplate.java:660)
	at org.springframework.jms.core.JmsTemplate.doSend(JmsTemplate.java:634)
	at org.springframework.jms.core.JmsTemplate.lambda$send$2(JmsTemplate.java:603)
	at org.springframework.jms.core.JmsTemplate.execute(JmsTemplate.java:530)
	at org.springframework.jms.core.JmsTemplate.send(JmsTemplate.java:602)
	at org.springframework.jms.core.JmsTemplate.convertAndSend(JmsTemplate.java:706)
        ...

The problem is that you check whether the destination is an instance of Queue and when it is, you cast to Queue and call the method getQueueName which in our case returns null.

Expected behavior
No NullPointerException. Could your code be re-written along the following lines?

  if (jmsDestination instanceof Queue queue && queue.getQueueName() != null) {
    return KeyValue.of(HighCardinalityKeyNames.DESTINATION_NAME, queue.getQueueName());
  }

Please let me know if you need a bit of code in order to reproduce the issue. I'd be more than happy to provide it to you,

[toellrich]

The code of our JMS provider can be found here.

[marcingrzejszczak]

Yes, I think we should do a null check and only set the value if it's not null. Are you willing to file a PR (preferably with a failing test?). Remember that we have to use JDK8 features at most :)

[toellrich]

Yes, I think we should do a null check and only set the value if it's not null. Are you willing to file a PR (preferably with a failing test?). Remember that we have to use JDK8 features at most :)

Will do. For the test, do you want me to mock the destination or should I actually add the dependency to SwiftMQ?

[marcingrzejszczak]

No, no, it's enough to stub it out.

[toellrich]

Sorry, no can do. I just checked out Micrometer and the Eclipse project is full of errors. That said, I do have a workaround for the bug, so I was still able to upgrade to Spring Boot 3.2.0:

  <dependency>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter-actuator</artifactId>
    <exclusions>
      <exclusion>
        <groupId>io.micrometer</groupId>
        <artifactId>micrometer-jakarta9</artifactId>			
      </exclusion>	  
    </exclusions>
  </dependency>

I can still write you a failing unit test. Let me get on that.

[toellrich]

Here's the test:

package io.micrometer.jakarta9.instrument.jms;

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.withSettings;

import io.micrometer.common.KeyValue;
import jakarta.jms.Message;
import jakarta.jms.Queue;
import jakarta.jms.Topic;
import org.junit.jupiter.api.Test;

class DefaultJmsPublishObservationConventionTests {

  private final DefaultJmsPublishObservationConvention convention = new DefaultJmsPublishObservationConvention();

  /**
   * @see https://github.com/micrometer-metrics/micrometer/issues/4505
   */
  @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;
  }
}

[marcingrzejszczak]

Thank you for the snippets, I've added also a NPE guard against queue names

[toellrich]

The same problem occurs in io.micrometer.jakarta9.instrument.jms.DefaultJmsProcessObservationConvention line 113 (version 1.12.4). Shall I create a new issue or should this one be reopened?

[shakuzen]

Thanks for letting us know. A new issue will be better for tracking. I've opened #4966