Change instrumentation enable/disable to use config

Also adds a test
quarkusio · Feb 4, 2021 · 6643b08 · 6643b08
1 parent f1a603e
commit 6643b08
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 17 deletions.
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/DevConfig.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/DevConfig.java
@@ -0,0 +1,18 @@
+package io.quarkus.deployment.dev;
+
+import io.quarkus.runtime.annotations.ConfigItem;
+import io.quarkus.runtime.annotations.ConfigPhase;
+import io.quarkus.runtime.annotations.ConfigRoot;
+
+@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
+public class DevConfig {
+
+    /**
+     * Whether or not Quarkus should disable it's ability to not do a full restart
+     * when changes to classes are compatible with JVM instrumentation.
+     * If this is set to true, Quarkus will always restart on changes and never perform class redefinition.
+     */
+    @ConfigItem(defaultValue = "true")
+    boolean instrumentation;
+
+}
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java b/core/deployment/src/main/java/io/quarkus/deployment/dev/RuntimeUpdatesProcessor.java
@@ -32,6 +32,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.eclipse.microprofile.config.ConfigProvider;
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.AnnotationTarget;
 import org.jboss.jandex.ClassInfo;
@@ -219,7 +220,8 @@ public boolean doScan(boolean userInitiated) throws IOException {
                                 classTransformers.apply(name, bytes));
                     }
                     Index current = indexer.complete();
-                    boolean ok = containsStartupCode(current);
+                    boolean ok = instrumentationEnabled()
+                            && !containsStartupCode(current);
                     if (ok) {
                         for (ClassInfo clazz : current.getKnownClasses()) {
                             ClassInfo old = lastStartIndex.getClassByName(clazz.name());
@@ -270,9 +272,20 @@ public boolean doScan(boolean userInitiated) throws IOException {
         return false;
     }
 
+    private Boolean instrumentationEnabled() {
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
+            return ConfigProvider.getConfig()
+                    .getOptionalValue("quarkus.dev.instrumentation", boolean.class).orElse(true);
+        } finally {
+            Thread.currentThread().setContextClassLoader(old);
+        }
+    }
+
     private boolean containsStartupCode(Index index) {
         if (!index.getAnnotations(STARTUP_NAME).isEmpty()) {
-            return false;
+            return true;
         }
         List<AnnotationInstance> observesInstances = index.getAnnotations(OBSERVES_NAME);
         if (!observesInstances.isEmpty()) {
@@ -281,12 +294,12 @@ private boolean containsStartupCode(Index index) {
                     MethodParameterInfo methodParameterInfo = observesInstance.target().asMethodParameter();
                     short paramPos = methodParameterInfo.position();
                     if (STARTUP_EVENT_NAME.equals(methodParameterInfo.method().parameters().get(paramPos).name())) {
-                        return false;
+                        return true;
                     }
                 }
             }
         }
-        return true;
+        return false;
     }
 
     @Override

