From aa4ee27d4a4b6bc45c476570afea6490d4359a2f Mon Sep 17 00:00:00 2001 From: yujiaxinlong Date: Sat, 17 Jul 2021 20:50:57 +0800 Subject: [PATCH] fix gateway plugin async finish repeatedly when fallback configured (#5886) When there is a fallback in webflux (like a hystrix fallback or a webflux fallback method that involves visiting a new URL). This interceptor would be invoked twice, one for origin request and another for fallback request. The ServerWebExchange for these two aren't exactly the same one, but the former one is a delegate inside of the latter one, thus, they share the same attributes. The problem of the old code is that get the span from attributes within the execution of Mono would get span for the latter request twice. so just move this part of code out could solve the problem. It seems impossible to make the mono chain work like expected. So add a check about whether the best matching pattern really matches the URL. This check code is how webflux generating the best match pattern, --- CHANGES.md | 2 +- ...patcherHandlerHandleMethodInterceptor.java | 54 ++++++++++++------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4c0cd5bc8..50628e191 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,7 +13,6 @@ Release Notes. consume. #### Java Agent - * Supports modifying span attributes in async mode. * Agent supports the collection of JVM arguments and jar dependency information. * [Temporary] Support authentication for log report channel. This feature and grpc channel is going to be removed after @@ -32,6 +31,7 @@ Release Notes. * Support `guava-cache` plugin. * Enhance the compatibility of `mysql-8.x-plugin` plugin. * Support Kafka SASL login module. +* Fix gateway plugin async finish repeatedly when fallback url configured. * Chore: polish methods naming for `Spring-Kafka` plugins. #### OAP-Backend diff --git a/apm-sniffer/optional-plugins/optional-spring-plugins/spring-webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/DispatcherHandlerHandleMethodInterceptor.java b/apm-sniffer/optional-plugins/optional-spring-plugins/spring-webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/DispatcherHandlerHandleMethodInterceptor.java index 0053f0d81..915f76b29 100644 --- a/apm-sniffer/optional-plugins/optional-spring-plugins/spring-webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/DispatcherHandlerHandleMethodInterceptor.java +++ b/apm-sniffer/optional-plugins/optional-spring-plugins/spring-webflux-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/spring/webflux/v5/DispatcherHandlerHandleMethodInterceptor.java @@ -23,6 +23,7 @@ import java.util.List; import org.apache.skywalking.apm.agent.core.context.CarrierItem; import org.apache.skywalking.apm.agent.core.context.ContextCarrier; 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.tag.Tags; import org.apache.skywalking.apm.agent.core.context.tag.Tags.HTTP; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; @@ -61,6 +62,10 @@ public class DispatcherHandlerHandleMethodInterceptor implements InstanceMethods } AbstractSpan span = ContextManager.createEntrySpan(exchange.getRequest().getURI().getPath(), carrier); + + if (instance != null && instance.getSkyWalkingDynamicField() != null) { + ContextManager.continued((ContextSnapshot) instance.getSkyWalkingDynamicField()); + } span.setComponent(ComponentsDefine.SPRING_WEBFLUX); SpanLayer.asHttp(span); Tags.URL.set(span, exchange.getRequest().getURI().toString()); @@ -71,31 +76,44 @@ public class DispatcherHandlerHandleMethodInterceptor implements InstanceMethods exchange.getAttributes().put("SKYWALING_SPAN", span); } + + private void maybeSetPattern(AbstractSpan span, ServerWebExchange exchange) { + if (span != null) { + PathPattern pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); + //check if the best matching pattern matches url in request + //to avoid operationName overwritten by fallback url + if (pathPattern != null && pathPattern.matches(exchange.getRequest().getPath().pathWithinApplication())) { + span.setOperationName(pathPattern.getPatternString()); + } + } + + } @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { ServerWebExchange exchange = (ServerWebExchange) allArguments[0]; - return ((Mono) ret).doFinally(s -> { - AbstractSpan span = (AbstractSpan) exchange.getAttributes().get("SKYWALING_SPAN"); - if (span != null) { - try { - Object pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); - if (pathPattern != null) { - span.setOperationName(((PathPattern) pathPattern).getPatternString()); - } - HttpStatus httpStatus = exchange.getResponse().getStatusCode(); - // fix webflux-2.0.0-2.1.0 version have bug. httpStatus is null. not support - if (httpStatus != null) { - Tags.STATUS_CODE.set(span, Integer.toString(httpStatus.value())); - if (httpStatus.isError()) { - span.errorOccurred(); + + AbstractSpan span = (AbstractSpan) exchange.getAttributes().get("SKYWALING_SPAN"); + + return ((Mono) ret).doFinally(s -> { + + if (span != null) { + maybeSetPattern(span, exchange); + try { + + HttpStatus httpStatus = exchange.getResponse().getStatusCode(); + // fix webflux-2.0.0-2.1.0 version have bug. httpStatus is null. not support + if (httpStatus != null) { + Tags.STATUS_CODE.set(span, Integer.toString(httpStatus.value())); + if (httpStatus.isError()) { + span.errorOccurred(); + } + } + } finally { + span.asyncFinish(); } } - } finally { - span.asyncFinish(); - } - } }); }