You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2019/06/07 15:19:26 UTC

[sling-whiteboard] 05/07: Create a simple log abstraction

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch url-connection-agent-cleanups
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit 06278cef3c7ada8e8a04647f1485d25108a96dcd
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Fri Jun 7 16:39:13 2019 +0200

    Create a simple log abstraction
---
 .../main/java/org/apache/sling/uca/impl/Agent.java |  17 ++--
 .../uca/impl/HttpClient3TimeoutTransformer.java    |   7 +-
 .../uca/impl/HttpClient4TimeoutTransformer.java    |   7 +-
 .../sling/uca/impl/JavaNetTimeoutTransformer.java  |  10 +-
 .../main/java/org/apache/sling/uca/impl/Log.java   | 113 +++++++++++++++++++++
 .../java/org/apache/sling/uca/impl/AgentIT.java    |   2 +-
 6 files changed, 135 insertions(+), 21 deletions(-)

diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
index a9320d6..e523155 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Agent.java
@@ -24,16 +24,20 @@ public class Agent {
 
     public static void premain(String args, Instrumentation inst) {
         
-        System.out.println("[AGENT] Loading agent...");
         String[] parsedArgs = args != null ? args.split(",") : new String[0];
         long connectTimeout =  TimeUnit.MINUTES.toMillis(1);
         long readTimeout = TimeUnit.MINUTES.toMillis(1);
+        String logSpec = "";
         if ( parsedArgs.length > 0 )
             connectTimeout = Long.parseLong(parsedArgs[0]);
         if ( parsedArgs.length > 1 )
             readTimeout = Long.parseLong(parsedArgs[1]);
+        if ( parsedArgs.length > 2)
+            logSpec = parsedArgs[2];
         
-        System.out.format("[AGENT] Set connectTimeout : %d, readTimeout: %d%n", connectTimeout, readTimeout);
+        Log.configure(logSpec);
+        
+        Log.get().log("Preparing to install URL transformers. Configured timeouts - connectTimeout : %d, readTimeout: %d", connectTimeout, readTimeout);
 
         ClassFileTransformer[] transformers = new ClassFileTransformer[] {
             new JavaNetTimeoutTransformer(connectTimeout, readTimeout),
@@ -44,11 +48,6 @@ public class Agent {
         for ( ClassFileTransformer transformer : transformers )
             inst.addTransformer(transformer, true);
 
-        System.out.println("[AGENT] Loaded agent!");
-    }
-
-    public static void agentmain(String args, Instrumentation inst) {
-        premain(args, inst);
-    }
-    
+        Log.get().log("All transformers installed");
+    }    
 }
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
index 6e57245..9bf5d92 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
@@ -48,7 +48,7 @@ public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
         try {
             if ( DEFAULT_HTTP_PARAMS_FACTORY_CLASS_NAME.equals(className) ) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 
                 ClassPool defaultPool = ClassPool.getDefault();
                 CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
@@ -63,11 +63,12 @@ public class HttpClient3TimeoutTransformer implements ClassFileTransformer {
                 
                 classfileBuffer = cc.toBytecode();
                 cc.detach();
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
 }
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
index e74dabf..230c1db 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
@@ -51,7 +51,7 @@ public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
         try {
             if ( REQUEST_CONFIG_BUILDER_CLASS_NAME.equals(className) ) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 
                 ClassPool defaultPool = ClassPool.getDefault();
                 CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
@@ -65,11 +65,12 @@ public class HttpClient4TimeoutTransformer implements ClassFileTransformer {
                 
                 classfileBuffer = cc.toBytecode();
                 cc.detach();
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
 
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
index 9978c33..121417c 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
@@ -60,18 +60,18 @@ class JavaNetTimeoutTransformer implements ClassFileTransformer {
             ProtectionDomain protectionDomain, byte[] classfileBuffer) {
         try {
             if (CLASSES_TO_TRANSFORM.contains(className)) {
-                System.out.println("[AGENT] Asked to transform " + className);
+                Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
                 CtMethod connectMethod = findConnectMethod(className);
                 connectMethod.insertBefore("if ( getConnectTimeout() == 0 ) { setConnectTimeout(" + connectTimeoutMillis + "); }");
                 connectMethod.insertBefore("if ( getReadTimeout() == 0 ) { setReadTimeout(" + readTimeoutMillis + "); }");
                 classfileBuffer = connectMethod.getDeclaringClass().toBytecode();
                 connectMethod.getDeclaringClass().detach();
-                System.out.println("[AGENT] Transformation complete!");
+                Log.get().log("Transformation complete.");
             }
             return classfileBuffer;
         } catch (Exception e) {
-            e.printStackTrace(); // ensure _something_ is printed
-            throw new RuntimeException("[AGENT] Transformation failed", e);
+            Log.get().fatal("Transformation failed", e);
+            return null;
         }
     }
     
@@ -80,7 +80,7 @@ class JavaNetTimeoutTransformer implements ClassFileTransformer {
         ClassPool defaultPool = ClassPool.getDefault();
         CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
         if (cc == null) {
-            System.out.println("[AGENT] no class found with name " + className);
+            Log.get().log("No class found with name %s", className);
             return null;
         }
         return cc.getDeclaredMethod("connect");
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java
new file mode 100644
index 0000000..900f426
--- /dev/null
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/Log.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.uca.impl;
+
+import static java.util.Objects.requireNonNull;
+
+import java.util.Formatter;
+
+/**
+ * Simple logger abstraction
+ * 
+ * <p>This is an intentionally simple and simplistic logger for agent-internal usage. Since the agent environment
+ * is limited and can make no assumption about how it will be used, it only uses the console and no
+ * external libraries.</p>
+ * 
+ * <p>It is required to call {@link #configure(String)} before accessing the log instance using {@link #get()}.</p>
+ *
+ */
+abstract class Log {
+    
+    private static Log INSTANCE;
+
+    /**
+     * Configures the global logger instance
+     * 
+     * @param spec the logger spec, <tt>v</tt> for a console log, anything else for a no-op log
+     */
+    public static void configure(String spec) {
+        INSTANCE = "v".equals(spec) ? new ConsoleLog() : new NoopLog();
+    }
+    
+    /**
+     * Gets the global logger instance, configured with {@link #configure(String)}
+     * 
+     * @return the global logger instance
+     * @throws NullPointerException in case the logger is not configured
+     */
+    public static Log get() {
+        return requireNonNull(INSTANCE, "Log is null, did you foget to call Log.configure() ?");
+    }
+
+    private Log() {
+
+    }
+
+    /**
+     * Logs a message
+     * 
+     * <p>The message and the arguments are interpolated using a {@link Formatter}, e.g.
+     * 
+     * <pre>Logger.get().log("Transforming %s", klazz.getName());</pre>
+     * 
+     *  </p>
+     *  
+     *  <p>The line separator <tt>%n</tt> is automatically appended to the message.</p>
+     * 
+     * @param msg the message
+     * @param args the arguments
+     */
+    public abstract void log(String msg, Object... args);
+    
+    /**
+     * Prints the throwable stack trace and throws a <tt>RuntimeException</tt>
+     * 
+     * @param message the message to include in the <tt>RuntimeException</tt>
+     * @param t the throwable to print the stack trace for 
+     */
+    public abstract void fatal(String message, Throwable t);
+
+    static class ConsoleLog extends Log {
+
+        private static final String LOG_ENTRY_PREFIX = "[AGENT] ";
+
+        @Override
+        public void log(String msg, Object... args) {
+            System.out.format(LOG_ENTRY_PREFIX + msg + " %n", args);
+        }
+        
+        @Override
+        public void fatal(String msg, Throwable t) {
+            t.printStackTrace(); // ensure _something_ is printed
+            throw new RuntimeException(LOG_ENTRY_PREFIX + msg, t);
+            
+        }
+    }
+    
+    static class NoopLog extends Log {
+
+        @Override
+        public void log(String msg, Object... args) {
+            // empty by design
+        }
+        
+        @Override
+        public void fatal(String message, Throwable t) {
+            // empty by design
+        }
+    }
+}
diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
index a14c700..3a343ba 100644
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
+++ b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/AgentIT.java
@@ -162,7 +162,7 @@ public class AgentIT {
         ProcessBuilder pb = new ProcessBuilder(
             javaExe.toString(),
             "-showversion",
-            "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds),
+            "-javaagent:" + jar +"=" + TimeUnit.SECONDS.toMillis(connectTimeoutSeconds) +"," + TimeUnit.SECONDS.toMillis(readTimeoutSeconds)+",v",
             "-cp",
             classPath,
             "org.apache.sling.uca.impl.Main",