Azure / azure-sdk-for-java

This repository is for active development of the Azure SDK for Java. For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/java/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-java.
MIT License
2.35k stars 1.98k forks source link

[FEATURE REQ] Provide reactive health indicators for Spring Cloud Azure #36309

Open paul-kraftlauget opened 1 year ago

paul-kraftlauget commented 1 year ago

Describe the bug

When using a WebFlux application, health indicators should extend org.springframework.boot.actuate.health.AbstractReactiveHealthIndicator and avoid calling Mono.block().

Furthermore, even if refactored, blockhound reports some blocking calls during the health check that need to be removed (for example, StorageBlobHealthIndicator, StorageQueueHealthIndicator, see stacktrace below).

Exception or Stack Trace

Original Stack Trace:
Caused by: reactor.blockhound.BlockingOperationError: Blocking call! java.io.RandomAccessFile#readBytes
    at java.base/java.io.RandomAccessFile.readBytes(RandomAccessFile.java)
    Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below: 
Assembly trace from producer [reactor.core.publisher.MonoHandle] :
    reactor.core.publisher.Mono.mapNotNull
    com.azure.core.implementation.http.rest.AsyncRestProxy.handleBodyReturnType(AsyncRestProxy.java:200)
Error has been observed at the following site(s):
    *_______Mono.mapNotNull ⇢ at com.azure.core.implementation.http.rest.AsyncRestProxy.handleBodyReturnType(AsyncRestProxy.java:200)
    |_             Mono.map ⇢ at com.azure.core.implementation.http.rest.AsyncRestProxy.handleRestResponseReturnType(AsyncRestProxy.java:148)
    |_   Mono.switchIfEmpty ⇢ at com.azure.core.implementation.http.rest.AsyncRestProxy.handleRestResponseReturnType(AsyncRestProxy.java:149)
    |_                      ⇢ at com.azure.core.implementation.http.rest.AsyncRestProxy.lambda$handleRestReturnType$9(AsyncRestProxy.java:230)
    *__________Mono.flatMap ⇢ at com.azure.core.implementation.http.rest.AsyncRestProxy.handleRestReturnType(AsyncRestProxy.java:229)
    |_                      ⇢ at com.azure.storage.queue.implementation.ServicesImpl.getPropertiesWithResponseAsync(ServicesImpl.java:444)
    |_             Mono.map ⇢ at com.azure.storage.queue.QueueServiceAsyncClient.getPropertiesWithResponse(QueueServiceAsyncClient.java:428)
    |_                      ⇢ at com.azure.core.util.FluxUtil.lambda$withContext$9(FluxUtil.java:401)
    *__Mono.deferContextual ⇢ at com.azure.core.util.FluxUtil.withContext(FluxUtil.java:389)
    |_                      ⇢ at com.azure.core.util.FluxUtil.withContext(FluxUtil.java:370)
    |_                      ⇢ at com.azure.storage.queue.QueueServiceAsyncClient.getPropertiesWithResponse(QueueServiceAsyncClient.java:419)