diff --git a/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java b/devtools/maven/src/main/java/io/quarkus/maven/DevMojo.java
@@ -271,14 +271,6 @@ public class DevMojo extends AbstractMojo {
     @Parameter(defaultValue = "${quarkus.enforceBuildGoal}")
     private boolean enforceBuildGoal = true;
 
-    /**
-     * Whether or not Quarkus should disable it's ability to not do a full restart
-     * when changes to classes are compatible with JVM instrumentation.
-     * If this is set to true, Quarkus will always restart on changes and never perform class redefinition.
-     */
-    @Parameter(defaultValue = "${disableInstrumentation}")
-    private boolean disableInstrumentation = false;
-
     @Component
     private WorkspaceReader wsReader;
 
@@ -831,9 +823,7 @@ private void addQuarkusDevModeDeps(MavenDevModeLauncher.Builder builder)
                     && appDep.getArtifact().getArtifactId().equals("quarkus-ide-launcher"))) {
                 if (appDep.getArtifact().getGroupId().equals("io.quarkus")
                         && appDep.getArtifact().getArtifactId().equals("quarkus-class-change-agent")) {
-                    if (!disableInstrumentation) {
-                        builder.jvmArgs("-javaagent:" + appDep.getArtifact().getFile().getAbsolutePath());
-                    }
+                    builder.jvmArgs("-javaagent:" + appDep.getArtifact().getFile().getAbsolutePath());
                 } else {
                     builder.classpathEntry(appDep.getArtifact().getFile());
                 }

diff --git a/integration-tests/maven/src/test/java/io/quarkus/maven/it/DevMojoIT.java b/integration-tests/maven/src/test/java/io/quarkus/maven/it/DevMojoIT.java
@@ -38,9 +38,9 @@
 
 /**
  * @author <a href="http://escoffier.me">Clement Escoffier</a>
- *
+ *         <p>
  *         NOTE to anyone diagnosing failures in this test, to run a single method use:
- *
+ *         <p>
  *         mvn install -Dit.test=DevMojoIT#methodName
  */
 @DisableForNative
@@ -234,6 +234,48 @@ public void testThatInstrumentationBasedReloadWorks() throws MavenInvocationExce
 
         //verify that this was an instrumentation based reload
         Assertions.assertEquals(firstUuid, DevModeTestUtils.getHttpResponse("/app/uuid"));
+
+        source = new File(testDir, "src/main/java/org/acme/HelloService.java");
+        filter(source, Collections.singletonMap("\"Stuart\"", "\"Stuart Douglas\""));
+
+        // Wait until we get "Stuart Douglas"
+        await()
+                .pollDelay(100, TimeUnit.MILLISECONDS)
+                .atMost(1, TimeUnit.MINUTES)
+                .until(() -> DevModeTestUtils.getHttpResponse("/app/name").contains("Stuart Douglas"));
+
+        //this bean observes startup event, so it should be different UUID
+        String secondUUid = DevModeTestUtils.getHttpResponse("/app/uuid");
+        Assertions.assertNotEquals(secondUUid, firstUuid);
+
+        //now disable instrumentation based restart, and try again
+        //change it back to hello
+        DevModeTestUtils.getHttpResponse("/app/disable");
+        source = new File(testDir, "src/main/java/org/acme/HelloResource.java");
+        filter(source, Collections.singletonMap("return \"" + uuid + "\";", "return \"hello\";"));
+
+        // Wait until we get "hello"
+        await()
+                .pollDelay(100, TimeUnit.MILLISECONDS)
+                .atMost(1, TimeUnit.MINUTES).until(() -> DevModeTestUtils.getHttpResponse("/app/hello").contains("hello"));
+
+        //verify that this was not instrumentation based reload
+        Assertions.assertNotEquals(secondUUid, DevModeTestUtils.getHttpResponse("/app/uuid"));
+        secondUUid = DevModeTestUtils.getHttpResponse("/app/uuid");
+
+        //now re-enable
+        //and repeat
+        DevModeTestUtils.getHttpResponse("/app/enable");
+        source = new File(testDir, "src/main/java/org/acme/HelloResource.java");
+        filter(source, Collections.singletonMap("return \"hello\";", "return \"" + uuid + "\";"));
+
+        // Wait until we get uuid
+        await()
+                .pollDelay(100, TimeUnit.MILLISECONDS)
+                .atMost(1, TimeUnit.MINUTES).until(() -> DevModeTestUtils.getHttpResponse("/app/hello").contains(uuid));
+
+        //verify that this was an instrumentation based reload
+        Assertions.assertEquals(secondUUid, DevModeTestUtils.getHttpResponse("/app/uuid"));
     }
 
     @Test

diff --git a/.../maven/src/test/resources/projects/classic-inst/src/main/java/org/acme/HelloResource.java b/.../maven/src/test/resources/projects/classic-inst/src/main/java/org/acme/HelloResource.java
@@ -43,4 +43,16 @@ public String name() {
     public String uuid() {
         return uuid.toString();
     }
+
+    @GET
+    @Path("disable")
+    public void disable() {
+        System.setProperty("quarkus.dev.instrumentation", "false");
+    }
+
+    @GET
+    @Path("enable")
+    public void enable() {
+        System.setProperty("quarkus.dev.instrumentation","true");
+    }
 }
diff --git a/...s/maven/src/test/resources/projects/classic-inst/src/main/java/org/acme/HelloService.java b/...s/maven/src/test/resources/projects/classic-inst/src/main/java/org/acme/HelloService.java
@@ -1,10 +1,17 @@
 package org.acme;
 
+import io.quarkus.runtime.StartupEvent;
+
 import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.event.Observes;
 
 @ApplicationScoped
 public class HelloService {
 
+    public void start(@Observes StartupEvent startupEvent) {
+
+    }
+
     public String name() {
         return "Stuart";
     }