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.

Sign up for GitHub

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

Jump to bottom

feat: enable TcpClient metrics #298

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

feat: enable TcpClient metrics #298

wants to merge 1 commit into from

Conversation

rxdcxdrnine
Copy link

Motivation:
cannot check if LoopResources event loop has pending tasks with metrics.

Modification:
add TcpClient.metrics(true) with if statements, check if micrometer is available.

Result:
expose the metrics of LoopResources including reactor_netty_event_loop_pending_tasks.
The following is a list of metrics exposed when the commit was applied (with default TcpResources)

reactor_netty_eventloop_pending_tasks{name="reactor-tcp-nio-4"} 0.0
reactor_netty_eventloop_pending_tasks{name="reactor-tcp-nio-5"} 0.0
reactor_netty_eventloop_pending_tasks{name="reactor-tcp-nio-6"} 0.0
reactor_netty_eventloop_pending_tasks{name="reactor-tcp-nio-7"} 0.0
reactor_netty_eventloop_pending_tasks{name="reactor-tcp-nio-8"} 0.0
reactor_netty_tcp_client_address_resolver_seconds_count{remote_address="localhost:64921",status="SUCCESS"} 100
reactor_netty_tcp_client_address_resolver_seconds_sum{remote_address="localhost:64921",status="SUCCESS"} 1.095592293
reactor_netty_tcp_client_address_resolver_seconds_max{remote_address="localhost:64921",status="SUCCESS"} 0.01396475
reactor_netty_tcp_client_connect_time_seconds_count{proxy_address="na",remote_address="localhost:64921",status="SUCCESS"} 100
reactor_netty_tcp_client_connect_time_seconds_sum{proxy_address="na",remote_address="localhost:64921",status="SUCCESS"} 3.49526084
reactor_netty_tcp_client_connect_time_seconds_max{proxy_address="na",remote_address="localhost:64921",status="SUCCESS"} 0.039677708
reactor_netty_tcp_client_data_received_bytes_count{proxy_address="na",remote_address="localhost:64921",uri="tcp"} 814
reactor_netty_tcp_client_data_received_bytes_sum{proxy_address="na",remote_address="localhost:64921",uri="tcp"} 325900.0
reactor_netty_tcp_client_data_received_bytes_max{proxy_address="na",remote_address="localhost:64921",uri="tcp"} 2048.0
reactor_netty_tcp_client_data_sent_bytes_count{proxy_address="na",remote_address="localhost:64921",uri="tcp"} 1200
reactor_netty_tcp_client_data_sent_bytes_sum{proxy_address="na",remote_address="localhost:64921",uri="tcp"} 108700.0

@rxdcxdrnine rxdcxdrnine changed the title enable TcpClient metrics feat: enable TcpClient metrics Jan 18, 2025
@jchrys jchrys added this to the 1.3.2 milestone Jan 18, 2025
@jchrys jchrys added the enhancement New feature or request label Jan 18, 2025
@jchrys jchrys self-assigned this Jan 18, 2025
jchrys
jchrys reviewed Jan 20, 2025
View reviewed changes
r2dbc-mysql/src/main/java/io/asyncer/r2dbc/mysql/client/Client.java
@@ -155,6 +156,10 @@ static Mono<Client> connect(MySqlSslConfiguration ssl, SocketAddress address, bo
tcpClient = tcpClient.resolver(resolver);
}

