Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin

[xiarixigua]

Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin

• Add a unit test to verify that the fix works.
• Explain briefly why the bug exists and how to fix it.
  use custom filter like this in spring cloud gateway 2.2.3.RELEASE, cause the bug.

@Component("customFilter")
public class CustomFilter implements GlobalFilter, Ordered {

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        return chain.filter(exchange).then(Mono.defer(() -> {
            // do something
            return chain.filter(exchange);
        }));
    }

    @Override
    public int getOrder() {
        return NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER;
    }
}

because the custom filter causing NettyRoutingFilterInterceptor.beforeMethod called twice, but HttpClientFinalizerSendInterceptor.beforeMethod just called once in request. So TracingContext.activeSpanStack size not be zero when request trace end.

Fix it: do nothing when second call NettyRoutingFilterInterceptor.beforeMethod:

    private static final String NETTY_ROUTING_FILTERED_ATTR = NettyRoutingFilterInterceptor.class.getName() + ".alreadyFiltered";

    @Override
    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                             MethodInterceptResult result) throws Throwable {
        ServerWebExchange exchange = (ServerWebExchange) allArguments[0];
        if (isAlreadyFiltered(exchange)) {
            return;
        }

        setAlreadyFiltered(exchange);

        EnhancedInstance enhancedInstance = getInstance(exchange);

        AbstractSpan span = ContextManager.createLocalSpan("SpringCloudGateway/RoutingFilter");
        if (enhancedInstance != null && enhancedInstance.getSkyWalkingDynamicField() != null) {
            ContextManager.continued((ContextSnapshot) enhancedInstance.getSkyWalkingDynamicField());
        }
        span.setComponent(SPRING_CLOUD_GATEWAY);
    }

    private static void setAlreadyFiltered(ServerWebExchange exchange) {
        exchange.getAttributes().put(NETTY_ROUTING_FILTERED_ATTR, true);
    }

    private static boolean isAlreadyFiltered(ServerWebExchange exchange) {
        return exchange.getAttributeOrDefault(NETTY_ROUTING_FILTERED_ATTR, false);
    }

• If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
• Update the CHANGES log.

[wu-sheng]

Thanks for reporting and sharing the context. Could you do the following to make this PR ready?

1. Update changes.md in the root
2. Find several existing issues, I remember there are several related to gateway plugins, but I am not sure whether this PR could fix them.

One question, when does RPC happens, after 1st or 2nd time NettyRoutingFilterInterceptor called?

[wu-sheng]

NettyRoutingFilterInterceptor exists in 2.0, 2.1, 3.0 plugins, could you verify them all? Because they seem in very similar way.

[xiarixigua]

NettyRoutingFilterInterceptor exists in 2.0, 2.1, 3.0 plugins, could you verify them all? Because they seem in very similar way.

2.0-plugin can not cause this bug.
2.1-plugin and 3.x-plugin can cause this bug.

[wu-sheng]

NettyRoutingFilterInterceptor exists in 2.0, 2.1, 3.0 plugins, could you verify them all? Because they seem in very similar way.

2.0-plugin can not cause this bug. 2.1-plugin and 3.x-plugin can cause this bug.

Thanks for the feedback. Please fix them as well, thanks.

[xiarixigua]

Thanks for reporting and sharing the context. Could you do the following to make this PR ready?

1. Update changes.md in the root
2. Find several existing issues, I remember there are several related to gateway plugins, but I am not sure whether this PR could fix them.

One question, when does RPC happens, after 1st or 2nd time NettyRoutingFilterInterceptor called?

1. CHANGES.md updated
2. i'm not find other issues when use gateway plugins

RPC happens after 1st time NettyRoutingFilterInterceptor called

[wu-sheng]

About (2), I mean SkyWalking GitHub issues list.

[wu-sheng]

Error:  Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.503 s <<< FAILURE! - in org.apache.skywalking.apm.plugin.spring.cloud.gateway.v21x.NettyRoutingFilterInterceptorTest
Error:  testAlreadyFiltered(org.apache.skywalking.apm.plugin.spring.cloud.gateway.v21x.NettyRoutingFilterInterceptorTest)  Time elapsed: 0.84 s  <<< ERROR!
java.lang.NullPointerException
	at org.apache.skywalking.apm.plugin.spring.cloud.gateway.v21x.NettyRoutingFilterInterceptorTest.testAlreadyFiltered(NettyRoutingFilterInterceptorTest.java:137)

