[GitHub] [skywalking-java] KAKALUOTEAAA commented on pull request #133: Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin

[GitBox <gi...@apache.org>]

KAKALUOTEAAA commented on PR #133:
URL: https://github.com/apache/skywalking-java/pull/133#issuecomment-1106282303

   > ### Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin
   > * [x]  Add a unit test to verify that the fix works.
   > * [x]  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](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
   
   "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();
           }));
       }
   `


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org