wu-sheng commented on a change in pull request #6194:
URL: https://github.com/apache/skywalking/pull/6194#discussion_r558845227



##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin

Review comment:
       ```suggestion
   # gRPC reporter
   ```

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin
+
+The grpc log plugin support report log to grpc Server. 
+
+* Dependency the toolkit, such as using maven or gradle
+
+```xml
+   <dependency>
+      <groupId>org.apache.skywalking</groupId>
+      <artifactId>apm-toolkit-log4j-1.x</artifactId>
+      <version>{project.release.version}</version>
+   </dependency>
+```

Review comment:
       ```suggestion
   ```
   
   Duplicated.

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin

Review comment:
       You also should add this title before `Config a layout`.
   ```
   # Print trace ID in your logs
   ```

##########
File path: docs/en/setup/service-agent/java-agent/README.md
##########
@@ -159,6 +159,10 @@ property key | Description | Default |
 `plugin.kafka.topic_profilings` | Specify which Kafka topic name for Thread 
Profiling snapshot to report to. | `skywalking_profilings` |
 `plugin.kafka.topic_management` | Specify which Kafka topic name for the 
register or heartbeat data of Service Instance to report to. | 
`skywalking_managements` |
 `plugin.springannotation.classname_match_regex` |  Match spring beans with 
regular expression for the class name. Multiple expressions could be separated 
by a comma. This only works when `Spring annotation plugin` has been activated. 
| `All the spring beans tagged with @Bean,@Service,@Dao, or @Repository.` |
+`plugin.grpclog.service_host` | Specify which grpc server's host for log data 
to report to. | `127.0.0.1` |
+`plugin.grpclog.service_port` | Specify which grpc server's port for traces 
data to report to. | `8000` |
+`plugin.grpclog.max_message_size` | Specify the maximum size of log data for 
grpc client to report to. | `10MB` |
+`plugin.grpclog.upstream_timeout` | How long grpc client will timeout in 
sending data to upstream. Unit is second.|`30` seconds|

Review comment:
       There is another place of document should be updated
   
   ```md
       * If you want to print trace context(e.g. traceId) in your logs, choose 
the log frameworks, [log4j](Application-toolkit-log4j-1.x.md), 
   [log4j2](Application-toolkit-log4j-2.x.md), 
[logback](Application-toolkit-logback-1.x.md)
   ```
   Should change to 
   > If you want to print trace context(e.g. traceId) in your logs, or collect 
logs, ...

##########
File path: docs/en/setup/service-agent/java-agent/README.md
##########
@@ -159,6 +159,10 @@ property key | Description | Default |
 `plugin.kafka.topic_profilings` | Specify which Kafka topic name for Thread 
Profiling snapshot to report to. | `skywalking_profilings` |
 `plugin.kafka.topic_management` | Specify which Kafka topic name for the 
register or heartbeat data of Service Instance to report to. | 
`skywalking_managements` |
 `plugin.springannotation.classname_match_regex` |  Match spring beans with 
regular expression for the class name. Multiple expressions could be separated 
by a comma. This only works when `Spring annotation plugin` has been activated. 
| `All the spring beans tagged with @Bean,@Service,@Dao, or @Repository.` |
+`plugin.grpclog.service_host` | Specify which grpc server's host for log data 
to report to. | `127.0.0.1` |
+`plugin.grpclog.service_port` | Specify which grpc server's port for traces 
data to report to. | `8000` |
+`plugin.grpclog.max_message_size` | Specify the maximum size of log data for 
grpc client to report to. | `10MB` |
+`plugin.grpclog.upstream_timeout` | How long grpc client will timeout in 
sending data to upstream. Unit is second.|`30` seconds|

Review comment:
       The config hierarchy seems doesn't make sense. You are putting reporter 
at level one, but nothing related toolkit. They should use 
`toolkit.log.grpc.reporter` for all these RPC related things.

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin
+
+The grpc log plugin support report log to grpc Server. 

Review comment:
       ```suggestion
   The gRPC report could forward the collected logs to SkyWalking OAP server, 
or [SkyWalking Satellite 
sidecar](https://github.com/apache/skywalking-satellite). Trace id, segment id, 
and span id will attach to logs automatically. You don't need to change the 
layout.
   ```

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin
+
+The grpc log plugin support report log to grpc Server. 
+
+* Dependency the toolkit, such as using maven or gradle
+
+```xml
+   <dependency>
+      <groupId>org.apache.skywalking</groupId>
+      <artifactId>apm-toolkit-log4j-1.x</artifactId>
+      <version>{project.release.version}</version>
+   </dependency>
+```
+
+* add `GRPCLogClientAppender` in logback.xml
+
+```xml
+    <appender name="grpc-log" 
class="org.apache.skywalking.apm.toolkit.log.log4j.v1.x.log.GRPCLogClientAppender">
+    </appender>
+```
+
+*  modify config of the plugin 
+```properties
+plugin.grpclog.service_host=${SW_GRPC_LOG_SERVER_HOST:127.0.0.1}
+plugin.grpclog.service_port=${SW_GRPC_LOG_SERVER_PORT:8000}
+plugin.grpclog.max_message_size=${SW_GRPC_LOG_MAX_MESSAGE_SIZE:10485760}
+plugin.grpclog.upstream_timeout=${SW_GRPC_LOG_GRPC_UPSTREAM_TIMEOUT:30}
+```

