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,
This commit is contained in:
yujiaxinlong 2021-07-17 20:50:57 +08:00 committed by GitHub
parent cd029df360
commit aa4ee27d4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 19 deletions

View File

@ -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

View File

@ -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();
}
}
});
}