Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to integrate with micrometer out of the box #47

Closed
marcingrzejszczak opened this issue Oct 3, 2019 · 6 comments · Fixed by #51
Closed

Add an option to integrate with micrometer out of the box #47

marcingrzejszczak opened this issue Oct 3, 2019 · 6 comments · Fixed by #51
Assignees
Labels
Projects
Milestone

Comments

@marcingrzejszczak
Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Oct 3, 2019

if I didn't do sth wrong then currently we need to pass quite a few setup for micrometer and resilience4j integration. It would great to have an option to allow adding a customization to any circuitbreaker (regardless of the id).

This is how I worked this around ATM:

pom.xml


		<dependency>
			<groupId>io.micrometer</groupId>
			<artifactId>micrometer-registry-prometheus</artifactId>
			<version>${micrometer-registry-prometheus.version}</version>
		</dependency>
<!-- I had to manually pass the version of resilience4j -->
		<dependency>
			<groupId>io.github.resilience4j</groupId>
			<artifactId>resilience4j-micrometer</artifactId>
			<version>${resilience4j.version}</version>
		</dependency>

Configuration class

@Bean
	Customizer<Resilience4JCircuitBreakerFactory> defaultCustomizer(CircuitBreakerRegistry circuitBreakerRegistry, MeterRegistry meterRegistry) {
		return factory -> {
			factory.configureDefault(id -> new Resilience4JConfigBuilder(id)
					.timeLimiterConfig(TimeLimiterConfig.custom()
							.timeoutDuration(Duration.ofSeconds(2))
							.build())
					.circuitBreakerConfig(CircuitBreakerConfig.ofDefaults())
					.build());
			// for metrics
			factory.configureCircuitBreakerRegistry(circuitBreakerRegistry);
			// we need to allow adding those customizers regardless of the id
			factory.addCircuitBreakerCustomizer(circuitBreaker -> {
				CircuitBreakerMetrics
						.ofCircuitBreakerRegistry(circuitBreakerRegistry)
						.bindTo(meterRegistry);
			}, "verifyNewUser");
		};
	}

	@Bean
	CircuitBreakerRegistry resilience4JCircuitBreakerRegistry() {
		return CircuitBreakerRegistry.ofDefaults();
	}

Circuit breaker

circuitBreakerFactory.create("verifyNewUser")
					.run(() -> restTemplate.getForObject(uriComponentsBuilder.toUriString(),
							VerificationResult.class), throwable -> userRejected(userUuid, userAge));
@making

This comment has been minimized.

Copy link
Contributor

@making making commented Oct 4, 2019

I’ve ever also digged into monitoring resillience4j with spring cloud circuit breaker. The next two points were concerns.

  1. addCircuitBreakerCustomizer requires all CircuitBreaker's ids. CircuitBreakerRegistry.getAllCircuitBreaker is the method to get all CBs. However, it’s difficult to retrieve all at the initializing phase. getAllCircuitBreaker will return empty list at the init phase as spring cloud circuit breaker creates and register a circuit breaker in the registry at the actual execution phase. So we need to put all known ids to addCircuitBreakerCustomizer in the configuration in advance...
  2. TaggedCircuitBreakerMetrics.bindTo(MeterRegistry) calls CircuitBreakerRegistry.getAllCircuitBreaker above and addMetrics to each CB. If we call TaggedCircuitBreakerMetrics.bindTo in the Customizer<CircuitBreaker>, which is the only customizable point as far as I know, every time when a CB is created, addMetrics for all CBs will be called. It’s quite inefficient. I’m not sure it really works.

With these restrictions, I came down to the implementation like following at that time,

	@Bean
	public Customizer<ReactiveResilience4JCircuitBreakerFactory> resilience4jCustomizer(
			MeterRegistry registry, BlogProperties props) {
		ConcurrentMap<String, Boolean> registered = new ConcurrentHashMap<>();
		return factory -> {
			factory.addCircuitBreakerCustomizer(circuitBreaker -> {
				this.configureMetrics(registry, circuitBreaker, registered);
				this.configureEventPublisher(circuitBreaker.getEventPublisher());
			}, "myCb1", "myCb2", "myCb3", "myCb4", "myCb5" /* ... */);
			// ommited
		};
	}


	private void configureMetrics(MeterRegistry registry, CircuitBreaker circuitBreaker,
			ConcurrentMap<String, Boolean> registered) {
		TaggedCircuitBreakerMetrics.MetricNames names = TaggedCircuitBreakerMetrics.MetricNames
				.ofDefaults();
		String circuitBreakerName = circuitBreaker.getName();
		if (!registered.containsKey(circuitBreakerName)) {
			registered.putIfAbsent(circuitBreakerName, true);
			log.info("Register metric for {}", circuitBreaker);
			// Copy from AbstractCircuitBreakerMetrics
			Gauge.builder(names.getStateMetricName(), circuitBreaker,
					(cb) -> cb.getState().getOrder())
					.tag(TagNames.NAME, circuitBreakerName).register(registry);
			Gauge.builder(names.getCallsMetricName(), circuitBreaker,
					(cb) -> cb.getMetrics().getNumberOfFailedCalls())
					.tag(TagNames.NAME, circuitBreakerName).tag(TagNames.KIND, "failed")
					.register(registry);
			Gauge.builder(names.getCallsMetricName(), circuitBreaker,
					(cb) -> cb.getMetrics().getNumberOfNotPermittedCalls())
					.tag(TagNames.NAME, circuitBreakerName)
					.tag(TagNames.KIND, "not_permitted").register(registry);
			Gauge.builder(names.getCallsMetricName(), circuitBreaker,
					(cb) -> cb.getMetrics().getNumberOfSuccessfulCalls())
					.tag(TagNames.NAME, circuitBreakerName)
					.tag(TagNames.KIND, "successful").register(registry);
			Gauge.builder(names.getBufferedCallsMetricName(), circuitBreaker,
					(cb) -> cb.getMetrics().getNumberOfBufferedCalls())
					.tag(TagNames.NAME, circuitBreakerName).register(registry);
			Gauge.builder(names.getMaxBufferedCallsMetricName(), circuitBreaker,
					(cb) -> cb.getMetrics().getMaxNumberOfBufferedCalls())
					.tag(TagNames.NAME, circuitBreakerName).register(registry);
		}
	}