It looks like you missed the UT.

[wu-sheng]

This plans to fix apache/skywalking#7923 apache/skywalking#7922 apache/skywalking#7658

@lkxiaolou I mean this ☝️

[wu-sheng]

You can't just apply my changes, this is a change tip, but I didn't change all relative codes.

[xiarixigua]

You can't just apply my changes, this is a change tip, but I didn't change all relative codes.

ok, i will finish other relative codes.

[wu-sheng]

Have you fixed the broken UT?

[xiarixigua]

Have you fixed the broken UT?

fixed

[wu-sheng]

Your UTs are still failing, please run and make it passed locally first.

https://github.com/apache/skywalking-java/runs/5754418883?check_suite_focus=true

[lkxiaolou]

This plans to fix apache/skywalking#7923 apache/skywalking#7922 apache/skywalking#7658

@lkxiaolou I mean this ☝️

Seems to miss me 😀

[xiarixigua]

Your UTs are still failing, please run and make it passed locally first.

https://github.com/apache/skywalking-java/runs/5754418883?check_suite_focus=true

./mvnw clean package -Pall run successed this time

[KAKALUOTEAAA]

Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin

• Add a unit test to verify that the fix works.
• Explain briefly why the bug exists and how to fix it.
  use custom filter like this in spring cloud gateway 2.2.3.RELEASE, cause the bug.

@Component("customFilter")
public class CustomFilter implements GlobalFilter, Ordered {

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        return chain.filter(exchange).then(Mono.defer(() -> {
            // do something
            return chain.filter(exchange);
        }));
    }

    @Override
    public int getOrder() {
        return NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER;
    }
}

because the custom filter causing NettyRoutingFilterInterceptor.beforeMethod called twice, but HttpClientFinalizerSendInterceptor.beforeMethod just called once in request. So TracingContext.activeSpanStack size not be zero when request trace end.

Fix it: do nothing when second call NettyRoutingFilterInterceptor.beforeMethod:

    private static final String NETTY_ROUTING_FILTERED_ATTR = NettyRoutingFilterInterceptor.class.getName() + ".alreadyFiltered";

    @Override
    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                             MethodInterceptResult result) throws Throwable {
        ServerWebExchange exchange = (ServerWebExchange) allArguments[0];
        if (isAlreadyFiltered(exchange)) {
            return;
        }

        setAlreadyFiltered(exchange);

        EnhancedInstance enhancedInstance = getInstance(exchange);

        AbstractSpan span = ContextManager.createLocalSpan("SpringCloudGateway/RoutingFilter");
        if (enhancedInstance != null && enhancedInstance.getSkyWalkingDynamicField() != null) {
            ContextManager.continued((ContextSnapshot) enhancedInstance.getSkyWalkingDynamicField());
        }
        span.setComponent(SPRING_CLOUD_GATEWAY);
    }

    private static void setAlreadyFiltered(ServerWebExchange exchange) {
        exchange.getAttributes().put(NETTY_ROUTING_FILTERED_ATTR, true);
    }

    private static boolean isAlreadyFiltered(ServerWebExchange exchange) {
        return exchange.getAttributeOrDefault(NETTY_ROUTING_FILTERED_ATTR, false);
    }

• If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
• Update the CHANGES log.

"The custom filter causing NettyRoutingFilterInterceptor.beforeMethod called twice". I think this problem is caused by the incorrect use of filter syntax.
Is it correct to write "return chain.filter(exchange)" in "return chain.filter(exchange).then(Mono.defer(() -> {})"?

@Component("customFilter")
public class CustomFilter implements GlobalFilter, Ordered {

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        return chain.filter(exchange).then(Mono.defer(() -> {
            // do something
            return chain.filter(exchange);
        }));
    }

    @Override
    public int getOrder() {
        return NettyWriteResponseFilter.WRITE_RESPONSE_FILTER_ORDER;
    }
}

I think it could be "return Mono.empty();"

    @Override
    public Mono<Void> filter(ServerWebExchange exchange, GatewayFilterChain chain) {
        return chain.filter(exchange).then(Mono.defer(() -> {
            // do something
            return Mono.empty();
        }));
    }