dapr
feature(trace): add context to Logger API
Signed-off-by: 1046102779 [email protected]
Description
At present, our servers need to correlate logs and traces.
This is a very urgent matter, I hope everyone can review these code at quickly. Thanks.
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
- [ ] Logger API with context.
- [ ] Support Opentelemetry.
- [ ] Unit tests.
/cc @pkedy @yaron2 @artursouza @daixiang0 @berndverst @ItalyPaleAle
Add package trace to associate micrologic and dapr runtime. as follows:
- service type: client && server,
- protocol type: http && gRPC
gRPC client(go-sdk):
// NewClientWithAddress instantiates Dapr using specific address (including port).
func NewClientWithAddress(address string) (client cc.Client, err error) {
if address == "" {
return nil, errors.New("nil address")
}
logger.Printf("dapr client initializing for: %s", address)
ctx, ctxCancel := context.WithTimeout(context.Background(), 1*time.Second)
conn, err := grpc.DialContext(
ctx,
address,
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithBlock(),
grpc.WithUnaryInterceptor(trace.GRPCClientUnaryInterceptor),
grpc.WithStreamInterceptor(trace.GRPCClientStreamInterceptor),
)
if err != nil {
ctxCancel()
return nil, errors.Wrapf(err, "error creating connection to '%s': %v", address, err)
}
if hasToken := os.Getenv(apiTokenEnvVarName); hasToken != "" {
logger.Println("client uses API token")
}
return newClientWithConnectionAndCancelFunc(conn, ctxCancel), nil
}
gRPC server(go-sdk):
// Start registers the server and starts it.
func (s *Server) Start() error {
gs := grpc.NewServer(
grpc.UnaryInterceptor(trace.GRPCServerUnaryInterceptor),
grpc.StreamInterceptor(trace.GRPCServerStreamInterceptor),
)
runtimepbv1.RegisterAppCallbackServer(gs, s)
s.grpcServer = gs
return gs.Serve(s.listener)
}
/cc @yaron2
We desperately need this Logger Context API feature.
ps. It seems that this repo is not maintained anymore, can I be the maintainer of this repo? Currently feedback is slow for issues and PRs .
/cc @yaron2
We desperately need this Logger Context API feature.
ps. It seems that this repo is not maintained anymore, can I be the maintainer of this repo? Currently feedback is slow for issues and PRs .
This repo is maintained, but it does not frequently require changes. In a way, it is controlled by Dapr runtime maintainers.
Codecov Report
Merging #20 (b91bb22) into main (fcb0995) will decrease coverage by
4.29%. The diff coverage is63.19%.
@@ Coverage Diff @@
## main #20 +/- ##
==========================================
- Coverage 90.46% 86.17% -4.29%
==========================================
Files 18 22 +4
Lines 891 1049 +158
==========================================
+ Hits 806 904 +98
- Misses 72 116 +44
- Partials 13 29 +16
| Impacted Files | Coverage Δ | |
|---|---|---|
| logger/logger.go | 100.00% <ø> (ø) |
|
| logger/nop_logger.go | 0.00% <0.00%> (ø) |
|
| trace/trace.go | 50.00% <50.00%> (ø) |
|
| trace/adapter.go | 60.52% <60.52%> (ø) |
|
| trace/span_context.go | 61.11% <61.11%> (ø) |
|
| trace/id_generator.go | 78.26% <78.26%> (ø) |
|
| logger/dapr_logger.go | 96.82% <100.00%> (+0.74%) |
:arrow_up: |
| logger/options.go | 82.35% <100.00%> (+1.70%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
id(traceid): 196d92cb7ed0571d7129a803cba99bed
➜ logger git:(feature/log0905) go test -v ./... -test.run=TestJSONLoggerFieldsWithSpanCo
ntext
=== RUN TestJSONLoggerFieldsWithSpanContext
=== RUN TestJSONLoggerFieldsWithSpanContext/info()
line: {"app_id":"dapr_app","instance":"dapr-pod","level":"info","msg":"King Dapr","scope":"fakeLogger","time":"2022-09-07T10:34:58.008556446+08:00","type":"log","ver":"unknown"}
=== RUN TestJSONLoggerFieldsWithSpanContext/info()#01
line: {"app_id":"dapr_app","id":"196d92cb7ed0571d7129a803cba99bed","instance":"dapr-pod","level":"info","msg":"King Dapr","scope":"fakeLogger","time":"2022-09-07T10:34:58.008773554+08:00","type":"log","ver":"dapr_app"}
--- PASS: TestJSONLoggerFieldsWithSpanContext (0.00s)
--- PASS: TestJSONLoggerFieldsWithSpanContext/info() (0.00s)
--- PASS: TestJSONLoggerFieldsWithSpanContext/info()#01 (0.00s)
PASS
ok github.com/dapr/kit/logger 0.006s
/cc @yaron2
We desperately need this Logger Context API feature.
ps. It seems that this repo is not maintained anymore, can I be the maintainer of this repo? Currently feedback is slow for issues and PRs .
I sent an email to [email protected], please check.
The code style in the screenshot is imported from a third-party library: google.golang.org/grpc/examples/features/proto/echo, this error cannot be handled
https://github.com/grpc/grpc-go/blob/master/examples/features/proto/echo/echo_grpc.pb.go#L236
@1046102779 Please do not ping maintainers every day.
As for this PR, I still am not clear exactly what this is for. I am also concerned with importing an entire tracing package (and dependencies on things like OTEL) in dapr/kit.
Can you help me understand why this is necessary? How will this be used in Dapr?
@1046102779 Please do not ping maintainers every day.
As for this PR, I still am not clear exactly what this is for. I am also concerned with importing an entire tracing package (and dependencies on things like OTEL) in dapr/kit.
Can you help me understand why this is necessary? How will this be used in Dapr?
@ItalyPaleAle as discussed, this is to add a trace id if present to the log output to help in correlating service calls with logs.
(MicroLogic, Dapr runtime)'s trace and log can be associated, and the effect is as follows:
Ok makes sense now.
My concern with code duplication remains. A lot of the code here seems to be present in dapr/dapr too, and it would cause duplications.
Can you (maybe in a separate PR first) move some of that code from dapr/dapr into dapr/kit, and then use those packages in both dapr/kit and dapr/dapr?