diff --git a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md index 7d749aec3ec8e..05e6a3776b841 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md +++ b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md @@ -12,6 +12,9 @@ ### Bugs Fixed - Fixed exception when attempting to populate trace context on received `EventData`. ([#33594](https://github.com/Azure/azure-sdk-for-java/issues/33594)) +- Fixed `NullPointerException` when ending span when `AmqpException` is thrown, but its `AmqpErrorCondition` is `null`. + ([#35299](https://github.com/Azure/azure-sdk-for-java/issues/35299)) + ### Other Changes diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracer.java index 37f052651c4f0..9d6fba525e39b 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracer.java @@ -83,7 +83,9 @@ public void endSpan(Throwable throwable, Context span, AutoCloseable scope) { String errorCondition = null; if (throwable instanceof AmqpException) { AmqpException exception = (AmqpException) throwable; - errorCondition = exception.getErrorCondition().getErrorCondition(); + if (exception.getErrorCondition() != null) { + errorCondition = exception.getErrorCondition().getErrorCondition(); + } } try { diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracerTests.java b/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracerTests.java new file mode 100644 index 0000000000000..c055307dd963b --- /dev/null +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/implementation/instrumentation/EventHubsTracerTests.java @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.messaging.eventhubs.implementation.instrumentation; + +import com.azure.core.amqp.exception.AmqpErrorCondition; +import com.azure.core.amqp.exception.AmqpException; +import com.azure.core.util.Context; +import com.azure.core.util.tracing.Tracer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class EventHubsTracerTests { + @Test + public void testSpanEndNoError() { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + EventHubsTracer tracer = new EventHubsTracer(inner, "fqdn", "entityPath"); + tracer.endSpan(null, Context.NONE, null); + + verify(inner, times(1)).end(isNull(), isNull(), same(Context.NONE)); + } + + @Test + public void testSpanEndNoErrorAndScope() { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + EventHubsTracer tracer = new EventHubsTracer(inner, "fqdn", "entityPath"); + AtomicBoolean closed = new AtomicBoolean(); + tracer.endSpan(null, Context.NONE, () -> closed.set(true)); + + verify(inner, times(1)).end(isNull(), isNull(), same(Context.NONE)); + assertTrue(closed.get()); + } + + @ParameterizedTest + @MethodSource("getAmqpException") + public void testSpanEndException(Exception amqpException, String expectedStatus) { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + EventHubsTracer tracer = new EventHubsTracer(inner, "fqdn", "entityPath"); + + tracer.endSpan(amqpException, Context.NONE, null); + + verify(inner, times(1)).end(eq(expectedStatus), eq(amqpException), same(Context.NONE)); + } + + public static Stream getAmqpException() { + return Stream.of( + Arguments.of(new RuntimeException("foo"), null), + Arguments.of(new AmqpException(false, "foo", null, null), null), + Arguments.of(new AmqpException(false, AmqpErrorCondition.NOT_FOUND, "foo", null), AmqpErrorCondition.NOT_FOUND.getErrorCondition()), + Arguments.of(new AmqpException(false, AmqpErrorCondition.TIMEOUT_ERROR, "", null), AmqpErrorCondition.TIMEOUT_ERROR.getErrorCondition()), + Arguments.of(new AmqpException(false, AmqpErrorCondition.SERVER_BUSY_ERROR, null, new RuntimeException("foo"), null), AmqpErrorCondition.SERVER_BUSY_ERROR.getErrorCondition())); + } +} diff --git a/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md b/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md index 3ea21d7078dec..48d6c33715bf8 100644 --- a/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md +++ b/sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md @@ -8,6 +8,9 @@ ### Bugs Fixed +- Fixed `NullPointerException` when ending span when `AmqpException` is thrown, but its `AmqpErrorCondition` is `null`. + ([#35299](https://github.com/Azure/azure-sdk-for-java/issues/35299)) + ### Other Changes ## 7.14.0 (2023-05-15) diff --git a/sdk/servicebus/azure-messaging-servicebus/pom.xml b/sdk/servicebus/azure-messaging-servicebus/pom.xml index 72bc413cc5251..1d2574115118a 100644 --- a/sdk/servicebus/azure-messaging-servicebus/pom.xml +++ b/sdk/servicebus/azure-messaging-servicebus/pom.xml @@ -43,6 +43,7 @@ --add-opens com.azure.messaging.servicebus/com.azure.messaging.servicebus.administration.models=ALL-UNNAMED --add-exports com.azure.core/com.azure.core.implementation.util=ALL-UNNAMED + --add-exports com.azure.core/com.azure.core.implementation.instrumentation=ALL-UNNAMED --add-opens com.azure.core/com.azure.core.implementation.util=ALL-UNNAMED --add-reads com.azure.messaging.servicebus=com.azure.http.netty --add-reads com.azure.messaging.servicebus=com.azure.core.tracing.opentelemetry diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracer.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracer.java index f7363b6859732..a31314a7d8cf9 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracer.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracer.java @@ -127,7 +127,9 @@ public void endSpan(Throwable throwable, Context span, AutoCloseable scope) { String errorCondition = null; if (throwable instanceof AmqpException) { AmqpException exception = (AmqpException) throwable; - errorCondition = exception.getErrorCondition().getErrorCondition(); + if (exception.getErrorCondition() != null) { + errorCondition = exception.getErrorCondition().getErrorCondition(); + } } try { diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracerTests.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracerTests.java new file mode 100644 index 0000000000000..bdf827931cb3b --- /dev/null +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/implementation/instrumentation/ServiceBusTracerTests.java @@ -0,0 +1,73 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.messaging.servicebus.implementation.instrumentation; + +import com.azure.core.amqp.exception.AmqpErrorCondition; +import com.azure.core.amqp.exception.AmqpException; +import com.azure.core.util.Context; +import com.azure.core.util.tracing.Tracer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ServiceBusTracerTests { + @Test + public void testSpanEndNoError() { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + ServiceBusTracer tracer = new ServiceBusTracer(inner, "fqdn", "entityPath"); + tracer.endSpan(null, Context.NONE, null); + + verify(inner, times(1)).end(isNull(), isNull(), same(Context.NONE)); + } + + @Test + public void testSpanEndNoErrorAndScope() { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + ServiceBusTracer tracer = new ServiceBusTracer(inner, "fqdn", "entityPath"); + AtomicBoolean closed = new AtomicBoolean(); + tracer.endSpan(null, Context.NONE, () -> closed.set(true)); + + verify(inner, times(1)).end(isNull(), isNull(), same(Context.NONE)); + assertTrue(closed.get()); + } + + @ParameterizedTest + @MethodSource("getAmqpException") + public void testSpanEndException(Exception amqpException, String expectedStatus) { + Tracer inner = mock(Tracer.class); + when(inner.isEnabled()).thenReturn(true); + + ServiceBusTracer tracer = new ServiceBusTracer(inner, "fqdn", "entityPath"); + + tracer.endSpan(amqpException, Context.NONE, null); + + verify(inner, times(1)).end(eq(expectedStatus), eq(amqpException), same(Context.NONE)); + } + + public static Stream getAmqpException() { + return Stream.of( + Arguments.of(new RuntimeException("foo"), null), + Arguments.of(new AmqpException(false, "foo", null, null), null), + Arguments.of(new AmqpException(false, AmqpErrorCondition.NOT_FOUND, "foo", null), AmqpErrorCondition.NOT_FOUND.getErrorCondition()), + Arguments.of(new AmqpException(false, AmqpErrorCondition.TIMEOUT_ERROR, "", null), AmqpErrorCondition.TIMEOUT_ERROR.getErrorCondition()), + Arguments.of(new AmqpException(false, AmqpErrorCondition.SERVER_BUSY_ERROR, null, new RuntimeException("foo"), null), AmqpErrorCondition.SERVER_BUSY_ERROR.getErrorCondition())); + } +}