SkyAPM / go2sky

Distributed tracing and monitor SDK in Go for Apache SkyWalking APM
https://skywalking.apache.org/
Apache License 2.0
447 stars 122 forks source link

Log & Trace integration #104

Closed wu-sheng closed 3 years ago

wu-sheng commented 3 years ago

SkyWalking is enhancing its log integration capability, so, it will accept logs collected by popular tools, such as fluentd, filebeat. It is better go2sky could inject the trace context information as a part of log text, which could make SkyWalking backend links logs and traces together. You could take Java agent's implementation for a reference, http://skywalking.apache.org/docs/main/v8.5.0/en/setup/service-agent/java-agent/application-toolkit-log4j-1.x/#print-skywalking-context-in-your-logs

mrproliu commented 3 years ago

I have done a little research, logrus and zap are more user use and popular framework on logging, also they use the MIT License, so we could integrate these frameworks. Unfortunately, the Golang logging framework is not such as java has MDC or ThreadLocal mechanism, so we need to pass context data at all the log method invoke.

Logrus has the log format mechanism and transmits the context data when format. such as log.SetFormatter(&log.JSONFormatter{}), so we could wrap the format and adding the trace fields when the user invokes the log method with context data. When user integration, the user need to do two things:

  1. Setting customizes format, eg. log.SetFormatter(&logrusplugin.WrapFormat{base: &log.JSONFormatter}).
  2. Add context before log, eg. log.WithContext(ctx).Info("test").

Zap does not have the contextual metadata mechanism, also I find an issue is still open to tracking this. after reading this, I think we could have two ways to let user integration.

  1. Such as elastic apm, just create a built-in method for the user: https://www.elastic.co/guide/en/apm/agent/go/master/builtin-modules.html#builtin-modules-apmzap.
  2. Wrap the logging, eg. logging = zapplugin.WrapWithContext(logging). after the wrap, such as logrus, the user needs to add context before log or as an argument introduction when logging, eg. logging.Ctx(ctx).Info("test") or logging.Info(ctx, "test"). When the logging method has been invoking, we could add trace fields before logging.

Through these framework integrations, we could support add trace Id to the logging data. If we also need gRPC reporter, logrus we only need to create a setting method to connect logrus with our Tracer object, zap is the same thing(in this way, I suggest use wrap way to integration, so we could get more information and more control before logging).

wu-sheng commented 3 years ago

About Zap, I think you could have both. Wrapper is easier to use for new codes, but (1) could change fewer existing codes, at least only change in one place.

mrproliu commented 3 years ago

About Zap, I think you could have both. Wrapper is easier to use for new codes, but (1) could change fewer existing codes, at least only change in one place.

Of course. We could have both.

wu-sheng commented 3 years ago

@mrproliu Sorry, I gave you a wrong reference. We could read 8.6.0-dev doc about context injection.

http://skywalking.apache.org/docs/main/latest/en/setup/service-agent/java-agent/application-toolkit-log4j-1.x/#print-skywalking-context-in-your-logs

Print SkyWalking context in your logs

mrproliu commented 3 years ago

@mrproliu Sorry, I gave you a wrong reference. We could read 8.6.0-dev doc about context injection.

http://skywalking.apache.org/docs/main/latest/en/setup/service-agent/java-agent/application-toolkit-log4j-1.x/#print-skywalking-context-in-your-logs

Print SkyWalking context in your logs

Ok, I think I need to expose some trace context method at go2sky project, such as SpanId(), ServiceInstanceName(). After this PR merged, I will continue working on the log plugins.

wu-sheng commented 3 years ago

Which PR do you mean?

mrproliu commented 3 years ago

Which PR do you mean?

Expose trace context data, current only exposing the trace id field, It needs to submit on the go2sky project. This PR I will submit recently.

wu-sheng commented 3 years ago

Which PR do you mean?

Expose trace context data, current only exposing the trace id field, It needs to submit on the go2sky project. This PR I will submit recently.

Yes, let's go to that first.

mrproliu commented 3 years ago

After the zap and logrus plugin merged. this issue could be close.