Review comment:
       All similar changes should be applied to all toolkit documentations.

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin
+
+The grpc log plugin support report log to grpc Server. 

Review comment:
       When you write documents, please keep in your mind, the reader will be 
like you when you first time read SkyWalking's documents. If you keep too many 
things in the dark, they will be very confused.

##########
File path: 
apm-sniffer/apm-toolkit-activation/apm-toolkit-logback-1.x-activation/src/main/java/org/apache/skywalking/apm/toolkit/activation/log/logback/v1/x/log/GRPCLogAppenderInterceptor.java
##########
@@ -0,0 +1,93 @@
+/*
+ * 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.skywalking.apm.toolkit.activation.log.logback.v1.x.log;
+
+import java.lang.reflect.Method;
+import java.util.Objects;
+
+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;
+import 
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import 
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
+import 
org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
+import org.apache.skywalking.apm.agent.core.remote.LogReportServiceClient;
+import org.apache.skywalking.apm.network.common.v3.KeyStringValuePair;
+import org.apache.skywalking.apm.network.logging.v3.LogData;
+import org.apache.skywalking.apm.network.logging.v3.LogDataBody;
+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 ch.qos.logback.classic.spi.ILoggingEvent;
+
+public class GRPCLogAppenderInterceptor implements 
InstanceMethodsAroundInterceptor {
+
+    private LogReportServiceClient client;
+
+    @Override
+    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] 
allArguments, Class<?>[] argumentsTypes,
+                             MethodInterceptResult result) throws Throwable {
+        if (Objects.isNull(client)) {
+            client = 
ServiceManager.INSTANCE.findService(LogReportServiceClient.class);
+            if (Objects.isNull(client)) {
+                return;
+            }
+        }
+        ILoggingEvent event = (ILoggingEvent) allArguments[0];
+        if (Objects.nonNull(event)) {
+            client.produce(transform(event));
+        }
+    }
+
+    @Override
+    public Object afterMethod(EnhancedInstance objInst, Method method, 
Object[] allArguments, Class<?>[] argumentsTypes,
+                              Object ret) throws Throwable {
+        return ret;
+    }
+
+    @Override
+    public void handleMethodException(EnhancedInstance objInst, Method method, 
Object[] allArguments,
+                                      Class<?>[] argumentsTypes, Throwable t) {
+
+    }
+
+    private LogData transform(ILoggingEvent event) {
+        return LogData.newBuilder()
+                .setTimestamp(event.getTimeStamp())
+                .setService(Config.Agent.SERVICE_NAME)
+                .setServiceInstance(Config.Agent.INSTANCE_NAME)
+                .setTraceContext(TraceContext.newBuilder()
+                        .setTraceId(ContextManager.getGlobalTraceId())
+                        .setSpanId(ContextManager.getSpanId())
+                        .setTraceSegmentId(ContextManager.getSegmentId())

Review comment:
       @EvanLjp As we are always sending this, should we still keep trace id as 
empty string, if there is no tracing context? What do you think?

##########
File path: 
docs/en/setup/service-agent/java-agent/Application-toolkit-log4j-1.x.md
##########
@@ -18,3 +18,32 @@ log4j.appender.CONSOLE.layout.ConversionPattern=%d [%T] %-5p 
%c{1}:%L - %m%n
 ```
 
 * When you use `-javaagent` to active the sky-walking tracer, log4j will 
output **traceId**, if it existed. If the tracer is inactive, the output will 
be `TID: N/A`.
+
+# grpc log log4j plugin
+
+The grpc log plugin support report log to grpc Server. 
+
+* Dependency the toolkit, such as using maven or gradle
+
+```xml
+   <dependency>
+      <groupId>org.apache.skywalking</groupId>
+      <artifactId>apm-toolkit-log4j-1.x</artifactId>
+      <version>{project.release.version}</version>
+   </dependency>
+```
+
+* add `GRPCLogClientAppender` in logback.xml

Review comment:
       This is `log4j` document. Please check the copy/paste issue.




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