This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-java.git
The following commit(s) were added to refs/heads/main by this push:
new f227543fc3 Fix a bug in Spring Cloud Gateway if
HttpClientFinalizer#send does not invoke, the span created at
NettyRoutingFilterInterceptor can not stop. (#672)
f227543fc3 is described below
commit f227543fc3a40170375b407c31376f5842771e2b
Author: cylx3126 <49175642+cylx3...@users.noreply.github.com>
AuthorDate: Thu Mar 7 15:55:08 2024 +0800
Fix a bug in Spring Cloud Gateway if HttpClientFinalizer#send does not
invoke, the span created at NettyRoutingFilterInterceptor can not stop. (#672)
---
CHANGES.md | 1 +
.../v21x/NettyRoutingFilterInterceptor.java | 4 +
.../v21x/NettyRoutingFilterInterceptorTest.java | 147 ++++++++++++++++++++-
.../gateway/v3x/NettyRoutingFilterInterceptor.java | 4 +
.../v3x/NettyRoutingFilterInterceptorTest.java | 11 +-
.../gateway/v4x/NettyRoutingFilterInterceptor.java | 4 +
.../v4x/NettyRoutingFilterInterceptorTest.java | 11 +-
7 files changed, 172 insertions(+), 10 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 222b2b7a7f..5ea8c5da7b 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -16,6 +16,7 @@ Release Notes.
* Rename system env name from `sw_plugin_kafka_producer_config` to
`SW_PLUGIN_KAFKA_PRODUCER_CONFIG`.
* Support for ActiveMQ-Artemis messaging tracing.
* Archive the expired plugins `impala-jdbc-2.6.x-plugin`.
+* Fix a bug in Spring Cloud Gateway if HttpClientFinalizer#send does not
invoke, the span created at NettyRoutingFilterInterceptor can not stop.
#### Documentation
* Update docs to describe `expired-plugins`.
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptor.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptor.java
index 578352e8e8..6e7dba5726 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptor.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptor.java
@@ -74,6 +74,10 @@ public class NettyRoutingFilterInterceptor implements
InstanceMethodsAroundInter
@Override
public Object afterMethod(EnhancedInstance objInst, Method method,
Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
+ if (ContextManager.isActive()) {
+ // if HttpClientFinalizerSendInterceptor does not invoke, we will
stop the span there to avoid context leak.
+ ContextManager.stopSpan();
+ }
return ret;
}
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptorTest.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptorTest.java
index 43d75b0cee..30bc2d6ce2 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptorTest.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-2.1.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v21x/NettyRoutingFilterInterceptorTest.java
@@ -21,13 +21,23 @@ package
org.apache.skywalking.apm.plugin.spring.cloud.gateway.v21x;
import java.security.Principal;
import java.time.Instant;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.function.Function;
import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.ContextSnapshot;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan;
+import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
+import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment;
+import
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.test.helper.SegmentHelper;
import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule;
import org.apache.skywalking.apm.agent.test.tools.SegmentStorage;
import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint;
+import org.apache.skywalking.apm.agent.test.tools.SpanAssert;
import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner;
+import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
@@ -47,6 +57,103 @@ import reactor.core.publisher.Mono;
@RunWith(TracingSegmentRunner.class)
public class NettyRoutingFilterInterceptorTest {
+
+ private static class ServerWebExchangeEnhancedInstance implements
ServerWebExchange, EnhancedInstance {
+ private ContextSnapshot snapshot;
+ Map<String, Object> attributes = new HashMap<>();
+
+ @Override
+ public Object getSkyWalkingDynamicField() {
+ return snapshot;
+ }
+
+ @Override
+ public void setSkyWalkingDynamicField(Object value) {
+ this.snapshot = (ContextSnapshot) value;
+ }
+
+ @Override
+ public ServerHttpRequest getRequest() {
+ return null;
+ }
+
+ @Override
+ public ServerHttpResponse getResponse() {
+ return null;
+ }
+
+ @Override
+ public Map<String, Object> getAttributes() {
+ return attributes;
+ }
+
+ @Override
+ public Mono<WebSession> getSession() {
+ return null;
+ }
+
+ @Override
+ public <T extends Principal> Mono<T> getPrincipal() {
+ return null;
+ }
+
+ @Override
+ public Mono<MultiValueMap<String, String>> getFormData() {
+ return null;
+ }
+
+ @Override
+ public Mono<MultiValueMap<String, Part>> getMultipartData() {
+ return null;
+ }
+
+ @Override
+ public LocaleContext getLocaleContext() {
+ return null;
+ }
+
+ @Override
+ public ApplicationContext getApplicationContext() {
+ return null;
+ }
+
+ @Override
+ public boolean isNotModified() {
+ return false;
+ }
+
+ @Override
+ public boolean checkNotModified(Instant instant) {
+ return false;
+ }
+
+ @Override
+ public boolean checkNotModified(String s) {
+ return false;
+ }
+
+ @Override
+ public boolean checkNotModified(String s, Instant instant) {
+ return false;
+ }
+
+ @Override
+ public String transformUrl(String s) {
+ return null;
+ }
+
+ @Override
+ public void addUrlTransformer(Function<String, String> function) {
+
+ }
+
+ @Override
+ public String getLogPrefix() {
+ return null;
+ }
+ }
+
+ private final ServerWebExchangeEnhancedInstance enhancedInstance = new
ServerWebExchangeEnhancedInstance();
private final NettyRoutingFilterInterceptor interceptor = new
NettyRoutingFilterInterceptor();
@Rule
public AgentServiceRule serviceRule = new AgentServiceRule();
@@ -151,12 +258,48 @@ public class NettyRoutingFilterInterceptorTest {
interceptor.beforeMethod(null, null, new Object[]{exchange}, null,
null);
interceptor.afterMethod(null, null, null, null, null);
Assert.assertEquals(exchange.getAttributes().get(NETTY_ROUTING_FILTER_TRACED_ATTR),
true);
- Assert.assertNotNull(ContextManager.activeSpan());
+ Assert.assertFalse(ContextManager.isActive());
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
interceptor.beforeMethod(null, null, new Object[]{exchange}, null,
null);
interceptor.afterMethod(null, null, null, null, null);
Assert.assertEquals(exchange.getAttributes().get(NETTY_ROUTING_FILTER_TRACED_ATTR),
true);
}
+
+ @Test
+ public void testWithNullDynamicField() throws Throwable {
+ interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
+ interceptor.afterMethod(null, null, null, null, null);
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
+ final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
+ Assert.assertEquals(traceSegments.size(), 1);
+ final List<AbstractTracingSpan> spans =
SegmentHelper.getSpans(traceSegments.get(0));
+ Assert.assertNotNull(spans);
+ Assert.assertEquals(spans.size(), 1);
+ SpanAssert.assertComponent(spans.get(0),
ComponentsDefine.SPRING_CLOUD_GATEWAY);
+ }
+
+ @Test
+ public void testWithContextSnapshot() throws Throwable {
+ final AbstractSpan entrySpan = ContextManager.createEntrySpan("/get",
null);
+ SpanLayer.asHttp(entrySpan);
+ entrySpan.setComponent(ComponentsDefine.SPRING_WEBFLUX);
+ enhancedInstance.setSkyWalkingDynamicField(ContextManager.capture());
+ interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
+ interceptor.afterMethod(null, null, null, null, null);
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
+ ContextManager.stopSpan(entrySpan);
+ final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
+ Assert.assertEquals(traceSegments.size(), 1);
+ final List<AbstractTracingSpan> spans =
SegmentHelper.getSpans(traceSegments.get(0));
+ Assert.assertNotNull(spans);
+ Assert.assertEquals(spans.size(), 2);
+ SpanAssert.assertComponent(spans.get(0),
ComponentsDefine.SPRING_CLOUD_GATEWAY);
+ SpanAssert.assertComponent(spans.get(1),
ComponentsDefine.SPRING_WEBFLUX);
+ SpanAssert.assertLayer(spans.get(1), SpanLayer.HTTP);
+ }
}
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptor.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptor.java
index 8b53595e4e..014dd4b6c4 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptor.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptor.java
@@ -81,6 +81,10 @@ public class NettyRoutingFilterInterceptor implements
InstanceMethodsAroundInter
@Override
public Object afterMethod(EnhancedInstance objInst, Method method,
Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
+ if (ContextManager.isActive()) {
+ // if HttpClientFinalizerSendInterceptor does not invoke, we will
stop the span there to avoid context leak.
+ ContextManager.stopSpan();
+ }
return ret;
}
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptorTest.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptorTest.java
index 36a262ab30..a2614b2245 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptorTest.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-3.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v3x/NettyRoutingFilterInterceptorTest.java
@@ -170,7 +170,8 @@ public class NettyRoutingFilterInterceptorTest {
public void testWithNullDynamicField() throws Throwable {
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
Assert.assertEquals(traceSegments.size(), 1);
final List<AbstractTracingSpan> spans =
SegmentHelper.getSpans(traceSegments.get(0));
@@ -187,7 +188,8 @@ public class NettyRoutingFilterInterceptorTest {
enhancedInstance.setSkyWalkingDynamicField(ContextManager.capture());
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
ContextManager.stopSpan(entrySpan);
final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
Assert.assertEquals(traceSegments.size(), 1);
@@ -207,9 +209,10 @@ public class NettyRoutingFilterInterceptorTest {
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
Assert.assertEquals(enhancedInstance.getAttributes().get(NETTY_ROUTING_FILTER_TRACED_ATTR),
true);
- Assert.assertNotNull(ContextManager.activeSpan());
+ Assert.assertFalse(ContextManager.isActive());
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptor.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptor.java
index c5271f9a82..f2337856e0 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptor.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptor.java
@@ -81,6 +81,10 @@ public class NettyRoutingFilterInterceptor implements
InstanceMethodsAroundInter
@Override
public Object afterMethod(EnhancedInstance objInst, Method method,
Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
+ if (ContextManager.isActive()) {
+ // if HttpClientFinalizerSendInterceptor does not invoke, we will
stop the span there to avoid context leak.
+ ContextManager.stopSpan();
+ }
return ret;
}
diff --git
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptorTest.java
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptorTest.java
index 25bbffb7cc..9eab1fdd36 100644
---
a/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptorTest.java
+++
b/apm-sniffer/optional-plugins/optional-spring-plugins/optional-spring-cloud/gateway-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/spring/cloud/gateway/v4x/NettyRoutingFilterInterceptorTest.java
@@ -170,7 +170,8 @@ public class NettyRoutingFilterInterceptorTest {
public void testWithNullDynamicField() throws Throwable {
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
Assert.assertEquals(traceSegments.size(), 1);
final List<AbstractTracingSpan> spans =
SegmentHelper.getSpans(traceSegments.get(0));
@@ -187,7 +188,8 @@ public class NettyRoutingFilterInterceptorTest {
enhancedInstance.setSkyWalkingDynamicField(ContextManager.capture());
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
ContextManager.stopSpan(entrySpan);
final List<TraceSegment> traceSegments =
segmentStorage.getTraceSegments();
Assert.assertEquals(traceSegments.size(), 1);
@@ -207,9 +209,10 @@ public class NettyRoutingFilterInterceptorTest {
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
Assert.assertEquals(enhancedInstance.getAttributes().get(NETTY_ROUTING_FILTER_TRACED_ATTR),
true);
- Assert.assertNotNull(ContextManager.activeSpan());
+ Assert.assertFalse(ContextManager.isActive());
- ContextManager.stopSpan();
+ // no more need this, span was stopped at interceptor#afterMethod
+ // ContextManager.stopSpan();
interceptor.beforeMethod(null, null, new Object[]{enhancedInstance},
null, null);
interceptor.afterMethod(null, null, null, null, null);
