You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2021/04/16 04:38:17 UTC

[GitHub] [skywalking] honganan opened a new issue #6766: [Bug]EndpointNameGrouping in SourceBuilder may not work

honganan opened a new issue #6766:
URL: https://github.com/apache/skywalking/issues/6766


   I glad to found the `EndpointNameGrouping` feature, and it can really solve problems that many enterprises will face.
   ### Bug
   `SourceBuilder` using `NamingControl` to do endpoint name length format and `end-name-grouping` when `setSourceEndpointName` or `setDestEndpointName` invoked. But `NameControl` dependence `serviceName` to finish the group matching according to the code:
   ```java
       @Getter
       private String destServiceName;
   
       public void setDestServiceName(final String destServiceName) {
           this.destServiceName = namingControl.formatServiceName(destServiceName);
       }
   
       @Getter
       @Setter
       private NodeType destNodeType;
       @Getter
       private String destServiceInstanceName;
   
       public void setDestServiceInstanceName(final String destServiceInstanceName) {
           this.destServiceInstanceName = namingControl.formatServiceName(destServiceInstanceName);
       }
   ```
   But this time the `serviceName` may not have been set. When using `SourceBuilder` in `MultiScopesAnalysisListener`, there are many place that set endpoint name first and service name later, like L118~L120
   
   ```java
   sourceBuilder.setDestEndpointName(span.getOperationName());
   sourceBuilder.setDestServiceInstanceName(segmentObject.getServiceInstance());
   sourceBuilder.setDestServiceName(segmentObject.getService());
   ```
   
   ### Requirement or improvement
   The fastest way to solve is change the set methods invoking order. But it's too easy to make a mistake as it depends on set method calling order.
   I suggest adding a `namingFormat()` method in `SourceBuilder` and call it manually after properties set. How do you think?
   
   


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

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



[GitHub] [skywalking] kezhenxu94 commented on issue #6766: [Bug]EndpointNameGrouping in SourceBuilder may not work

Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on issue #6766:
URL: https://github.com/apache/skywalking/issues/6766#issuecomment-820916358


   I don't see the codes in the latest branch, maybe you're looking at an old commit


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

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



[GitHub] [skywalking] kezhenxu94 closed issue #6766: [Bug]EndpointNameGrouping in SourceBuilder may not work

Posted by GitBox <gi...@apache.org>.
kezhenxu94 closed issue #6766:
URL: https://github.com/apache/skywalking/issues/6766


   


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

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



[GitHub] [skywalking] honganan commented on issue #6766: [Bug]EndpointNameGrouping in SourceBuilder may not work

Posted by GitBox <gi...@apache.org>.
honganan commented on issue #6766:
URL: https://github.com/apache/skywalking/issues/6766#issuecomment-820939227


   Oh yes, I have an old version code.


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

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