BFergerson opened a new issue #6701:
URL: https://github.com/apache/skywalking/issues/6701


   Please answer these questions before submitting your issue.
   
   - Why do you submit this issue?
   - [x] Feature or performance improvement
   
   ___
   ### Requirement or improvement
   
   Calling `Instrumentation.retransformClasses())` on a class which SkyWalking 
has already enhanced will cause issues such as 
https://github.com/raphw/byte-buddy/issues/908. This issue boils down to 
SkyWalking not generating the same bytecode modifications each time it's called.
   
   As an example, here is the loaded class after SkyWalking enhancements:
   <details>
     <summary>Decompiled source code</summary>
     
   ```java
   package spp.example.webapp.controller;
   
   import spp.example.webapp.service.*;
   import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.*;
   import java.lang.reflect.*;
   import spp.example.webapp.model.*;
   import java.util.concurrent.*;
   import org.springframework.web.bind.annotation.*;
   import org.springframework.http.*;
   import org.apache.skywalking.apm.toolkit.trace.*;
   import org.slf4j.*;
   
   @RestController
   public class WebappController implements EnhancedInstance
   {
       private static final Logger log;
       private final UserService userService = userService;
       private volatile Object _$EnhancedClassField_ws;
       public static volatile /* synthetic */ InstMethodsInter delegate$v1o0sl0;
       public static volatile /* synthetic */ InstMethodsInter delegate$1p16ip1;
       public static volatile /* synthetic */ ConstructorInter delegate$bn83ge1;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$tcgf6s2;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$551d7u2;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$r5lbqa2;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$pslu6e0;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$tv4hlg2;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$fe38712;
       private static final /* synthetic */ Method cachedValue$5aQdBH3R$moqjd11;
       
       public WebappController(final UserService userService) {
           this(userService, null);
           WebappController.delegate$bn83ge1.intercept((Object)this, new 
Object[] { userService });
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.GET })
       public ResponseEntity<Iterable<User>> userList() {
           return 
(ResponseEntity<Iterable<User>>)WebappController.delegate$1p16ip1.intercept((Object)this,
 new Object[0], (Callable)new WebappController$auxiliary$rC0tNskD(this), 
WebappController.cachedValue$5aQdBH3R$tv4hlg2);
       }
       
       @RequestMapping(value = { "/users/{id}" }, method = { RequestMethod.GET 
})
       public ResponseEntity<User> getUser(@PathVariable final long id) {
           return 
(ResponseEntity<User>)WebappController.delegate$1p16ip1.intercept((Object)this, 
new Object[] { id }, (Callable)new WebappController$auxiliary$3VgxmlNG(this, 
id), WebappController.cachedValue$5aQdBH3R$fe38712);
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.POST })
       public ResponseEntity<User> createUser(@RequestParam final String 
firstName, @RequestParam final String lastName) {
           return 
(ResponseEntity<User>)WebappController.delegate$1p16ip1.intercept((Object)this, 
new Object[] { firstName, lastName }, (Callable)new 
WebappController$auxiliary$giwk9nlQ(this, firstName, lastName), 
WebappController.cachedValue$5aQdBH3R$r5lbqa2);
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.DELETE })
       public void deleteUser(@RequestParam final String firstName, 
@RequestParam final String lastName) {
           WebappController.delegate$1p16ip1.intercept((Object)this, new 
Object[] { firstName, lastName }, (Callable)new 
WebappController$auxiliary$TTasfNa2(this, firstName, lastName), 
WebappController.cachedValue$5aQdBH3R$pslu6e0);
       }
       
       @RequestMapping(value = { "/throws-exception" }, method = { 
RequestMethod.GET })
       public void throwsException() {
           WebappController.delegate$1p16ip1.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$dwKvz7iY(this), 
WebappController.cachedValue$5aQdBH3R$551d7u2);
       }
       
       @Trace
       private void caughtException() {
           WebappController.delegate$v1o0sl0.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$O6efGa79(this), 
WebappController.cachedValue$5aQdBH3R$moqjd11);
       }
       
       @Trace
       private void uncaughtException() {
           WebappController.delegate$v1o0sl0.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$7x058cxd(this), 
WebappController.cachedValue$5aQdBH3R$tcgf6s2);
       }
       
       static {
           
ClassLoader.getSystemClassLoader().loadClass("org.apache.skywalking.apm.dependencies.net.bytebuddy.dynamic.Nexus").getMethod("initialize",
 Class.class, Integer.TYPE).invoke(null, WebappController.class, -1316167843);
           cachedValue$5aQdBH3R$tcgf6s2 = 
WebappController.class.getDeclaredMethod("uncaughtException", (Class<?>[])new 
Class[0]);
           cachedValue$5aQdBH3R$551d7u2 = 
WebappController.class.getMethod("throwsException", (Class<?>[])new Class[0]);
           cachedValue$5aQdBH3R$r5lbqa2 = 
WebappController.class.getMethod("createUser", String.class, String.class);
           cachedValue$5aQdBH3R$pslu6e0 = 
WebappController.class.getMethod("deleteUser", String.class, String.class);
           cachedValue$5aQdBH3R$tv4hlg2 = 
WebappController.class.getMethod("userList", (Class<?>[])new Class[0]);
           cachedValue$5aQdBH3R$fe38712 = 
WebappController.class.getMethod("getUser", Long.TYPE);
           cachedValue$5aQdBH3R$moqjd11 = 
WebappController.class.getDeclaredMethod("caughtException", (Class<?>[])new 
Class[0]);
           log = LoggerFactory.getLogger((Class)WebappController.class);
       }
       
       public void setSkyWalkingDynamicField(final Object 
$EnhancedClassField_ws) {
           this._$EnhancedClassField_ws = $EnhancedClassField_ws;
       }
       
       public Object getSkyWalkingDynamicField() {
           return this._$EnhancedClassField_ws;
       }
   }
   ```
   </details>
   
   This is the same class after calling `Instrumentation.retransformClasses()`:
   <details>
     <summary>Decompiled source code</summary>
     
   ```java
   package spp.example.webapp.controller;
   
   import spp.example.webapp.service.*;
   import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.*;
   import java.lang.reflect.*;
   import spp.example.webapp.model.*;
   import java.util.concurrent.*;
   import org.springframework.web.bind.annotation.*;
   import org.springframework.http.*;
   import org.apache.skywalking.apm.toolkit.trace.*;
   import org.slf4j.*;
   
   @RestController
   public class WebappController implements EnhancedInstance
   {
       private static final Logger log;
       private final UserService userService = userService;
       private volatile Object _$EnhancedClassField_ws;
       public static volatile /* synthetic */ InstMethodsInter delegate$4vkd9v0;
       public static volatile /* synthetic */ InstMethodsInter delegate$memmhe1;
       public static volatile /* synthetic */ ConstructorInter delegate$20v8b80;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$tcgf6s2;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$551d7u2;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$r5lbqa2;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$pslu6e0;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$tv4hlg2;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$fe38712;
       private static final /* synthetic */ Method cachedValue$5e7WjZyN$moqjd11;
       
       public WebappController(final UserService userService) {
           this(userService, null);
           WebappController.delegate$20v8b80.intercept((Object)this, new 
Object[] { userService });
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.GET })
       public ResponseEntity<Iterable<User>> userList() {
           return 
(ResponseEntity<Iterable<User>>)WebappController.delegate$memmhe1.intercept((Object)this,
 new Object[0], (Callable)new WebappController$auxiliary$nPDeHsP4(this), 
WebappController.cachedValue$5e7WjZyN$tv4hlg2);
       }
       
       @RequestMapping(value = { "/users/{id}" }, method = { RequestMethod.GET 
})
       public ResponseEntity<User> getUser(@PathVariable final long id) {
           return 
(ResponseEntity<User>)WebappController.delegate$memmhe1.intercept((Object)this, 
new Object[] { id }, (Callable)new WebappController$auxiliary$cByl7fIX(this, 
id), WebappController.cachedValue$5e7WjZyN$fe38712);
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.POST })
       public ResponseEntity<User> createUser(@RequestParam final String 
firstName, @RequestParam final String lastName) {
           return 
(ResponseEntity<User>)WebappController.delegate$memmhe1.intercept((Object)this, 
new Object[] { firstName, lastName }, (Callable)new 
WebappController$auxiliary$sJY8TJLs(this, firstName, lastName), 
WebappController.cachedValue$5e7WjZyN$r5lbqa2);
       }
       
       @RequestMapping(value = { "/users" }, method = { RequestMethod.DELETE })
       public void deleteUser(@RequestParam final String firstName, 
@RequestParam final String lastName) {
           WebappController.delegate$memmhe1.intercept((Object)this, new 
Object[] { firstName, lastName }, (Callable)new 
WebappController$auxiliary$HdooN252(this, firstName, lastName), 
WebappController.cachedValue$5e7WjZyN$pslu6e0);
       }
       
       @RequestMapping(value = { "/throws-exception" }, method = { 
RequestMethod.GET })
       public void throwsException() {
           WebappController.delegate$memmhe1.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$vZfbpnR9(this), 
WebappController.cachedValue$5e7WjZyN$551d7u2);
       }
       
       @Trace
       private void caughtException() {
           WebappController.delegate$4vkd9v0.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$P2tJxjm1(this), 
WebappController.cachedValue$5e7WjZyN$moqjd11);
       }
       
       @Trace
       private void uncaughtException() {
           WebappController.delegate$4vkd9v0.intercept((Object)this, new 
Object[0], (Callable)new WebappController$auxiliary$7xenKkaR(this), 
WebappController.cachedValue$5e7WjZyN$tcgf6s2);
       }
       
       static {
           
ClassLoader.getSystemClassLoader().loadClass("org.apache.skywalking.apm.dependencies.net.bytebuddy.dynamic.Nexus").getMethod("initialize",
 Class.class, Integer.TYPE).invoke(null, WebappController.class, 433866945);
           cachedValue$5e7WjZyN$tcgf6s2 = 
WebappController.class.getDeclaredMethod("uncaughtException", (Class<?>[])new 
Class[0]);
           cachedValue$5e7WjZyN$551d7u2 = 
WebappController.class.getMethod("throwsException", (Class<?>[])new Class[0]);
           cachedValue$5e7WjZyN$r5lbqa2 = 
WebappController.class.getMethod("createUser", String.class, String.class);
           cachedValue$5e7WjZyN$pslu6e0 = 
WebappController.class.getMethod("deleteUser", String.class, String.class);
           cachedValue$5e7WjZyN$tv4hlg2 = 
WebappController.class.getMethod("userList", (Class<?>[])new Class[0]);
           cachedValue$5e7WjZyN$fe38712 = 
WebappController.class.getMethod("getUser", Long.TYPE);
           cachedValue$5e7WjZyN$moqjd11 = 
WebappController.class.getDeclaredMethod("caughtException", (Class<?>[])new 
Class[0]);
           log = LoggerFactory.getLogger((Class)WebappController.class);
       }
   }
   ```
   </details>
   
   ----
   
   The primary difference between the two is that fields like 
`cachedValue$5aQdBH3R$551d7u2` have been renamed and the second file is missing 
the necessary `setSkyWalkingDynamicField/getSkyWalkingDynamicField` methods.
   
   The latter issue seems easy to solve as you could simply check the trace 
stack for `Instrumentation.retransformClasses()` and allow the enhancement to 
occur again. The former issue seems like it might be a bit harder and I'm 
curious if anyone has an idea how to solve it. The ways I've been thinking 
about solving it are either to:
    - generate unique but consistent fields names (so each time it comes up 
with the same field names as before)
    - save an additional map to each class which specificies what the existing 
fields are and reuse the previous names


-- 
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:
[email protected]


Reply via email to