When using resillience4j directly without spring cloud circuit breaker, it’s usual to create a CircuitBreaker at the initializing phase. Then we can achieve register all CBs to MeterRegistry as follows:

	@Bean
	public InitializingBean init(MeterRegistry meterRegistry, CircuitBreakerRegistry circuitBreakerRegistry) {
		return () -> TaggedCircuitBreakerMetrics.ofCircuitBreakerRegistry(circuitBreakerRegistry).bindTo(meterRegistry);
	}
@making

This comment has been minimized.

Copy link
Contributor

@making making commented Oct 5, 2019

I've created a simple project

Here is a project that uses resillience4j without spring cloud circuit breaker
https://github.com/making/demo-sccb-gh47/tree/master/demo-webflux-r4j
It looks easier to give the out of the box support for micrometer

To get the same experience with spring cloud circuit breaker, a project looks like
https://github.com/making/demo-sccb-gh47/tree/master/demo-webflux-sccb-r4j
Seems to need to change something in spring cloud circuit breaker

@ryanjbaxter ryanjbaxter self-assigned this Oct 8, 2019
@ryanjbaxter ryanjbaxter added this to To do in Hoxton.RC1 via automation Oct 8, 2019
@ryanjbaxter ryanjbaxter added this to the 1.0.0.M2 milestone Oct 8, 2019
@ryanjbaxter

This comment has been minimized.

Copy link
Collaborator

@ryanjbaxter ryanjbaxter commented Oct 15, 2019

@making @marcingrzejszczak

I could see making this simpler, but I am really curious about @making's experience

I could see metrics for my CB just by doing the following

@Bean
	public Customizer<Resilience4JCircuitBreakerFactory> defaultCustomizer(MeterRegistry meterRegistry) {
		CircuitBreakerRegistry cbr = CircuitBreakerRegistry.ofDefaults();
		TaggedCircuitBreakerMetrics.ofCircuitBreakerRegistry(cbr).bindTo(meterRegistry);
		return factory -> {
			factory.configureDefault(id -> new Resilience4JConfigBuilder(id)
					.timeLimiterConfig(TimeLimiterConfig.custom().timeoutDuration(Duration.ofSeconds(3)).build())
					.circuitBreakerConfig(CircuitBreakerConfig.ofDefaults())
					.build());
			factory.configureCircuitBreakerRegistry(cbr);
		};
	}

This was in a non-reactive app, is the experience with a reactive app different?

ryanjbaxter added a commit to ryanjbaxter/spring-cloud-circuitbreaker that referenced this issue Oct 21, 2019
@making

This comment has been minimized.

Copy link
Contributor

@making making commented Oct 22, 2019

This was in a non-reactive app, is the experience with a reactive app different?

I've confirmed my sample worked after applying same changes in #51 👍

What I misunderstood was that all circuit breakers must be registered when TaggedCircuitBreakerMetrics is created but It is not necessary.
https://github.com/resilience4j/resilience4j/blob/master/resilience4j-micrometer/src/main/java/io/github/resilience4j/micrometer/tagged/TaggedCircuitBreakerMetrics.java#L64

@ryanjbaxter

This comment has been minimized.

Copy link
Collaborator

@ryanjbaxter ryanjbaxter commented Oct 22, 2019

@making thanks for confirming!

Hoxton.RC1 automation moved this from To do to Done Oct 22, 2019
@RobWin

This comment has been minimized.

Copy link

@RobWin RobWin commented Oct 26, 2019

If you have questions about Resilience4j, don't hesitate to ping us.
Did you test if your auto configuration works together with Spring Cloud Config? We added some new classes to support it, like https://github.com/resilience4j/resilience4j/blob/master/resilience4j-micrometer/src/main/java/io/github/resilience4j/micrometer/tagged/TaggedCircuitBreakerMetricsPublisher.java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Hoxton.RC1
  
Done
4 participants
You can’t perform that action at this time.