Enhance gRPC log appender to allow layout pattern (#6403)

apache · Feb 21, 2021 · 9de9311 · 9de9311
1 parent 317a658
commit 9de9311
Show file tree

Hide file tree

Showing 16 changed files with 175 additions and 52 deletions.
diff --git a/.github/actions/e2e-test/action.yml b/.github/actions/e2e-test/action.yml
@@ -25,6 +25,17 @@ inputs:
 runs:
   using: "composite"
   steps:
+    - name: Check Unintended Changes
+      shell: bash
+      run: |
+        echo "::group::Check sw.version"
+        sudo apt install -y -q xmlstarlet
+        SW_VERSION=$(xmlstarlet sel -N pom=http://maven.apache.org/POM/4.0.0 -t -v "/pom:project/pom:properties/pom:sw.version" test/e2e/pom.xml)
+        if [[ "$(echo $(echo $SW_VERSION))" != "" ]]; then
+          echo "Please don't submit the change of sw.version in test/e2e/pom.xml"
+          exit 1
+        fi
+        echo "::endgroup::"
     - name: Check License
       shell: bash
       run: |
@@ -53,8 +64,14 @@ runs:
     - name: Run E2E Test
       shell: bash
       run: |
+        echo "::group::Install SNAPSHOT apm-application-toolkit"
+        ./mvnw -DskipTests -N install
+        ./mvnw -f apm-application-toolkit -DskipTests -am install
+        echo "::endgroup::"
+
         echo "::group::Run E2E Test ${{ inputs.test_class }}"
-        ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false verify -Dit.test=${{ inputs.test_class }}
+        SW_VERSION=$(./mvnw -q -DforceStdout -N org.apache.maven.plugins:maven-help-plugin:3.2.0:evaluate -Dexpression=project.version)
+        ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false -Dsw.version=${SW_VERSION} verify -Dit.test=${{ inputs.test_class }}
         echo "::endgroup::"
     - name: Report Coverage
       shell: bash

diff --git a/.github/actions/plugins-test/action.yml b/.github/actions/plugins-test/action.yml
@@ -29,6 +29,17 @@ inputs:
 runs:
   using: "composite"
   steps:
+    - name: Check Unintended Changes
+      shell: bash
+      run: |
+        echo "::group::Check sw.version"
+        sudo apt install -y -q xmlstarlet
+        SW_VERSION=$(xmlstarlet sel -N pom=http://maven.apache.org/POM/4.0.0 -t -v "/pom:project/pom:properties/pom:sw.version" test/e2e/pom.xml)
+        if [[ "$(echo $(echo $SW_VERSION))" != "" ]]; then
+          echo "Please don't submit the change of sw.version in test/e2e/pom.xml"
+          exit 1
+        fi
+        echo "::endgroup::"
     - name: Check License
       shell: bash
       run: |

diff --git a/.github/workflows/e2e.istio.yaml b/.github/workflows/e2e.istio.yaml
@@ -48,6 +48,17 @@ jobs:
         with:
           submodules: true
 
+      - name: Check Unintended Changes
+        run: |
+          echo "::group::Check sw.version"
+          sudo apt install -y -q xmlstarlet
+          SW_VERSION=$(xmlstarlet sel -N pom=http://maven.apache.org/POM/4.0.0 -t -v "/pom:project/pom:properties/pom:sw.version" test/e2e/pom.xml)
+          if [[ "$(echo $(echo $SW_VERSION))" != "" ]]; then
+            echo "::error Please don't submit the change of sw.version in test/e2e/pom.xml"
+            exit 1
+          fi
+          echo "::endgroup::"
+
       - name: Set Skip Env Var
         uses: ./.github/actions/skip
 
@@ -132,7 +143,12 @@ jobs:
           export WEBAPP_HOST=127.0.0.1
           export WEBAPP_PORT=8080
 
-          ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false verify -Dit.test=org.apache.skywalking.e2e.mesh.ALSE2E
+          export SW_VERSION=$(./mvnw -q -DforceStdout -N org.apache.maven.plugins:maven-help-plugin:3.2.0:evaluate -Dexpression=project.version)
+
+          ./mvnw -DskipTests -N install
+          ./mvnw -f apm-application-toolkit -DskipTests -am install
+
+          ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false -Dsw.version=${SW_VERSION} verify -Dit.test=org.apache.skywalking.e2e.mesh.ALSE2E
 
       - name: Logs
         if: ${{ failure() }}
@@ -162,6 +178,17 @@ jobs:
         with:
           submodules: true
 
+      - name: Check Unintended Changes
+        run: |
+          echo "::group::Check sw.version"
+          sudo apt install -y -q xmlstarlet
+          SW_VERSION=$(xmlstarlet sel -N pom=http://maven.apache.org/POM/4.0.0 -t -v "/pom:project/pom:properties/pom:sw.version" test/e2e/pom.xml)
+          if [[ "$(echo $(echo $SW_VERSION))" != "" ]]; then
+            echo "Please don't submit the change of sw.version in test/e2e/pom.xml"
+            exit 1
+          fi
+          echo "::endgroup::"
+
       - name: Set Skip Env Var
         uses: ./.github/actions/skip
 
@@ -246,7 +273,12 @@ jobs:
           export WEBAPP_HOST=127.0.0.1
           export WEBAPP_PORT=8080
 
-          ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false verify -Dit.test=org.apache.skywalking.e2e.mesh.MetricsServiceE2E
+          export SW_VERSION=$(./mvnw -q -DforceStdout -N org.apache.maven.plugins:maven-help-plugin:3.2.0:evaluate -Dexpression=project.version)
+
+          ./mvnw -DskipTests -N install
+          ./mvnw -f apm-application-toolkit -DskipTests -am install
+
+          ./mvnw --batch-mode -f test/e2e/pom.xml -am -DfailIfNoTests=false -Dsw.version=${SW_VERSION} verify -Dit.test=org.apache.skywalking.e2e.mesh.MetricsServiceE2E
 
       - name: Logs
         if: ${{ failure() }}

diff --git a/CHANGES.md b/CHANGES.md
@@ -13,6 +13,7 @@ Release Notes.
 * Add net.bytebuddy.agent.builder.AgentBuilder.RedefinitionStrategy.Listener to show detail message when redefine errors occur.
 * Fix ClassCastException of log4j gRPC reporter.
 * Fix NPE when Kafka reporter activated.
+* Enhance gRPC log appender to allow layout pattern.
 
 #### OAP-Backend
 * Allow user-defined `JAVA_OPTS` in the startup script.

diff --git a/...main/java/org/apache/skywalking/apm/toolkit/log/log4j/v1/x/log/GRPCLogClientAppender.java b/...main/java/org/apache/skywalking/apm/toolkit/log/log4j/v1/x/log/GRPCLogClientAppender.java
@@ -19,9 +19,18 @@
 package org.apache.skywalking.apm.toolkit.log.log4j.v1.x.log;
 
 import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.Layout;
 import org.apache.log4j.spi.LoggingEvent;
 
 public class GRPCLogClientAppender extends AppenderSkeleton {
+
+    public GRPCLogClientAppender() {
+    }
+
+    public GRPCLogClientAppender(Layout layout) {
+        this.setLayout(layout);
+    }
+
     @Override
     protected void append(LoggingEvent loggingEvent) {
 
@@ -34,6 +43,6 @@ public void close() {
 
     @Override
     public boolean requiresLayout() {
-        return false;
+        return true;
     }
 }
diff --git a/...main/java/org/apache/skywalking/apm/toolkit/log/log4j/v2/x/log/GRPCLogClientAppender.java b/...main/java/org/apache/skywalking/apm/toolkit/log/log4j/v2/x/log/GRPCLogClientAppender.java
@@ -18,9 +18,14 @@
 
 package org.apache.skywalking.apm.toolkit.log.log4j.v2.x.log;
 
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.Serializable;
 import org.apache.logging.log4j.core.Filter;
+import org.apache.logging.log4j.core.Layout;
 import org.apache.logging.log4j.core.LogEvent;
-import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.appender.AbstractOutputStreamAppender;
+import org.apache.logging.log4j.core.appender.OutputStreamManager;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
 import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
@@ -29,25 +34,45 @@
 import org.apache.logging.log4j.core.config.plugins.PluginFactory;
 
 @Plugin(name = "GRPCLogClientAppender", category = "Core", elementType = "appender")
-public class GRPCLogClientAppender extends AbstractAppender {
+public class GRPCLogClientAppender extends AbstractOutputStreamAppender<OutputStreamManager> {
+    private static final OutputStream DISCARDED_STREAM = new OutputStream() {
+        @Override
+        public void write(final int b) throws IOException {
+            // discarded
+        }
+    };
 
-    private GRPCLogClientAppender(final String name, final Filter filter, final boolean ignoreExceptions) {
-        super(name, filter, null, ignoreExceptions);
+    protected GRPCLogClientAppender(final String name,
+                                    final Layout<? extends Serializable> layout,
+                                    final Filter filter,
+                                    final boolean ignoreExceptions) {
+        super(
+            name,
+            layout,
+            filter,
+            ignoreExceptions,
+            true,
+            getManager0(layout)
+        );
     }
 
     @Override
-    public void append(LogEvent logEvent) {
-
+    public void append(final LogEvent event) {
     }
 
     @PluginFactory
     public static GRPCLogClientAppender createAppender(@PluginAttribute("name") final String name,
+                                                       @PluginElement("Layout") final Layout<? extends Serializable> layout,
                                                        @PluginElement("Filter") final Filter filter,
                                                        @PluginConfiguration final Configuration config,
                                                        @PluginAttribute("ignoreExceptions") final String ignore) {
-
         String appenderName = name == null ? "gRPCLogClientAppender" : name;
         final boolean ignoreExceptions = "true".equalsIgnoreCase(ignore) || !"false".equalsIgnoreCase(ignore);
-        return new GRPCLogClientAppender(appenderName, filter, ignoreExceptions);
+        return new GRPCLogClientAppender(appenderName, layout, filter, ignoreExceptions);
+    }
+
+    private static OutputStreamManager getManager0(final Layout<? extends Serializable> layout) {
+        return OutputStreamManager.getManager("Discard", new Object(), (s, o) -> new OutputStreamManager(DISCARDED_STREAM, "Discard", layout, false) {
+        });
     }
 }
diff --git a/...in/java/org/apache/skywalking/apm/toolkit/log/logback/v1/x/log/GRPCLogClientAppender.java b/...in/java/org/apache/skywalking/apm/toolkit/log/logback/v1/x/log/GRPCLogClientAppender.java
@@ -18,12 +18,21 @@
 
 package org.apache.skywalking.apm.toolkit.log.logback.v1.x.log;
 
-import ch.qos.logback.core.AppenderBase;
+import ch.qos.logback.core.OutputStreamAppender;
+import java.io.IOException;
+import java.io.OutputStream;
 
-public class GRPCLogClientAppender<E> extends AppenderBase<E> {
+public class GRPCLogClientAppender<E> extends OutputStreamAppender<E> {
+    public GRPCLogClientAppender() {
+        setOutputStream(new OutputStream() {
+            @Override
+            public void write(final int b) throws IOException {
+                // discarded
+            }
+        });
+    }
 
     @Override
-    protected void append(E eventObject) {
+    protected void subAppend(final E event) {
     }
-
 }
diff --git a/...ache/skywalking/apm/toolkit/activation/log/log4j/v1/x/log/GRPCLogAppenderInterceptor.java b/...ache/skywalking/apm/toolkit/activation/log/log4j/v1/x/log/GRPCLogAppenderInterceptor.java
@@ -20,7 +20,8 @@
 
 import java.lang.reflect.Method;
 import java.util.Objects;
-
+import org.apache.log4j.AppenderSkeleton;
+import org.apache.log4j.spi.LoggingEvent;
 import org.apache.skywalking.apm.agent.core.boot.ServiceManager;
 import org.apache.skywalking.apm.agent.core.conf.Config;
 import org.apache.skywalking.apm.agent.core.context.ContextManager;
@@ -35,7 +36,6 @@
 import org.apache.skywalking.apm.network.logging.v3.LogTags;
 import org.apache.skywalking.apm.network.logging.v3.TextLog;
 import org.apache.skywalking.apm.network.logging.v3.TraceContext;
-import org.apache.log4j.spi.LoggingEvent;
 
 public class GRPCLogAppenderInterceptor implements InstanceMethodsAroundInterceptor {
 
@@ -52,7 +52,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
         }
         LoggingEvent event = (LoggingEvent) allArguments[0];
         if (Objects.nonNull(event)) {
-            client.produce(transform(event));
+            client.produce(transform((AppenderSkeleton) objInst, event));
         }
     }
 
@@ -71,10 +71,12 @@ public void handleMethodException(EnhancedInstance objInst, Method method, Objec
     /**
      * transforms {@link LoggingEvent}  to {@link LogData}
      *
+     *
+     * @param appender the real {@link AppenderSkeleton appender}
      * @param event {@link LoggingEvent}
      * @return {@link LogData} with filtered trace context in order to reduce the cost on the network
      */
-    private LogData transform(LoggingEvent event) {
+    private LogData transform(final AppenderSkeleton appender, LoggingEvent event) {
         LogData.Builder builder = LogData.newBuilder()
                 .setTimestamp(event.getTimeStamp())
                 .setService(Config.Agent.SERVICE_NAME)
@@ -93,7 +95,7 @@ private LogData transform(LoggingEvent event) {
                                 .setKey("thread").setValue(event.getThreadName()).build())
                         .build())
                 .setBody(LogDataBody.newBuilder().setType(LogDataBody.ContentCase.TEXT.name())
-                                    .setText(TextLog.newBuilder().setText(transformLogText(event)).build()).build());
+                                    .setText(TextLog.newBuilder().setText(transformLogText(appender, event)).build()).build());
         return -1 == ContextManager.getSpanId() ? builder.build()
                 : builder.setTraceContext(TraceContext.newBuilder()
                         .setTraceId(ContextManager.getGlobalTraceId())
@@ -102,8 +104,11 @@ private LogData transform(LoggingEvent event) {
                         .build()).build();
     }
 
-    private String transformLogText(final LoggingEvent event) {
-        final String throwableString = Objects.isNull(event.getThrowableInformation()) ? "" : 
+    private String transformLogText(final AppenderSkeleton appender, final LoggingEvent event) {
+        if (appender.getLayout() != null) {
+            return appender.getLayout().format(event);
+        }
+        final String throwableString = Objects.isNull(event.getThrowableInformation()) ? "" :
             ThrowableTransformer.INSTANCE.convert2String(event.getThrowableInformation().getThrowable(), 2048);
         return event.getMessage() + "\n" + throwableString;
     }

diff --git a/...ache/skywalking/apm/toolkit/activation/log/log4j/v2/x/log/GRPCLogAppenderInterceptor.java b/...ache/skywalking/apm/toolkit/activation/log/log4j/v2/x/log/GRPCLogAppenderInterceptor.java
@@ -21,8 +21,8 @@
 import java.lang.reflect.Method;
 import java.util.Objects;
 import java.util.Optional;
-
 import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.skywalking.apm.agent.core.boot.ServiceManager;
 import org.apache.skywalking.apm.agent.core.conf.Config;
 import org.apache.skywalking.apm.agent.core.context.ContextManager;
@@ -54,7 +54,7 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
         }
         LogEvent event = (LogEvent) allArguments[0];
         if (Objects.nonNull(event)) {
-            client.produce(transform(event));
+            client.produce(transform((AbstractAppender) objInst, event));
         }
     }
 
@@ -73,10 +73,12 @@ public void handleMethodException(EnhancedInstance objInst, Method method, Objec
     /**
      * transforms {@link LogEvent}  to {@link LogData}
      *
+     *
+     * @param appender the real {@link AbstractAppender appender}
      * @param event {@link LogEvent}
      * @return {@link LogData} with filtered trace context in order to reduce the cost on the network
      */
-    private LogData transform(LogEvent event) {
+    private LogData transform(final AbstractAppender appender, LogEvent event) {
         LogTags.Builder logTags = LogTags.newBuilder()
                 .addData(KeyStringValuePair.newBuilder()
                         .setKey("level").setValue(event.getLevel().toString()).build())
@@ -105,7 +107,7 @@ private LogData transform(LogEvent event) {
                 .setServiceInstance(Config.Agent.INSTANCE_NAME)
                 .setTags(logTags.build())
                 .setBody(LogDataBody.newBuilder().setType(LogDataBody.ContentCase.TEXT.name())
-                        .setText(TextLog.newBuilder().setText(transformLogText(event)).build()).build());
+                        .setText(TextLog.newBuilder().setText(transformLogText(appender, event)).build()).build());
         return -1 == ContextManager.getSpanId() ? builder.build()
                 : builder.setTraceContext(TraceContext.newBuilder()
                 .setTraceId(ContextManager.getGlobalTraceId())
@@ -114,8 +116,11 @@ private LogData transform(LogEvent event) {
                 .build()).build();
     }
 
-    private String transformLogText(final LogEvent event) {
+    private String transformLogText(final AbstractAppender appender, final LogEvent event) {
         if (ToolkitConfig.Plugin.Toolkit.Log.TRANSMIT_FORMATTED) {
+            if (appender.getLayout() != null) {
+                return new String(appender.getLayout().toByteArray(event));
+            }
             return event.getMessage().getFormattedMessage() + "\n" + ThrowableTransformer.INSTANCE.convert2String(event.getThrown(), 2048);
         } else {
             return event.getMessage().getFormat();

diff --git a/...che/skywalking/apm/toolkit/activation/log/logback/v1/x/log/GRPCLogAppenderActivation.java b/...che/skywalking/apm/toolkit/activation/log/logback/v1/x/log/GRPCLogAppenderActivation.java
@@ -39,7 +39,7 @@ public class GRPCLogAppenderActivation extends ClassInstanceMethodsEnhancePlugin
             "org.apache.skywalking.apm.toolkit.activation.log.logback.v1.x.log.GRPCLogAppenderInterceptor";
     public static final String ENHANCE_CLASS =
             "org.apache.skywalking.apm.toolkit.log.logback.v1.x.log.GRPCLogClientAppender";
-    public static final String ENHANCE_METHOD = "append";
+    public static final String ENHANCE_METHOD = "subAppend";
 
     @Override
     protected ClassMatch enhanceClass() {