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]