Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to zap 1.0 release candidate #290

Merged
merged 13 commits into from
Feb 24, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions auth/uauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ import (
"testing"

"go.uber.org/fx/config"
"go.uber.org/fx/ulog"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber-go/tally"
"go.uber.org/zap"
)

func withAuthClientSetup(t *testing.T, registerFunc RegisterFunc, info CreateAuthInfo, fn func()) {
Expand Down Expand Up @@ -86,8 +86,8 @@ func (fakeAuthInfo) Config() config.Provider {
return nil
}

func (fakeAuthInfo) Logger() ulog.Log {
return ulog.NopLogger
func (fakeAuthInfo) Logger() *zap.Logger {
return zap.NewNop()
}

func (fakeAuthInfo) Metrics() tally.Scope {
Expand Down
6 changes: 2 additions & 4 deletions examples/keyvalue/server/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@
package main

import (
"context"

"go.uber.org/fx/service"
"go.uber.org/fx/ulog"
"go.uber.org/zap"
)

// Observer receives callbacks during various service lifecycle events
Expand All @@ -37,7 +35,7 @@ type Observer struct {

// OnInit is called during service init process. Returning an error halts the init?
func (o *Observer) OnInit(svc service.Host) error {
ulog.Logger(context.Background()).Info(
zap.S().Info(
"Received service init callback",
"service_name", o.Name(),
"some_number", o.ServiceConfig.SomeNumber,
Expand Down
23 changes: 13 additions & 10 deletions glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions glide.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package: go.uber.org/fx
import:
- package: github.com/uber-go/zap
# Pinned temporarily while zap reaches 1.0 (https://github.com/uber-go/fx/pull/243)
version: c064b5c44b285a7e2fd5c9b26e8c38228ce2bccb
- package: go.uber.org/zap
version: v1.0.0-rc.2
- package: github.com/uber-go/tally
version: ^1.1.0
- package: github.com/gorilla/mux
Expand Down
26 changes: 0 additions & 26 deletions internal/context.go

This file was deleted.

17 changes: 5 additions & 12 deletions modules/rpc/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"go.uber.org/fx/auth"
"go.uber.org/fx/service"
"go.uber.org/fx/ulog"
"go.uber.org/zap"

"github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"go.uber.org/yarpc/api/transport"
)
Expand All @@ -50,10 +50,7 @@ func (f contextInboundMiddleware) Handle(
Timer(req.Procedure).
Start()
defer stopwatch.Stop()
// Span is populated by YARPC, we just extract and use it
if span := opentracing.SpanFromContext(ctx); span != nil {
ctx = ulog.ContextWithTraceLogger(ctx, span)
}

return handler.Handle(ctx, req, resw)
}

Expand All @@ -66,10 +63,6 @@ func (f contextOnewayInboundMiddleware) HandleOneway(
req *transport.Request,
handler transport.OnewayHandler,
) error {
// Span is populated by YARPC, we just extract and use it
if span := opentracing.SpanFromContext(ctx); span != nil {
ctx = ulog.ContextWithTraceLogger(ctx, span)
}
return handler.HandleOneway(ctx, req)
}

Expand Down Expand Up @@ -111,7 +104,7 @@ func (a authOnewayInboundMiddleware) HandleOneway(
func authorize(ctx context.Context, host service.Host, statsClient *statsClient) (context.Context, error) {
if err := host.AuthClient().Authorize(ctx); err != nil {
statsClient.RPCAuthFailCounter().Inc(1)
ulog.Logger(ctx).Error(auth.ErrAuthorization, "error", err)
ulog.Logger(ctx).Error(auth.ErrAuthorization, zap.Error(err))
// TODO(anup): GFM-255 update returned error to transport.BadRequestError (user error than server error)
// https://github.com/yarpc/yarpc-go/issues/687
return nil, err
Expand Down Expand Up @@ -149,8 +142,8 @@ func (p panicOnewayInboundMiddleware) HandleOneway(
func panicRecovery(ctx context.Context, statsClient *statsClient) {
if err := recover(); err != nil {
statsClient.RPCPanicCounter().Inc(1)
ulog.Logger(ctx).Error(
"Panic recovered serving request", "error", errors.Errorf("panic in handler: %+v", err),
ulog.Logger(ctx).Error("Panic recovered serving request",
zap.Error(errors.Errorf("panic in handler: %+v", err)),
)
// rethrow panic back to yarpc
// before https://github.com/yarpc/yarpc-go/issues/734 fixed, throw a generic error.
Expand Down
23 changes: 11 additions & 12 deletions modules/rpc/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber-go/tally"
"github.com/uber-go/zap"
"go.uber.org/zap"
ztest "go.uber.org/zap/testutils"
)

type fakeEnveloper struct {
Expand All @@ -58,11 +59,10 @@ func (f fakeEnveloper) ToWire() (wire.Value, error) {
func TestInboundMiddleware_Context(t *testing.T) {
host := service.NopHost()
unary := contextInboundMiddleware{host, newStatsClient(host.Metrics())}
testutils.WithInMemoryLogger(t, nil, func(zapLogger zap.Logger, buf *testutils.TestBuffer) {
loggerWithZap := ulog.Builder().SetLogger(zapLogger).Build()
testutils.WithInMemoryLogger(t, nil, func(loggerWithZap *zap.Logger, buf *ztest.Buffer) {
defer ulog.SetLogger(loggerWithZap)()
tracing.WithSpan(t, loggerWithZap, func(span opentracing.Span) {
ctx := ulog.ContextWithLogger(context.Background(), loggerWithZap)
ctx = opentracing.ContextWithSpan(ctx, span)
ctx := opentracing.ContextWithSpan(context.Background(), span)
err := unary.Handle(ctx, &transport.Request{}, nil, &fakeUnary{t: t})
require.Contains(t, err.Error(), "handle")
checkLogForTrace(t, buf)
Expand All @@ -74,23 +74,22 @@ func TestOnewayInboundMiddleware_Context(t *testing.T) {
oneway := contextOnewayInboundMiddleware{
Host: service.NopHost(),
}
testutils.WithInMemoryLogger(t, nil, func(zapLogger zap.Logger, buf *testutils.TestBuffer) {
loggerWithZap := ulog.Builder().SetLogger(zapLogger).Build()
testutils.WithInMemoryLogger(t, nil, func(loggerWithZap *zap.Logger, buf *ztest.Buffer) {
defer ulog.SetLogger(loggerWithZap)()
tracing.WithSpan(t, loggerWithZap, func(span opentracing.Span) {
ctx := ulog.ContextWithLogger(context.Background(), loggerWithZap)
ctx = opentracing.ContextWithSpan(ctx, span)
ctx := opentracing.ContextWithSpan(context.Background(), span)
err := oneway.HandleOneway(ctx, &transport.Request{}, &fakeOneway{t: t})
require.Contains(t, err.Error(), "oneway handle")
checkLogForTrace(t, buf)
})
})
}

func checkLogForTrace(t *testing.T, buf *testutils.TestBuffer) {
func checkLogForTrace(t *testing.T, buf *ztest.Buffer) {
require.True(t, len(buf.Lines()) > 0)
for _, line := range buf.Lines() {
assert.Contains(t, line, "traceID")
assert.Contains(t, line, "spanID")
assert.Contains(t, line, "trace")
assert.Contains(t, line, "span")
}

}
Expand Down
9 changes: 4 additions & 5 deletions modules/rpc/yarpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@
package rpc

import (
"context"
"errors"
"fmt"
"strconv"
"sync"

"go.uber.org/fx/modules"
"go.uber.org/fx/service"
"go.uber.org/fx/ulog"

errs "github.com/pkg/errors"
"go.uber.org/fx/dig"
Expand All @@ -38,6 +36,7 @@ import (
"go.uber.org/yarpc/api/transport"
"go.uber.org/yarpc/transport/http"
tch "go.uber.org/yarpc/transport/tchannel"
"go.uber.org/zap"
)

// YARPCModule is an implementation of a core RPC module using YARPC.
Expand All @@ -50,7 +49,7 @@ type YARPCModule struct {
modules.ModuleBase
register registerServiceFunc
config yarpcConfig
log ulog.Log
log *zap.Logger
statsClient *statsClient
stateMu sync.RWMutex
isRunning bool
Expand Down Expand Up @@ -246,7 +245,7 @@ func newYARPCModule(
statsClient: newStatsClient(mi.Host.Metrics()),
}

module.log = ulog.Logger(context.Background()).With("moduleName", name)
module.log = zap.L().With(zap.String("moduleName", name))
for _, opt := range options {
if err := opt(&mi); err != nil {
return module, errs.Wrap(err, "unable to apply option to YARPC module")
Expand Down Expand Up @@ -287,7 +286,7 @@ func newYARPCModule(

module.controller.addConfig(module.config)

module.log.Info("Module successfuly created", "inbounds", module.config.Inbounds)
module.log.Info("Module successfuly created", zap.Any("inbounds", module.config.Inbounds))

return module, nil
}
Expand Down
5 changes: 3 additions & 2 deletions modules/uhttp/client/client_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.uber.org/fx/auth"
"go.uber.org/fx/config"
"go.uber.org/fx/ulog"
"go.uber.org/zap"

"github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
Expand Down Expand Up @@ -95,13 +96,13 @@ func authenticationOutbound(info auth.CreateAuthInfo) OutboundMiddlewareFunc {

authCtx, err = authClient.Authenticate(authCtx)
if err != nil {
ulog.Logger(ctx).Error(auth.ErrAuthentication, "error", err)
ulog.Logger(ctx).Error(auth.ErrAuthentication, zap.Error(err))
return nil, err
}

span := opentracing.SpanFromContext(authCtx)
if err := injectSpanIntoHeaders(req.Header, span); err != nil {
ulog.Logger(authCtx).Error("Error injecting auth context", "error", err)
ulog.Logger(authCtx).Error("Error injecting auth context", zap.Error(err))
return nil, err
}

Expand Down
4 changes: 2 additions & 2 deletions modules/uhttp/client/client_middleware_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@ import (
"go.uber.org/fx/auth"
"go.uber.org/fx/metrics"
"go.uber.org/fx/tracing"
"go.uber.org/fx/ulog"

"github.com/opentracing/opentracing-go"
jconfig "github.com/uber/jaeger-client-go/config"
"go.uber.org/zap"
)

// BenchmarkClientMiddleware/empty-8 100000000 10.8 ns/op 0 B/op 0 allocs/op
// BenchmarkClientMiddleware/tracing-8 500000 3918 ns/op 1729 B/op 27 allocs/op
// BenchmarkClientMiddleware/auth-8 1000000 1866 ns/op 719 B/op 14 allocs/op
// BenchmarkClientMiddleware/default-8 300000 5604 ns/op 2477 B/op 41 allocs/op
func BenchmarkClientMiddleware(b *testing.B) {
tracer, closer, err := tracing.InitGlobalTracer(&jconfig.Configuration{}, "Test", ulog.NopLogger, metrics.NopCachedStatsReporter)
tracer, closer, err := tracing.InitGlobalTracer(&jconfig.Configuration{}, "Test", zap.NewNop(), metrics.NopCachedStatsReporter)
if err != nil {
b.Error(err)
}
Expand Down
8 changes: 4 additions & 4 deletions modules/uhttp/client/client_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ import (
"go.uber.org/fx/config"
"go.uber.org/fx/metrics"
"go.uber.org/fx/tracing"
"go.uber.org/fx/ulog"

"github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber-go/tally"
jconfig "github.com/uber/jaeger-client-go/config"
"go.uber.org/zap"
)

var (
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestExecutionChainOutboundMiddlewareError(t *testing.T) {
}

func withOpentracingSetup(t *testing.T, registerFunc auth.RegisterFunc, fn func(tracer opentracing.Tracer)) {
tracer, closer, err := tracing.InitGlobalTracer(&jconfig.Configuration{}, "Test", ulog.NopLogger, metrics.NopCachedStatsReporter)
tracer, closer, err := tracing.InitGlobalTracer(&jconfig.Configuration{}, "Test", zap.NewNop(), metrics.NopCachedStatsReporter)
defer closer.Close()
assert.NotNil(t, closer)
require.NoError(t, err)
Expand Down Expand Up @@ -158,8 +158,8 @@ func (f fakeAuthInfo) Config() config.Provider {
return config.NewYAMLProviderFromBytes(f.yaml)
}

func (f fakeAuthInfo) Logger() ulog.Log {
return ulog.NopLogger
func (f fakeAuthInfo) Logger() *zap.Logger {
return zap.NewNop()
}

func (f fakeAuthInfo) Metrics() tally.Scope {
Expand Down
Loading