if (Metrics.isMicrometerAvailable()) {
Copy link
Collaborator

@jchrys jchrys Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove Metrics.isMicrometerAvailable(). Directly enabling metrics with tcpClient.metrics(true) simplifies the code and avoids using Reactor Netty's internal utilities.

Copy link
Author

@rxdcxdrnine rxdcxdrnine Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jchrys Thanks for checking my PR!
I think this if statement is required.

TcpClient extends reactor.netty.transport.Transport, and TcpClient.metrics(true) would call Transport.metrics(true). Calling TcpClient.metrics(true) without checking Metrics.isMicrometerAvailable() may cause exception in Transport.metrics(true) if micrometer-core dependency is not added to the classpath.

https://github.com/reactor/reactor-netty/blob/main/reactor-netty-core/src/main/java/reactor/netty/transport/Transport.java#L138-L142


I simulated this situation with the demo project.
https://github.com/rxdcxdrnine/r2dbc-mysql-client-metrics-demo/tree/without_micrometer_core

If I remove spring-boot-starter-actuator dependency from project which includes micrometer-core
and also remove checking Metrics.isMicrometerAvailable() in Client, I got the error log below.

error log 
reactor.core.Exceptions$ErrorCallbackNotImplemented: org.springframework.dao.DataAccessResourceFailureException: Failed to obtain R2DBC Connection
Caused by: org.springframework.dao.DataAccessResourceFailureException: Failed to obtain R2DBC Connection
	at org.springframework.r2dbc.connection.ConnectionFactoryUtils.lambda$getConnection$0(ConnectionFactoryUtils.java:100) ~[spring-r2dbc-6.2.1.jar:6.2.1]
	at reactor.core.publisher.Mono.lambda$onErrorMap$29(Mono.java:3862) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onError(FluxOnErrorResume.java:94) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onError(FluxOnErrorResume.java:106) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onError(MonoFlatMap.java:180) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Operators.error(Operators.java:198) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:162) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoFlatMap.subscribeOrReturn(MonoFlatMap.java:53) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:63) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4576) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onError(FluxOnErrorResume.java:103) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoFlatMap$FlatMapMain.onError(MonoFlatMap.java:180) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxMap$MapSubscriber.onError(FluxMap.java:134) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Operators.error(Operators.java:198) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoError.subscribe(MonoError.java:53) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoDeferContextual.subscribe(MonoDeferContextual.java:55) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4576) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxUsingWhen.subscribe(FluxUsingWhen.java:105) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4576) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:202) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoFlatMap.subscribeOrReturn(MonoFlatMap.java:53) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4560) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:202) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoFlatMap.subscribeOrReturn(MonoFlatMap.java:53) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:63) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.MonoUsingWhen.subscribe(MonoUsingWhen.java:87) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4576) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.onNext(FluxFlatMap.java:430) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxIterable$IterableSubscription.slowPath(FluxIterable.java:335) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxIterable$IterableSubscription.request(FluxIterable.java:294) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxFlatMap$FlatMapMain.onSubscribe(FluxFlatMap.java:373) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:201) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:83) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Flux.subscribe(Flux.java:8891) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Flux.subscribeWith(Flux.java:9012) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Flux.subscribe(Flux.java:8856) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Flux.subscribe(Flux.java:8780) ~[reactor-core-3.7.1.jar:3.7.1]
	at reactor.core.publisher.Flux.subscribe(Flux.java:8698) ~[reactor-core-3.7.1.jar:3.7.1]
	at r2dbc.mysql.client.metrics.demo.service.PersonService.init(PersonService.kt:22) ~[main/:na]
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:580) ~[na:na]
	at org.springframework.beans.factory.annotation.InitDestroyAnnotationBeanPostProcessor$LifecycleMethod.invoke(InitDestroyAnnotationBeanPostProcessor.java:457) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.annotation.InitDestroyAnnotationBeanPostProcessor$LifecycleMetadata.invokeInitMethods(InitDestroyAnnotationBeanPostProcessor.java:401) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.annotation.InitDestroyAnnotationBeanPostProcessor.postProcessBeforeInitialization(InitDestroyAnnotationBeanPostProcessor.java:219) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsBeforeInitialization(AbstractAutowireCapableBeanFactory.java:423) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1800) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:601) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:523) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:336) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:289) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:334) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.instantiateSingleton(DefaultListableBeanFactory.java:1122) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingleton(DefaultListableBeanFactory.java:1093) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:1030) ~[spring-beans-6.2.1.jar:6.2.1]
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:987) ~[spring-context-6.2.1.jar:6.2.1]
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:627) ~[spring-context-6.2.1.jar:6.2.1]
	at org.springframework.boot.web.reactive.context.ReactiveWebServerApplicationContext.refresh(ReactiveWebServerApplicationContext.java:66) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:752) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:439) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:318) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1361) ~[spring-boot-3.4.1.jar:3.4.1]
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1350) ~[spring-boot-3.4.1.jar:3.4.1]
	at r2dbc.mysql.client.metrics.demo.R2dbcMysqlClientMetricsDemoKt.main(R2dbcMysqlClientMetricsDemo.kt:13) ~[main/:na]
Caused by: java.lang.UnsupportedOperationException: To enable metrics, you must add the dependency `io.micrometer:micrometer-core` to the class path first
	at reactor.netty.transport.Transport.metrics(Transport.java:139) ~[reactor-netty-core-1.2.1.jar:1.2.1]
	at reactor.netty.tcp.TcpClient.metrics(TcpClient.java:241) ~[reactor-netty-core-1.2.1.jar:1.2.1]
	at io.asyncer.r2dbc.mysql.client.Client.connect(Client.java:158) ~[r2dbc-mysql-1.3.2-SNAPSHOT.jar:1.3.2-SNAPSHOT]
	at io.asyncer.r2dbc.mysql.MySqlConnectionFactory.lambda$getMySqlConnection$3(MySqlConnectionFactory.java:143) ~[r2dbc-mysql-1.3.2-SNAPSHOT.jar:1.3.2-SNAPSHOT]
	at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:153) ~[reactor-core-3.7.1.jar:3.7.1]
	... 58 common frames omitted

Copy link
Collaborator

@jchrys jchrys Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. Thanks for correcting me. :D

After I had reviewed the PR again, I’ve decided that it’s important to maintain the current default behavior where metrics are disabled unless explicitly configured(given that this is a low-level lib, we need to be mindful of such changes). To achieve this, we could introduce a property (e.g., metrics) to give users the flexibility to enable metrics when needed.
Would you be interested in contributing to implement this feature? If so, I’d be happy to guide you through the process. you can also refer to this as a reference material.

Looking forward to your response.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea to introduce a property,
and thanks for giving me the chance to add more option on this feature.

I'll add commits on this PR and re-request your review soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jchrys jchrys jchrys left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@jchrys jchrys

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.3.2
Development

Successfully merging this pull request may close these issues.
2 participants
@rxdcxdrnine @jchrys