From 38a1620d6b2a69401a87fbc505a80ab0d845c6eb Mon Sep 17 00:00:00 2001 From: jiangML <1060319118@qq.com> Date: Fri, 19 May 2023 21:17:31 +0800 Subject: [PATCH] Modify proxy_execute_latency_millis metric to be collected only when QueryCommandExecutor (#25781) --- .../proxy/ExecuteLatencyHistogramAdvice.java | 9 +++++-- .../ExecuteLatencyHistogramAdviceTest.java | 24 +++++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/agent/plugins/metrics/core/src/main/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdvice.java b/agent/plugins/metrics/core/src/main/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdvice.java index 8e0631bc769c0..b3159528399e1 100644 --- a/agent/plugins/metrics/core/src/main/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdvice.java +++ b/agent/plugins/metrics/core/src/main/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdvice.java @@ -24,6 +24,7 @@ import org.apache.shardingsphere.agent.plugin.metrics.core.collector.type.HistogramMetricsCollector; import org.apache.shardingsphere.agent.plugin.metrics.core.config.MetricCollectorType; import org.apache.shardingsphere.agent.plugin.metrics.core.config.MetricConfiguration; +import org.apache.shardingsphere.proxy.frontend.command.executor.QueryCommandExecutor; import java.lang.reflect.Method; import java.util.Collections; @@ -51,11 +52,15 @@ private Map getBuckets() { @Override public void beforeMethod(final TargetAdviceObject target, final Method method, final Object[] args, final String pluginType) { - methodTimeRecorder.recordNow(method); + if (args[2] instanceof QueryCommandExecutor) { + methodTimeRecorder.recordNow(method); + } } @Override public void afterMethod(final TargetAdviceObject target, final Method method, final Object[] args, final Object result, final String pluginType) { - MetricsCollectorRegistry.get(config, pluginType).observe(methodTimeRecorder.getElapsedTimeAndClean(method)); + if (args[2] instanceof QueryCommandExecutor) { + MetricsCollectorRegistry.get(config, pluginType).observe(methodTimeRecorder.getElapsedTimeAndClean(method)); + } } } diff --git a/agent/plugins/metrics/core/src/test/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdviceTest.java b/agent/plugins/metrics/core/src/test/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdviceTest.java index 1a7774c56370c..c8a221069424d 100644 --- a/agent/plugins/metrics/core/src/test/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdviceTest.java +++ b/agent/plugins/metrics/core/src/test/java/org/apache/shardingsphere/agent/plugin/metrics/core/advice/proxy/ExecuteLatencyHistogramAdviceTest.java @@ -20,8 +20,10 @@ import org.apache.shardingsphere.agent.plugin.metrics.core.collector.MetricsCollectorRegistry; import org.apache.shardingsphere.agent.plugin.metrics.core.config.MetricCollectorType; import org.apache.shardingsphere.agent.plugin.metrics.core.config.MetricConfiguration; -import org.apache.shardingsphere.agent.plugin.metrics.core.fixture.collector.MetricsCollectorFixture; import org.apache.shardingsphere.agent.plugin.metrics.core.fixture.TargetAdviceObjectFixture; +import org.apache.shardingsphere.agent.plugin.metrics.core.fixture.collector.MetricsCollectorFixture; +import org.apache.shardingsphere.proxy.frontend.mysql.command.admin.quit.MySQLComQuitExecutor; +import org.apache.shardingsphere.proxy.frontend.mysql.command.query.text.query.MySQLComQueryPacketExecutor; import org.awaitility.Awaitility; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -31,6 +33,7 @@ import java.util.concurrent.TimeUnit; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.mockito.Mockito.mock; @@ -44,13 +47,26 @@ void reset() { } @Test - void assertExecuteLatencyHistogram() { + void assertExecuteLatencyHistogramWhenQueryCommandExecutor() { ExecuteLatencyHistogramAdvice advice = new ExecuteLatencyHistogramAdvice(); TargetAdviceObjectFixture targetObject = new TargetAdviceObjectFixture(); Method method = mock(Method.class); - advice.beforeMethod(targetObject, method, new Object[]{}, "FIXTURE"); + Object[] args = new Object[]{null, null, mock(MySQLComQueryPacketExecutor.class)}; + advice.beforeMethod(targetObject, method, args, "FIXTURE"); Awaitility.await().pollDelay(500L, TimeUnit.MILLISECONDS).until(() -> true); - advice.afterMethod(targetObject, method, new Object[]{}, null, "FIXTURE"); + advice.afterMethod(targetObject, method, args, null, "FIXTURE"); assertThat(Double.parseDouble(MetricsCollectorRegistry.get(config, "FIXTURE").toString()), greaterThanOrEqualTo(500d)); } + + @Test + void assertExecuteLatencyHistogramWhenNotQueryCommandExecutor() { + ExecuteLatencyHistogramAdvice advice = new ExecuteLatencyHistogramAdvice(); + TargetAdviceObjectFixture targetObject = new TargetAdviceObjectFixture(); + Method method = mock(Method.class); + Object[] args = new Object[]{null, null, mock(MySQLComQuitExecutor.class)}; + advice.beforeMethod(targetObject, method, args, "FIXTURE"); + Awaitility.await().pollDelay(20L, TimeUnit.MILLISECONDS).until(() -> true); + advice.afterMethod(targetObject, method, args, null, "FIXTURE"); + assertThat(Double.parseDouble(MetricsCollectorRegistry.get(config, "FIXTURE").toString()), equalTo(0d)); + } }