...
Original Stack Trace:
        at java.base/java.io.RandomAccessFile.readBytes(RandomAccessFile.java)
        at java.base/java.io.RandomAccessFile.read(RandomAccessFile.java:405)
        at java.base/java.io.RandomAccessFile.readFully(RandomAccessFile.java:469)
        at java.base/java.util.zip.ZipFile$Source.readFullyAt(ZipFile.java:1348)
        at java.base/java.util.zip.ZipFile$ZipFileInputStream.initDataOffset(ZipFile.java:915)
        at java.base/java.util.zip.ZipFile$ZipFileInputStream.read(ZipFile.java:931)
        at java.base/java.util.zip.ZipFile$ZipFileInflaterInputStream.fill(ZipFile.java:448)
        at java.base/java.util.zip.InflaterInputStream.read(InflaterInputStream.java:158)
        at java.base/java.io.InputStream.readNBytes(InputStream.java:506)
        at java.base/java.util.jar.JarFile.getBytes(JarFile.java:812)
        at java.base/java.util.jar.JarFile.checkForSpecialAttributes(JarFile.java:1002)
        at java.base/java.util.jar.JarFile.isMultiRelease(JarFile.java:389)
        at java.base/java.util.jar.JarFile.getEntry(JarFile.java:511)
        at java.base/sun.net.www.protocol.jar.URLJarFile.getEntry(URLJarFile.java:131)
        at java.base/sun.net.www.protocol.jar.JarURLConnection.connect(JarURLConnection.java:135)
        at java.base/sun.net.www.protocol.jar.JarURLConnection.getInputStream(JarURLConnection.java:175)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.parse(ServiceLoader.java:1172)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.nextProviderClass(ServiceLoader.java:1213)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1228)
        at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1273)
        at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
        at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1393)
        at java.xml/javax.xml.stream.FactoryFinder$1.run(FactoryFinder.java:350)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
        at java.xml/javax.xml.stream.FactoryFinder.findServiceProvider(FactoryFinder.java:339)
        at java.xml/javax.xml.stream.FactoryFinder.find(FactoryFinder.java:310)
        at java.xml/javax.xml.stream.XMLInputFactory.newFactory(XMLInputFactory.java:288)
        at com.fasterxml.jackson.dataformat.xml.util.StaxUtil.defaultInputFactory(StaxUtil.java:128)
        at com.fasterxml.jackson.dataformat.xml.XmlFactory.<init>(XmlFactory.java:123)
        at com.fasterxml.jackson.dataformat.xml.XmlFactory.<init>(XmlFactory.java:110)
        at com.fasterxml.jackson.dataformat.xml.XmlFactory.<init>(XmlFactory.java:103)
        at com.fasterxml.jackson.dataformat.xml.XmlFactory.<init>(XmlFactory.java:87)
        at com.fasterxml.jackson.dataformat.xml.XmlMapper.<init>(XmlMapper.java:135)
        at com.fasterxml.jackson.dataformat.xml.XmlMapper.builder(XmlMapper.java:226)
        at com.azure.core.implementation.jackson.XmlMapperFactory.createXmlMapper(XmlMapperFactory.java:88)
        at com.azure.core.implementation.jackson.ObjectMapperFactory.createXmlMapper(ObjectMapperFactory.java:42)
        at com.azure.core.implementation.jackson.ObjectMapperShim.createXmlMapper(ObjectMapperShim.java:78)
        at com.azure.core.util.serializer.JacksonAdapter$GlobalXmlMapper.<clinit>(JacksonAdapter.java:59)
        at com.azure.core.util.serializer.JacksonAdapter.getXmlMapper(JacksonAdapter.java:462)
        at com.azure.core.util.serializer.JacksonAdapter.lambda$deserialize$8(JacksonAdapter.java:343)
        at com.azure.core.util.serializer.JacksonAdapter.useAccessHelper(JacksonAdapter.java:483)
        at com.azure.core.util.serializer.JacksonAdapter.deserialize(JacksonAdapter.java:338)
        at com.azure.core.implementation.serializer.HttpResponseBodyDecoder.deserialize(HttpResponseBodyDecoder.java:166)
        at com.azure.core.implementation.serializer.HttpResponseBodyDecoder.deserializeBody(HttpResponseBodyDecoder.java:139)
        at com.azure.core.implementation.serializer.HttpResponseBodyDecoder.decodeByteArray(HttpResponseBodyDecoder.java:82)
        at com.azure.core.implementation.serializer.HttpResponseDecoder$HttpDecodedResponse.getDecodedBody(HttpResponseDecoder.java:93)
        at reactor.core.publisher.Mono.lambda$mapNotNull$25(Mono.java:3432)
        at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:113)

To Reproduce

Steps to reproduce the behavior:

Change health indicators to extend org.springframework.boot.actuate.health.AbstractReactiveHealthIndicator and add blockhound library to test classpath to discover blocking invocation.

Expected behavior

The Azure SDK should automatically discover a WebFlux application and initialize reactive health indicators for all health indicators. An application using both Azure SDK and blockhound should not fail on azure SDK's blocking implementations (for example, load classes containing blocking calls during spring startup and not during health indicator execution).

joshfree commented 1 year ago

@srnagar could you please follow up?

srnagar commented 1 year ago

I have fixed the blocking calls reported in AsyncRestProxy in this PR.

@saragluna could you please take a look at updating the health indicators to use AbstractReactiveHealthIndicator in Spring libraries?

saragluna commented 1 year ago

Thanks @srnagar!

saragluna commented 1 year ago

Hi @paul-kraftlauget, are you defining your own AzureXXXReactiveHealthIndicator classes in your project? Why I am asking this is to decide whether to switch our health indicator to reactive implementations.

Referring to Spring Boot's configuration for imperative and reactive health indicators, they will inject the reactive health indicator beans if the Flux.class is on the classpath. For Azure's case, azure-core depends on reactor-core, which means the Flux.class will always be there.

See https://github.com/spring-projects/spring-boot/issues/29107, and https://github.com/spring-projects/spring-boot/issues/22692.

Defaulting to reactive won't be an issue. But if users want to explicitly use the imperative implementation, they need to provide the bean.

paul-kraftlauget commented 1 year ago

Sorry for the late reply.

Yes, I have seen the way Spring has chosen to do the autoconfiguration discovery, and just including azure SDK already activates all the reactive health indicators then. My application is actually sort of a half / half application as input is not received from REST but from other sources (JMS). It doesn't use WebFlux, but it is a fully reactive application when processing the messages. My unit tests for the entire Spring application context include reading actuator's health check and these unit tests failed with blockhound enabled. Any custom health indicators I have written for the application have been reactive health indicators, which work just fine even when the application is not WebFlux.

So then, I would suggest to just follow Spring's implementation pattern here.

If you want to give people the option, just add an 'enabled' config flag so reactive health indicators can be disabled.

saragluna commented 1 year ago

@paul-kraftlauget, so I will add this feature request to our backlog since based on what I have read, the issue blocking your project has been resolved.