Skip to content

Commit

Permalink
Merge b69958d into 0626e44
Browse files Browse the repository at this point in the history
  • Loading branch information
anuptalwalkar committed Feb 15, 2017
2 parents 0626e44 + b69958d commit 68a7610
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 61 deletions.
2 changes: 0 additions & 2 deletions auth/uauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/uber-go/tally"
"go.uber.org/fx/config"
"go.uber.org/fx/ulog"
)

var (
Expand All @@ -48,7 +47,6 @@ var (
// CreateAuthInfo interface provides necessary data
type CreateAuthInfo interface {
Config() config.Provider
Logger() ulog.Log
Metrics() tally.Scope
}

Expand Down
9 changes: 7 additions & 2 deletions examples/keyvalue/server/observer.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@

package main

import "go.uber.org/fx/service"
import (
"context"

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

// Observer receives callbacks during various service lifecycle events
type Observer struct {
Expand All @@ -32,7 +37,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 {
svc.Logger().Info(
ulog.Logger(context.Background()).Info(
"Received service init callback",
"service_name", o.Name(),
"some_number", o.ServiceConfig.SomeNumber,
Expand Down
4 changes: 2 additions & 2 deletions modules/rpc/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (f contextInboundMiddleware) Handle(ctx context.Context, req *transport.Req
Start()
defer stopwatch.Stop()

ctx = ulog.NewLogContext(ctx)
ctx = ulog.NewLogContext(ctx, ulog.Logger(ctx))
return handler.Handle(ctx, req, resw)
}

Expand All @@ -54,7 +54,7 @@ type contextOnewayInboundMiddleware struct {
}

func (f contextOnewayInboundMiddleware) HandleOneway(ctx context.Context, req *transport.Request, handler transport.OnewayHandler) error {
ctx = ulog.NewLogContext(ctx)
ctx = ulog.NewLogContext(ctx, ulog.Logger(ctx))
return handler.HandleOneway(ctx, req)
}

Expand Down
3 changes: 2 additions & 1 deletion modules/rpc/yarpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package rpc

import (
"context"
"errors"
"fmt"
"sync"
Expand Down Expand Up @@ -226,7 +227,7 @@ func newYARPCModule(

stats.SetupRPCMetrics(mi.Host.Metrics())

module.log = module.Host().Logger().With("moduleName", name)
module.log = ulog.Logger(context.Background()).With("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
19 changes: 8 additions & 11 deletions modules/uhttp/filterchain_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ package uhttp
import (
"net/http"

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

type filterChain struct {
Expand All @@ -43,29 +44,25 @@ func (fc filterChain) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

type filterChainBuilder struct {
service.Host

finalHandler http.Handler
filters []Filter
}

func defaultFilterChainBuilder(host service.Host) filterChainBuilder {
fcb := newFilterChainBuilder(host)
func defaultFilterChainBuilder(log ulog.Log, authClient auth.Client) filterChainBuilder {
fcb := newFilterChainBuilder()
return fcb.AddFilters(
contextFilter{host},
contextFilter{log},
panicFilter{},
metricsFilter{},
tracingServerFilter{},
authorizationFilter{
authClient: host.AuthClient(),
authClient: authClient,
})
}

// NewFilterChainBuilder creates an empty filterChainBuilder for setup
func newFilterChainBuilder(host service.Host) filterChainBuilder {
return filterChainBuilder{
Host: host,
}
func newFilterChainBuilder() filterChainBuilder {
return filterChainBuilder{}
}

func (f filterChainBuilder) AddFilters(filters ...Filter) filterChainBuilder {
Expand Down
5 changes: 2 additions & 3 deletions modules/uhttp/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"go.uber.org/fx/auth"
"go.uber.org/fx/modules/uhttp/internal/stats"
"go.uber.org/fx/service"
"go.uber.org/fx/ulog"

"github.com/opentracing/opentracing-go"
Expand All @@ -51,11 +50,11 @@ func (f FilterFunc) Apply(w http.ResponseWriter, r *http.Request, next http.Hand
}

type contextFilter struct {
host service.Host
log ulog.Log
}

func (f contextFilter) Apply(w http.ResponseWriter, r *http.Request, next http.Handler) {
ctx := ulog.NewLogContext(r.Context())
ctx := ulog.NewLogContext(r.Context(), f.log)
next.ServeHTTP(w, r.WithContext(ctx))
}

Expand Down
15 changes: 6 additions & 9 deletions modules/uhttp/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"strings"
"testing"

"go.uber.org/fx/auth"
"go.uber.org/fx/metrics"
"go.uber.org/fx/modules/uhttp/internal/stats"
"go.uber.org/fx/service"
Expand All @@ -43,8 +42,7 @@ import (
)

func TestFilterChain(t *testing.T) {
host := service.NopHost()
chain := newFilterChainBuilder(host).AddFilters([]Filter{}...).Build(getNopHandler())
chain := newFilterChainBuilder().AddFilters([]Filter{}...).Build(getNopHandler())
response := testServeHTTP(chain)
assert.True(t, strings.Contains(response.Body.String(), "filters ok"))
}
Expand All @@ -65,9 +63,8 @@ func TestTracingFilterWithLogs(t *testing.T) {
opentracing.InitGlobalTracer(tracer)
defer opentracing.InitGlobalTracer(opentracing.NoopTracer{})

host := service.NopHostConfigured(auth.NopClient, loggerWithZap, tracer)
ulog.SetLogger(host.Logger())
chain := newFilterChainBuilder(host).AddFilters([]Filter{contextFilter{host}, tracingServerFilter{}}...).Build(getNopHandler())
ulog.SetLogger(loggerWithZap)
chain := newFilterChainBuilder().AddFilters([]Filter{contextFilter{loggerWithZap}, tracingServerFilter{}}...).Build(getNopHandler())
response := testServeHTTP(chain)
assert.Contains(t, response.Body.String(), "filters ok")
assert.True(t, len(buf.Lines()) > 0)
Expand All @@ -88,7 +85,7 @@ func TestTracingFilterWithLogs(t *testing.T) {

func TestFilterChainFilters(t *testing.T) {
host := service.NopHost()
chain := newFilterChainBuilder(host).AddFilters(
chain := newFilterChainBuilder().AddFilters(
tracingServerFilter{},
authorizationFilter{
authClient: host.AuthClient(),
Expand All @@ -101,7 +98,7 @@ func TestFilterChainFilters(t *testing.T) {
func TestFilterChainFilters_AuthFailure(t *testing.T) {
host := service.NopHostAuthFailure()
stats.SetupHTTPMetrics(host.Metrics())
chain := newFilterChainBuilder(host).AddFilters(
chain := newFilterChainBuilder().AddFilters(
tracingServerFilter{},
authorizationFilter{
authClient: host.AuthClient(),
Expand All @@ -115,7 +112,7 @@ func TestPanicFilter(t *testing.T) {
host := service.NopHost()
testScope := host.Metrics()

chain := newFilterChainBuilder(host).AddFilters(
chain := newFilterChainBuilder().AddFilters(
panicFilter{},
).Build(getPanicHandler())
response := testServeHTTP(chain)
Expand Down
2 changes: 1 addition & 1 deletion modules/uhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type handlerWithHost struct {

// ServeHTTP calls Handler.ServeHTTP( w, r) and injects a new service context for use.
func (h *handlerWithHost) ServeHTTP(w http.ResponseWriter, r *http.Request) {
ctx := ulog.NewLogContext(r.Context())
ctx := ulog.NewLogContext(r.Context(), ulog.Logger(r.Context()))
stopwatch := stats.HTTPMethodTimer.Timer(r.Method).Start()
defer stopwatch.Stop()

Expand Down
9 changes: 6 additions & 3 deletions modules/uhttp/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package uhttp

import (
"context"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -115,22 +116,24 @@ func newModule(

handlers := addHealth(getHandlers(mi.Host))

log := ulog.Logger(context.Background()).With("moduleName", mi.Name)

// TODO (madhu): Add other middleware - logging, metrics.
module := &Module{
ModuleBase: *modules.NewModuleBase(mi.Name, mi.Host, []string{}),
handlers: handlers,
fcb: defaultFilterChainBuilder(mi.Host),
fcb: defaultFilterChainBuilder(log, mi.Host.AuthClient()),
}

module.fcb = module.fcb.AddFilters(filters...)

err := module.Host().Config().Get(getConfigKey(mi.Name)).PopulateStruct(cfg)
if err != nil {
module.Host().Logger().Error("Error loading http module configuration", "error", err)
log.Error("Error loading http module configuration", "error", err)
}
module.config = *cfg

module.log = module.Host().Logger().With("moduleName", mi.Name)
module.log = log

for _, option := range options {
if err := option(&mi); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions service/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ type Host interface {
Observer() Observer
Config() config.Provider
Resources() map[string]interface{}
Logger() ulog.Log
Tracer() opentracing.Tracer
}

Expand Down Expand Up @@ -102,10 +101,6 @@ type loggingCore struct {
logConfig ulog.Configuration
}

func (lc *loggingCore) Logger() ulog.Log {
return lc.log
}

type serviceCore struct {
loggingCore
metricsCore
Expand Down
30 changes: 15 additions & 15 deletions service/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (s *host) IsRunning() bool {
func (s *host) OnCriticalError(err error) {
shutdown := true
if s.observer == nil {
s.Logger().Warn(
s.log.Warn(
"No observer set to handle lifecycle events. Shutting down.",
"event", "OnCriticalError",
)
Expand All @@ -106,7 +106,7 @@ func (s *host) OnCriticalError(err error) {
if shutdown {
if ok, err := s.shutdown(err, "", nil); !ok || err != nil {
// TODO(ai) verify we flush logs
s.Logger().Info("Problem shutting down module", "success", ok, "error", err)
s.log.Info("Problem shutting down module", "success", ok, "error", err)
}
}
}
Expand Down Expand Up @@ -146,7 +146,7 @@ func (s *host) shutdown(err error, reason string, exitCode *int) (bool, error) {
errs := s.stopModules()
if len(errs) > 0 {
for k, v := range errs {
s.Logger().Error("Failure to shut down module", "name", k.Name(), "error", v.Error())
s.log.Error("Failure to shut down module", "name", k.Name(), "error", v.Error())
}
}

Expand All @@ -158,15 +158,15 @@ func (s *host) shutdown(err error, reason string, exitCode *int) (bool, error) {
// Stop the metrics reporting
if s.metricsCloser != nil {
if err = s.metricsCloser.Close(); err != nil {
s.Logger().Error("Failure to close metrics", "error", err)
s.log.Error("Failure to close metrics", "error", err)
}
}

// Flush tracing buffers
if s.tracerCloser != nil {
s.Logger().Debug("Closing tracer")
s.log.Debug("Closing tracer")
if err = s.tracerCloser.Close(); err != nil {
s.Logger().Error("Failure to close tracer", "error", err)
s.log.Error("Failure to close tracer", "error", err)
}
}

Expand Down Expand Up @@ -197,7 +197,7 @@ func (s *host) AddModules(modules ...ModuleCreateFunc) error {
}

if !s.supportsRole(mi.Roles...) {
s.Logger().Info(
s.log.Info(
"module will not be added due to selected roles",
"roles", mi.Roles,
)
Expand Down Expand Up @@ -278,9 +278,9 @@ func (s *host) start() Control {

s.shutdownMu.Unlock()
if _, err := s.shutdown(e, "", nil); err != nil {
s.Logger().Error("Unable to shut down modules", "initialError", e, "shutdownError", err)
s.log.Error("Unable to shut down modules", "initialError", e, "shutdownError", err)
}
s.Logger().Error("Error starting the module", "error", e)
s.log.Error("Error starting the module", "error", e)
// return first service error
if serviceErr == nil {
serviceErr = e
Expand Down Expand Up @@ -309,9 +309,9 @@ func (s *host) registerSignalHandlers() {
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
go func() {
sig := <-ch
s.Logger().Warn("Received shutdown signal", "signal", sig.String())
s.log.Warn("Received shutdown signal", "signal", sig.String())
if err := s.Stop("Received syscall", 0); err != nil {
s.Logger().Error("Error shutting down", "error", err.Error())
s.log.Error("Error shutting down", "error", err.Error())
}
}()
}
Expand All @@ -338,13 +338,13 @@ func (s *host) startModules() map[Module]error {

select {
case <-readyCh:
s.Logger().Info("Module started up cleanly", "module", m.Name())
s.log.Info("Module started up cleanly", "module", m.Name())
case <-time.After(defaultStartupWait):
results[m] = fmt.Errorf("module didn't start after %v", defaultStartupWait)
}

if startError := <-startResult; startError != nil {
s.Logger().Error("Error received while starting module", "module", m.Name(), "error", startError)
s.log.Error("Error received while starting module", "module", m.Name(), "error", startError)
results[m] = startError
}
}
Expand Down Expand Up @@ -383,7 +383,7 @@ type ExitCallback func(shutdown Exit) int

func (s *host) WaitForShutdown(exitCallback ExitCallback) {
shutdown := <-s.closeChan
s.Logger().Info("Shutting down", "reason", shutdown.Reason)
s.log.Info("Shutting down", "reason", shutdown.Reason)

exit := 0
if exitCallback != nil {
Expand All @@ -400,7 +400,7 @@ func (s *host) transitionState(to State) {

// TODO(ai) this isn't used yet
if to < s.state {
s.Logger().Fatal("Can't down from state", "from", s.state, "to", to, "service", s.Name())
s.log.Fatal("Can't down from state", "from", s.state, "to", to, "service", s.Name())
}

for s.state < to {
Expand Down
4 changes: 2 additions & 2 deletions service/service_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (svc *serviceCore) setupLogging() {
svc.log = logBuilder.WithConfiguration(svc.logConfig).Build()
ulog.SetLogger(svc.log)
} else {
svc.log.Debug("Using custom log provider due to service.WithLogger option")
ulog.Logger(context.Background()).Debug("Using custom log provider due to service.WithLogger option")
}
}

Expand All @@ -55,7 +55,7 @@ func (svc *serviceCore) setupStandardConfig() error {
}

if errs := validator.Validate(svc.standardConfig); errs != nil {
svc.Logger().Error("Invalid service configuration", "error", errs)
svc.log.Error("Invalid service configuration", "error", errs)
return errors.Wrap(errs, "service configuration failed validation")
}
return nil
Expand Down

0 comments on commit 68a7610

Please sign in to comment.