spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
328 stars 109 forks source link

Improve documentation #183

Closed xenoterracide closed 6 months ago

xenoterracide commented 6 months ago

Is your feature request related to a problem? Please describe.

Ok, so I'm going to ask the question, in hopes that this will result in a documentation improvement. I always feel that if someone is asking, and it can be answered by documentation, that is the better way; because then the next person can read the documentation instead of repeating the question (you hope).

I'm having a little trouble understanding the reactive example. I think it's because of the interaction with reactor and webclient, and I'm just not that used to reactor, and my brain is trying to map it to other async libraries and languages I've worked with. I think now I know what it does, and the problem I'm having is verbage (retrieve is not our blocking operation). yay for promises without the async keyword.

I'm not certain if, at this exact second if this is what I should do, it seems like the sensible thing.

Mono<?> otherClientCall() // actual http client
cbFactory.create("otherClientCall").run(otherClientCall()).map(...)

Could you add an example of using a client that already uses reactor? and maybe one that uses CompletableFuture and/or CompletionStage, maybe even executors.

To follow on, how does one write a unit test for this? sample is empty

https://github.com/spring-cloud-samples/spring-cloud-circuitbreaker-demo/blob/main/spring-cloud-circuitbreaker-demo-reactive/src/test/java/org/springframework/cloud/circuitbreaker/demo/reactive/SpringCloudCircuitbreakerDemoReactiveApplicationTests.java

The default constructor on Resilience4JCircuitBreakerFactory is marked as deprecated, and the customizer simply takes that class. making me wonder how that works under the hood and why I'm not just creating that type as a bean instead.

ryanjbaxter commented 6 months ago

Does this this guide help in terms of an example? https://spring.io/guides/gs/cloud-circuit-breaker/

xenoterracide commented 6 months ago

No, it has all the same problems I just laid out in my description. It only shows webclient, not a native reactor client, or native java. It doesn't show you how to write tests with it, especially not unit tests to avoid setting up the spring context, even if you did.

ryanjbaxter commented 6 months ago

to avoid setting up the spring context

So are you not using Spring at all or are you using Spring but just not using WebClient? What HTTP client are you using?

xenoterracide commented 6 months ago

We are generating a client with open API that uses web client under the hood and exposes reactor, I'm then wrapping it with a repository like interface. The plan is to put the circuit breaker in the interface. The client is developed in a separate repo from the main spring boot app and so I don't really want to involve spring boot a lot in testing. I'm not really trying to spin up a full app here. The reality is that over time I've used any number of HTTP clients... Web client is nice but I don't see why it would be my only choice. Which is why I'm hoping for a couple more examples.

ryanjbaxter commented 6 months ago

Let me know if this starts to head down the path you are thinking... https://github.com/ryanjbaxter/cb-183/blob/main/src/main/java/org/example/cbsample/CbsampleApplication.java

xenoterracide commented 6 months ago

yeah, that kind of confirms what I wasn't sure of reading the existing sample.

I was thinking, if you were unsure of what to expand on client wise in examples. Seems to me spring now supports 3 of it's own clients (side note: why?!!) and there's cloud openfeign, and lastly the jdk native. This is also a good example.

Although in my tests, it still didn't seem to behave the way I was expecting it to (like the code wasn't doing anything), hence wanting test examples too.

ryanjbaxter commented 6 months ago

So does this help explain how to use it with other clients?

Also note that the reactive API requires you return a Mono or Flux, that doesn't mean the thing you are wrapping necessarily needs to return an Mono or Flux but it needs to be able to be wrapped in one. https://github.com/spring-cloud/spring-cloud-commons/blob/main/spring-cloud-commons/src/main/java/org/springframework/cloud/client/circuitbreaker/ReactiveCircuitBreaker.java

Seems to me spring now supports 3 of it's own clients (side note: why?!!

Why wouldn't we?

and there's cloud openfeign

Spring Cloud OpenFeign uses Spring Cloud Circuit Breaker, not sure what the confusion is there

I think the best way to move forward is to provide me a concrete example of something you are trying to wrap in a circuit breaker that is not working or you or not understanding so I can help fill the gaps.

spring-cloud-issues commented 6 months ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

spring-cloud-issues commented 6 months ago

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.