From bb1c8f58bbbbd630a9608d3db43c9f406457196f Mon Sep 17 00:00:00 2001 From: Andre Hahn Pereira Date: Thu, 27 Dec 2018 11:37:54 -0200 Subject: [PATCH 01/21] Update README.md --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 912b9d22..649cc008 100644 --- a/README.md +++ b/README.md @@ -71,3 +71,7 @@ Getting coverage data can be achieved with `make coverage`, while reading the ac Khan goes through some static analysis tools for go. To run them just use `make static`. Right now, gocyclo can't process the vendor folder, so we just ignore the exit code for it, while maintaining the output for anything not in the vendor folder. + +### Security + +If you have found a security vulnerability, please email security@tfgco.com From 6e7f21a31279cf178b575ad9e11d506a20e4527d Mon Sep 17 00:00:00 2001 From: Andre Hahn Pereira Date: Thu, 27 Dec 2018 11:38:13 -0200 Subject: [PATCH 02/21] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 649cc008..d1598f90 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,6 @@ Khan goes through some static analysis tools for go. To run them just use `make Right now, gocyclo can't process the vendor folder, so we just ignore the exit code for it, while maintaining the output for anything not in the vendor folder. -### Security +## Security If you have found a security vulnerability, please email security@tfgco.com From e74b5441d482c1ad794496e53485d82720205189 Mon Sep 17 00:00:00 2001 From: Camila de Andrade Scatolini Date: Fri, 25 Jan 2019 19:06:17 -0200 Subject: [PATCH 03/21] Change request logging to debug level. --- api/middleware.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/middleware.go b/api/middleware.go index a853a953..f1fd7b96 100644 --- a/api/middleware.go +++ b/api/middleware.go @@ -241,18 +241,18 @@ func (l *LoggerMiddleware) Serve(next echo.HandlerFunc) echo.HandlerFunc { //request failed if status > 399 && status < 500 { - log.W(reqLog, "Request failed.") + log.D(reqLog, "Request failed.") return err } //request is ok, but server failed if status > 499 { - log.E(reqLog, "Response failed.") + log.D(reqLog, "Response failed.") return err } //Everything went ok - if cm := reqLog.Check(zap.InfoLevel, "Request successful."); cm.OK() { + if cm := reqLog.Check(zap.DebugLevel, "Request successful."); cm.OK() { cm.Write() } From 0e6d0b62280616b0f0a4783767e0e0379e713d84 Mon Sep 17 00:00:00 2001 From: Camila de Andrade Scatolini Date: Fri, 25 Jan 2019 19:06:51 -0200 Subject: [PATCH 04/21] Release v4.1.1 --- util/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/version.go b/util/version.go index c6ee58d6..d3327754 100644 --- a/util/version.go +++ b/util/version.go @@ -8,4 +8,4 @@ package util // VERSION identifies Khan's current version -var VERSION = "4.1.0" +var VERSION = "4.1.1" From 435e68a49ec90cd156481eddd803d2cd5a816f70 Mon Sep 17 00:00:00 2001 From: Andre Hahn Date: Thu, 7 Feb 2019 15:19:37 -0200 Subject: [PATCH 05/21] Updated mocks --- Gopkg.lock | 6 +- lib/mocks/khan.go | 150 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 5cfefffe..b490d854 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -125,12 +125,12 @@ revision = "0b58b37b664c21f3010e836f1b931e1d0b0b0685" [[projects]] - digest = "1:a62049a8fa554d688c8db4845c65ddcd5986b75f3a851e4fb563c6893d292e38" + digest = "1:b60efdeb75d3c0ceed88783ac2495256aba3491a537d0f31401202579fd62a94" name = "github.com/golang/mock" packages = ["gomock"] pruneopts = "UT" - revision = "13f360950a79f5864a972c786a10a50e44b69541" - version = "v1.0.0" + revision = "51421b967af1f557f93a59e0057aaf15ca02e29c" + version = "v1.2.0" [[projects]] digest = "1:ffc060c551980d37ee9e428ef528ee2813137249ccebb0bfc412ef83071cac91" diff --git a/lib/mocks/khan.go b/lib/mocks/khan.go index fbccd5f3..55f03d05 100644 --- a/lib/mocks/khan.go +++ b/lib/mocks/khan.go @@ -34,8 +34,54 @@ func (m *MockKhanInterface) EXPECT() *MockKhanInterfaceMockRecorder { return m.recorder } +// ApplyForMembership mocks base method +func (m *MockKhanInterface) ApplyForMembership(arg0 context.Context, arg1 *lib.ApplicationPayload) (*lib.ClanApplyResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ApplyForMembership", arg0, arg1) + ret0, _ := ret[0].(*lib.ClanApplyResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ApplyForMembership indicates an expected call of ApplyForMembership +func (mr *MockKhanInterfaceMockRecorder) ApplyForMembership(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApplyForMembership", reflect.TypeOf((*MockKhanInterface)(nil).ApplyForMembership), arg0, arg1) +} + +// ApproveDenyMembershipApplication mocks base method +func (m *MockKhanInterface) ApproveDenyMembershipApplication(arg0 context.Context, arg1 *lib.ApplicationApprovalPayload) (*lib.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ApproveDenyMembershipApplication", arg0, arg1) + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ApproveDenyMembershipApplication indicates an expected call of ApproveDenyMembershipApplication +func (mr *MockKhanInterfaceMockRecorder) ApproveDenyMembershipApplication(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApproveDenyMembershipApplication", reflect.TypeOf((*MockKhanInterface)(nil).ApproveDenyMembershipApplication), arg0, arg1) +} + +// ApproveDenyMembershipInvitation mocks base method +func (m *MockKhanInterface) ApproveDenyMembershipInvitation(arg0 context.Context, arg1 *lib.InvitationApprovalPayload) (*lib.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ApproveDenyMembershipInvitation", arg0, arg1) + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ApproveDenyMembershipInvitation indicates an expected call of ApproveDenyMembershipInvitation +func (mr *MockKhanInterfaceMockRecorder) ApproveDenyMembershipInvitation(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApproveDenyMembershipInvitation", reflect.TypeOf((*MockKhanInterface)(nil).ApproveDenyMembershipInvitation), arg0, arg1) +} + // CreateClan mocks base method func (m *MockKhanInterface) CreateClan(arg0 context.Context, arg1 *lib.ClanPayload) (string, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateClan", arg0, arg1) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) @@ -44,11 +90,13 @@ func (m *MockKhanInterface) CreateClan(arg0 context.Context, arg1 *lib.ClanPaylo // CreateClan indicates an expected call of CreateClan func (mr *MockKhanInterfaceMockRecorder) CreateClan(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateClan", reflect.TypeOf((*MockKhanInterface)(nil).CreateClan), arg0, arg1) } // CreatePlayer mocks base method func (m *MockKhanInterface) CreatePlayer(arg0 context.Context, arg1, arg2 string, arg3 interface{}) (string, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreatePlayer", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(string) ret1, _ := ret[1].(error) @@ -57,11 +105,73 @@ func (m *MockKhanInterface) CreatePlayer(arg0 context.Context, arg1, arg2 string // CreatePlayer indicates an expected call of CreatePlayer func (mr *MockKhanInterfaceMockRecorder) CreatePlayer(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreatePlayer", reflect.TypeOf((*MockKhanInterface)(nil).CreatePlayer), arg0, arg1, arg2, arg3) } +// DeleteMembership mocks base method +func (m *MockKhanInterface) DeleteMembership(arg0 context.Context, arg1 *lib.DeleteMembershipPayload) (*lib.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteMembership", arg0, arg1) + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DeleteMembership indicates an expected call of DeleteMembership +func (mr *MockKhanInterfaceMockRecorder) DeleteMembership(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMembership", reflect.TypeOf((*MockKhanInterface)(nil).DeleteMembership), arg0, arg1) +} + +// InviteForMembership mocks base method +func (m *MockKhanInterface) InviteForMembership(arg0 context.Context, arg1 *lib.InvitationPayload) (*lib.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InviteForMembership", arg0, arg1) + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InviteForMembership indicates an expected call of InviteForMembership +func (mr *MockKhanInterfaceMockRecorder) InviteForMembership(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InviteForMembership", reflect.TypeOf((*MockKhanInterface)(nil).InviteForMembership), arg0, arg1) +} + +// LeaveClan mocks base method +func (m *MockKhanInterface) LeaveClan(arg0 context.Context, arg1 string) (*lib.LeaveClanResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LeaveClan", arg0, arg1) + ret0, _ := ret[0].(*lib.LeaveClanResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// LeaveClan indicates an expected call of LeaveClan +func (mr *MockKhanInterfaceMockRecorder) LeaveClan(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LeaveClan", reflect.TypeOf((*MockKhanInterface)(nil).LeaveClan), arg0, arg1) +} + +// PromoteDemote mocks base method +func (m *MockKhanInterface) PromoteDemote(arg0 context.Context, arg1 *lib.PromoteDemotePayload) (*lib.Result, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PromoteDemote", arg0, arg1) + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// PromoteDemote indicates an expected call of PromoteDemote +func (mr *MockKhanInterfaceMockRecorder) PromoteDemote(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PromoteDemote", reflect.TypeOf((*MockKhanInterface)(nil).PromoteDemote), arg0, arg1) +} + // RetrieveClan mocks base method func (m *MockKhanInterface) RetrieveClan(arg0 context.Context, arg1 string) (*lib.Clan, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RetrieveClan", arg0, arg1) ret0, _ := ret[0].(*lib.Clan) ret1, _ := ret[1].(error) @@ -70,11 +180,13 @@ func (m *MockKhanInterface) RetrieveClan(arg0 context.Context, arg1 string) (*li // RetrieveClan indicates an expected call of RetrieveClan func (mr *MockKhanInterfaceMockRecorder) RetrieveClan(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrieveClan", reflect.TypeOf((*MockKhanInterface)(nil).RetrieveClan), arg0, arg1) } // RetrieveClanSummary mocks base method func (m *MockKhanInterface) RetrieveClanSummary(arg0 context.Context, arg1 string) (*lib.ClanSummary, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RetrieveClanSummary", arg0, arg1) ret0, _ := ret[0].(*lib.ClanSummary) ret1, _ := ret[1].(error) @@ -83,11 +195,13 @@ func (m *MockKhanInterface) RetrieveClanSummary(arg0 context.Context, arg1 strin // RetrieveClanSummary indicates an expected call of RetrieveClanSummary func (mr *MockKhanInterfaceMockRecorder) RetrieveClanSummary(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrieveClanSummary", reflect.TypeOf((*MockKhanInterface)(nil).RetrieveClanSummary), arg0, arg1) } // RetrieveClansSummary mocks base method func (m *MockKhanInterface) RetrieveClansSummary(arg0 context.Context, arg1 []string) ([]*lib.ClanSummary, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RetrieveClansSummary", arg0, arg1) ret0, _ := ret[0].([]*lib.ClanSummary) ret1, _ := ret[1].(error) @@ -96,11 +210,13 @@ func (m *MockKhanInterface) RetrieveClansSummary(arg0 context.Context, arg1 []st // RetrieveClansSummary indicates an expected call of RetrieveClansSummary func (mr *MockKhanInterfaceMockRecorder) RetrieveClansSummary(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrieveClansSummary", reflect.TypeOf((*MockKhanInterface)(nil).RetrieveClansSummary), arg0, arg1) } // RetrievePlayer mocks base method func (m *MockKhanInterface) RetrievePlayer(arg0 context.Context, arg1 string) (*lib.Player, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "RetrievePlayer", arg0, arg1) ret0, _ := ret[0].(*lib.Player) ret1, _ := ret[1].(error) @@ -109,29 +225,51 @@ func (m *MockKhanInterface) RetrievePlayer(arg0 context.Context, arg1 string) (* // RetrievePlayer indicates an expected call of RetrievePlayer func (mr *MockKhanInterfaceMockRecorder) RetrievePlayer(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RetrievePlayer", reflect.TypeOf((*MockKhanInterface)(nil).RetrievePlayer), arg0, arg1) } +// TransferOwnership mocks base method +func (m *MockKhanInterface) TransferOwnership(arg0 context.Context, arg1, arg2 string) (*lib.TransferOwnershipResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TransferOwnership", arg0, arg1, arg2) + ret0, _ := ret[0].(*lib.TransferOwnershipResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TransferOwnership indicates an expected call of TransferOwnership +func (mr *MockKhanInterfaceMockRecorder) TransferOwnership(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TransferOwnership", reflect.TypeOf((*MockKhanInterface)(nil).TransferOwnership), arg0, arg1, arg2) +} + // UpdateClan mocks base method -func (m *MockKhanInterface) UpdateClan(arg0 context.Context, arg1 *lib.ClanPayload) error { +func (m *MockKhanInterface) UpdateClan(arg0 context.Context, arg1 *lib.ClanPayload) (*lib.Result, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdateClan", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 } // UpdateClan indicates an expected call of UpdateClan func (mr *MockKhanInterfaceMockRecorder) UpdateClan(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateClan", reflect.TypeOf((*MockKhanInterface)(nil).UpdateClan), arg0, arg1) } // UpdatePlayer mocks base method -func (m *MockKhanInterface) UpdatePlayer(arg0 context.Context, arg1, arg2 string, arg3 interface{}) error { +func (m *MockKhanInterface) UpdatePlayer(arg0 context.Context, arg1, arg2 string, arg3 interface{}) (*lib.Result, error) { + m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdatePlayer", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(*lib.Result) + ret1, _ := ret[1].(error) + return ret0, ret1 } // UpdatePlayer indicates an expected call of UpdatePlayer func (mr *MockKhanInterfaceMockRecorder) UpdatePlayer(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePlayer", reflect.TypeOf((*MockKhanInterface)(nil).UpdatePlayer), arg0, arg1, arg2, arg3) } From 26b1101bf71e5121bb6d983fa0bae00358e9b679 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Tue, 26 Feb 2019 14:50:01 -0300 Subject: [PATCH 06/21] Fix panic when error while approving/denying membership. --- api/membership.go | 82 +++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 42 deletions(-) diff --git a/api/membership.go b/api/membership.go index 5fb0406b..adfea108 100644 --- a/api/membership.go +++ b/api/membership.go @@ -287,11 +287,6 @@ func InviteForMembershipHandler(app *App) func(c echo.Context) error { // ApproveOrDenyMembershipApplicationHandler is the handler responsible for approving or denying a membership invitation func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) error { return func(c echo.Context) error { - var game *models.Game - var membership *models.Membership - var requestor *models.Player - var err error - var tx interfaces.Transaction c.Set("route", "ApproverOrDenyApplication") start := time.Now() @@ -308,7 +303,7 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er ) var payload BasePayloadWithRequestorAndPlayerPublicIDs - err = WithSegment("payload", c, func() error { + err := WithSegment("payload", c, func() error { if err := LoadJSONPayload(&payload, c, l); err != nil { return err } @@ -323,11 +318,13 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er zap.String("requestorPublicID", payload.RequestorPublicID), ) + var game *models.Game err = WithSegment("game-retrieve", c, func() error { - game, err = app.GetGame(c.StdContext(), gameID) - if err != nil { + var gErr error + game, gErr = app.GetGame(c.StdContext(), gameID) + if gErr != nil { log.W(l, "Could not find game.") - return err + return gErr } return nil }) @@ -335,28 +332,28 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er return FailWith(404, err.Error(), c) } - rb := func(err error) error { - txErr := app.Rollback(tx, "Approving/Denying membership application failed", c, l, err) - if txErr != nil { - return txErr - } - - return nil + var tx interfaces.Transaction + rb := func(rbErr error) error { + return app.Rollback(tx, "Approving/Denying membership application failed", c, l, rbErr) } + var membership *models.Membership + var requestor *models.Player err = WithSegment("membership-approve-deny", c, func() error { - err = WithSegment("tx-begin", c, func() error { - tx, err = app.BeginTrans(c.StdContext(), l) - return err + txErr := WithSegment("tx-begin", c, func() error { + var txBErr error + tx, txBErr = app.BeginTrans(c.StdContext(), l) + return txBErr }) - if err != nil { - return err + if txErr != nil { + return txErr } log.D(l, "DB Tx begun successful.") - err = WithSegment("membership-approve-deny-query", c, func() error { + qErr := WithSegment("membership-approve-deny-query", c, func() error { log.D(l, "Approving/Denying membership application.") - membership, err = models.ApproveOrDenyMembershipApplication( + var mErr error + membership, mErr = models.ApproveOrDenyMembershipApplication( tx, game, gameID, @@ -366,39 +363,40 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er action, ) - if err != nil { - txErr := rb(err) + if mErr != nil { + txErr := rb(mErr) if txErr == nil { log.E(l, "Approving/Denying membership application failed.", func(cm log.CM) { - cm.Write(zap.Error(err)) + cm.Write(zap.Error(mErr)) }) } - return err + return mErr } return nil }) - if err != nil { - return FailWithError(err, c) + if qErr != nil { + return FailWithError(qErr, c) } - err = WithSegment("player-retrieve", c, func() error { + pErr := WithSegment("player-retrieve", c, func() error { log.D(l, "Retrieving requestor details.") - requestor, err = models.GetPlayerByPublicID(tx, gameID, payload.RequestorPublicID) - if err != nil { + var gErr error + requestor, gErr = models.GetPlayerByPublicID(tx, gameID, payload.RequestorPublicID) + if gErr != nil { msg := "Requestor details retrieval failed." - txErr := rb(err) + txErr := rb(gErr) if txErr == nil { log.E(l, msg, func(cm log.CM) { - cm.Write(zap.Error(err)) + cm.Write(zap.Error(gErr)) }) } - return err + return gErr } log.D(l, "Requestor details retrieved successfully.") return nil }) - if err != nil { - return err + if pErr != nil { + return pErr } return nil }) @@ -411,20 +409,20 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er if action == "deny" { hookType = models.MembershipDeniedHook } - err = dispatchApproveDenyMembershipHookByID( + aErr := dispatchApproveDenyMembershipHookByID( app, tx, hookType, membership.GameID, membership.ClanID, membership.PlayerID, requestor.ID, membership.RequestorID, membership.Message, membership.Level, ) - if err != nil { + if aErr != nil { msg := "Membership approved/denied application dispatch hook failed." - txErr := rb(err) + txErr := rb(aErr) if txErr == nil { log.E(l, msg, func(cm log.CM) { - cm.Write(zap.Error(err)) + cm.Write(zap.Error(aErr)) }) } - return err + return aErr } return nil }) From df0a9384cc957bf5a95a996694f12909f68982b4 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Tue, 26 Feb 2019 15:04:23 -0300 Subject: [PATCH 07/21] Release v4.1.3 --- util/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/version.go b/util/version.go index d3327754..6822c263 100644 --- a/util/version.go +++ b/util/version.go @@ -8,4 +8,4 @@ package util // VERSION identifies Khan's current version -var VERSION = "4.1.1" +var VERSION = "4.1.3" From 79ee84cda559b7f81276d6980a8e32fc3e181760 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Tue, 26 Feb 2019 19:22:24 -0300 Subject: [PATCH 08/21] Fix panic: FailWithError swallows the err and return nil if err was properly serialized. --- api/app.go | 7 ++----- api/membership.go | 11 +++-------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/api/app.go b/api/app.go index 6460c71f..61ee71fe 100644 --- a/api/app.go +++ b/api/app.go @@ -633,7 +633,7 @@ func (app *App) BeginTrans(ctx context.Context, l zap.Logger) (gorp.Transaction, //Rollback transaction func (app *App) Rollback(tx gorp.Transaction, msg string, c echo.Context, l zap.Logger, err error) error { - sErr := WithSegment("tx-rollback", c, func() error { + return WithSegment("tx-rollback", c, func() error { txErr := tx.Rollback() if txErr != nil { log.E(l, fmt.Sprintf("%s and failed to rollback transaction.", msg), func(cm log.CM) { @@ -643,12 +643,11 @@ func (app *App) Rollback(tx gorp.Transaction, msg string, c echo.Context, l zap. } return nil }) - return sErr } //Commit transaction func (app *App) Commit(tx gorp.Transaction, msg string, c echo.Context, l zap.Logger) error { - err := WithSegment("tx-commit", c, func() error { + return WithSegment("tx-commit", c, func() error { txErr := tx.Commit() if txErr != nil { log.E(l, fmt.Sprintf("%s failed to commit transaction.", msg), func(cm log.CM) { @@ -656,10 +655,8 @@ func (app *App) Commit(tx gorp.Transaction, msg string, c echo.Context, l zap.Lo }) return txErr } - return nil }) - return err } // GetCtxDB returns the proper database connection depending on the request context diff --git a/api/membership.go b/api/membership.go index adfea108..0ae137ea 100644 --- a/api/membership.go +++ b/api/membership.go @@ -287,7 +287,6 @@ func InviteForMembershipHandler(app *App) func(c echo.Context) error { // ApproveOrDenyMembershipApplicationHandler is the handler responsible for approving or denying a membership invitation func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) error { return func(c echo.Context) error { - c.Set("route", "ApproverOrDenyApplication") start := time.Now() action := c.Param("action") @@ -375,10 +374,10 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er return nil }) if qErr != nil { - return FailWithError(qErr, c) + return qErr } - pErr := WithSegment("player-retrieve", c, func() error { + return WithSegment("player-retrieve", c, func() error { log.D(l, "Retrieving requestor details.") var gErr error requestor, gErr = models.GetPlayerByPublicID(tx, gameID, payload.RequestorPublicID) @@ -395,13 +394,9 @@ func ApproveOrDenyMembershipApplicationHandler(app *App) func(c echo.Context) er log.D(l, "Requestor details retrieved successfully.") return nil }) - if pErr != nil { - return pErr - } - return nil }) if err != nil { - return FailWith(http.StatusInternalServerError, err.Error(), c) + return FailWithError(err, c) } err = WithSegment("hook-dispatch", c, func() error { From be762679c4bce1857c8ddcf25fcf32e89d262e1b Mon Sep 17 00:00:00 2001 From: cscatolini Date: Tue, 26 Feb 2019 20:20:36 -0300 Subject: [PATCH 09/21] Bump extensions version to include game id in metrics. --- Gopkg.lock | 104 ++++++++++++++++++++--------------------------------- Gopkg.toml | 7 ++-- 2 files changed, 43 insertions(+), 68 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index b490d854..f57f2f65 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -24,14 +24,6 @@ pruneopts = "UT" revision = "01563c9f5c2dd648444bde7b3705a37698581364" -[[projects]] - digest = "1:9752dad5e89cd779096bf2477a4ded16bea7ac62de453c8d6b4bf841d51a8512" - name = "github.com/apache/thrift" - packages = ["lib/go/thrift"] - pruneopts = "UT" - revision = "b2a4d4ae21c789b689dd162deb819665567f481c" - version = "0.10.0" - [[projects]] digest = "1:320e7ead93de9fd2b0e59b50fd92a4d50c1f8ab455d96bc2eb083267453a9709" name = "github.com/asaskevich/govalidator" @@ -55,14 +47,6 @@ pruneopts = "UT" revision = "8a28e9752dbc744626695b749a3ab474b8b372fa" -[[projects]] - branch = "master" - digest = "1:4c4c33075b704791d6a7f09dfb55c66769e8a1dc6adf87026292d274fe8ad113" - name = "github.com/codahale/hdrhistogram" - packages = ["."] - pruneopts = "UT" - revision = "3a0bb77429bd3a61596f5e8a3172445844342120" - [[projects]] digest = "1:02630c70b55ecda2dcb85805170923d8389c632487568ccffadc5482e8a191ed" name = "github.com/dgrijalva/jwt-go" @@ -94,6 +78,20 @@ pruneopts = "UT" revision = "c9d3cc542ad199f62c0264286be537f9bce6063c" +[[projects]] + digest = "1:4b08116de0de75c041bb341686f0b139930f26cb84dfdf7641d435548114181d" + name = "github.com/globalsign/mgo" + packages = [ + ".", + "bson", + "internal/json", + "internal/sasl", + "internal/scram", + ] + pruneopts = "UT" + revision = "113d3961e7311526535a1ef7042196563d442761" + version = "r2018.06.15" + [[projects]] digest = "1:670d1f29fa2aa15ea777cc5bcf95881f379bf8a71dbbe145be0774da97fede72" name = "github.com/go-gorp/gorp" @@ -103,7 +101,7 @@ version = "2.1" [[projects]] - digest = "1:e1bb493d8504f7721e8a5da3e272d71dcf1db9a81666a10879f9e611484b74a4" + digest = "1:0a2260bcfa15237979ef496f03fb6af215c247ad998881a4d039992228876ae3" name = "github.com/go-pg/pg" packages = [ ".", @@ -114,8 +112,8 @@ "types", ] pruneopts = "UT" - revision = "8c7fa0eed0d9e9d07fca2cc9e08e3171da0521d5" - version = "v6.13.5" + revision = "ae5d5e7df4b2e598390e10b66b849c6af94f092b" + version = "v6.15.1" [[projects]] digest = "1:4e717b211b133963a5c761d9e93d0b91f004187cc808891e7039186daa28ea4f" @@ -400,11 +398,10 @@ version = "v1.2.0" [[projects]] - digest = "1:450b7623b185031f3a456801155c8320209f75d0d4c4e633c6b1e59d44d6e392" + digest = "1:9d7b1f07e44cada28e6160cb0a808ad2f71f32bd7114793fb0baa302db0e6229" name = "github.com/opentracing/opentracing-go" packages = [ ".", - "ext", "log", ] pruneopts = "UT" @@ -508,10 +505,9 @@ packages = ["."] pruneopts = "UT" revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" - version = "v1.0.0" [[projects]] - digest = "1:4bf1a0c3e8d7ac03a7ee0bed6dea162a93a5eed7f2a40d5a6edb9e03b7aa3ec4" + digest = "1:5deb18e60063a7156c41845e97669b57258064be5145211a4a663c7b8c093da1" name = "github.com/topfreegames/extensions" packages = [ "dogstatsd", @@ -520,21 +516,21 @@ "gorp", "gorp/interfaces", "http", - "jaeger", - "jaeger/echo", - "jaeger/gorp", - "jaeger/http", - "jaeger/mongo", "middleware", "mongo", "mongo/interfaces", "oauth2", "pg/interfaces", + "tracing", + "tracing/echo", + "tracing/gorp", + "tracing/http", + "tracing/mongo", "worker/middleware", ] pruneopts = "UT" - revision = "e732c691014172aa81c280d852530cc03fcc590a" - version = "v6.2.4" + revision = "b915720a3f4b5f0b27627c575f43088bd76dbdbc" + version = "v8.0.5" [[projects]] branch = "master" @@ -559,36 +555,6 @@ pruneopts = "UT" revision = "d11d2851fcabcf03421c4dbdbc3146fc74eb1035" -[[projects]] - digest = "1:8b7a2609692cfade851ff69f371314a8ab77dfb89242e7836f8213af7b16b74c" - name = "github.com/uber/jaeger-client-go" - packages = [ - ".", - "config", - "internal/baggage", - "internal/baggage/remote", - "internal/spanlog", - "log", - "rpcmetrics", - "thrift-gen/agent", - "thrift-gen/baggage", - "thrift-gen/jaeger", - "thrift-gen/sampling", - "thrift-gen/zipkincore", - "utils", - ] - pruneopts = "UT" - revision = "3ac96c6e679cb60a74589b0d0aa7c70a906183f7" - version = "v2.11.2" - -[[projects]] - digest = "1:0da2810678a062e0567c3215911869b0423da0e497c56683ff8e87e7a6952597" - name = "github.com/uber/jaeger-lib" - packages = ["metrics"] - pruneopts = "UT" - revision = "4267858c0679cd4e47cefed8d7f70fd386cfb567" - version = "v1.4.0" - [[projects]] branch = "master" digest = "1:c468422f334a6b46a19448ad59aaffdfc0a36b08fdcc1c749a0b29b6453d7e59" @@ -627,12 +593,13 @@ revision = "df6241f6355c5595d99e9661039f8648c440deba" [[projects]] - digest = "1:3e9bc10b1094bd3a422b18b3adf4fda467ad57b813f69907f96e5b0293c7073d" + digest = "1:ffed49936eb3812b6824211b95112a01ebab612a83c21c3597d9bde4e2f170ed" name = "golang.org/x/crypto" packages = [ "curve25519", "ed25519", "ed25519/internal/edwards25519", + "pbkdf2", "ssh", "ssh/terminal", ] @@ -730,14 +697,11 @@ [[projects]] branch = "v2" - digest = "1:5052389c6c809eeced44ec24ff9958bfea70b9859b7a25061c6fd9086c8f8989" + digest = "1:9e8ac8aa4f4c5557ba0b03a55d407b07b7030ed552b437a9dc7e5ec90f16ab39" name = "gopkg.in/mgo.v2" packages = [ - ".", "bson", "internal/json", - "internal/sasl", - "internal/scram", ] pruneopts = "UT" revision = "3f83fa5005286a7fe593b055f0d7771a7dce4655" @@ -761,6 +725,14 @@ pruneopts = "UT" revision = "e4d366fc3c7938e2958e662b4258c7a89e1f0e3e" +[[projects]] + digest = "1:999d566ad1ae4303c456af5a155f7b42401cca4fd33552567ed9dd997b107a08" + name = "mellium.im/sasl" + packages = ["."] + pruneopts = "UT" + revision = "e58e780e1378aa4df5dc705364ff5ae9a4dd1e04" + version = "v0.2.1" + [solve-meta] analyzer-name = "dep" analyzer-version = 1 @@ -795,10 +767,10 @@ "github.com/topfreegames/extensions/gorp", "github.com/topfreegames/extensions/gorp/interfaces", "github.com/topfreegames/extensions/http", - "github.com/topfreegames/extensions/jaeger", "github.com/topfreegames/extensions/middleware", "github.com/topfreegames/extensions/mongo", "github.com/topfreegames/extensions/mongo/interfaces", + "github.com/topfreegames/extensions/tracing", "github.com/topfreegames/extensions/worker/middleware", "github.com/topfreegames/goose/lib/goose", "github.com/uber-go/zap", diff --git a/Gopkg.toml b/Gopkg.toml index 9c03e216..f6ee860b 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -20,7 +20,7 @@ [[constraint]] name = "github.com/topfreegames/extensions" - version = "6.2.4" + version = "8.0.5" [[constraint]] name = "github.com/go-gorp/gorp" @@ -30,11 +30,14 @@ name = "gopkg.in/olivere/elastic.v5" version = "5.0.39" - [[constraint]] name = "github.com/mailru/easyjson" branch = "master" +[[override]] + name = "github.com/spf13/viper" + revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" + [prune] go-tests = true unused-packages = true From 4a550a1f4fcd80500ddf5a1894229e9d9cfd98f2 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Tue, 26 Feb 2019 20:24:36 -0300 Subject: [PATCH 10/21] Release v4.1.4 --- util/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/version.go b/util/version.go index 6822c263..4fadb3ab 100644 --- a/util/version.go +++ b/util/version.go @@ -8,4 +8,4 @@ package util // VERSION identifies Khan's current version -var VERSION = "4.1.3" +var VERSION = "4.1.4" From 2772173e3dbd19c98d591f4ceea604292996aba9 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Wed, 27 Feb 2019 11:40:07 -0300 Subject: [PATCH 11/21] Fix bson dep after updating extensions. --- Gopkg.lock | 64 +++++++++++++++++++++++++++++++++++++------------- models/clan.go | 4 +++- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index f57f2f65..3959f575 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -47,6 +47,14 @@ pruneopts = "UT" revision = "8a28e9752dbc744626695b749a3ab474b8b372fa" +[[projects]] + branch = "master" + digest = "1:4c4c33075b704791d6a7f09dfb55c66769e8a1dc6adf87026292d274fe8ad113" + name = "github.com/codahale/hdrhistogram" + packages = ["."] + pruneopts = "UT" + revision = "3a0bb77429bd3a61596f5e8a3172445844342120" + [[projects]] digest = "1:02630c70b55ecda2dcb85805170923d8389c632487568ccffadc5482e8a191ed" name = "github.com/dgrijalva/jwt-go" @@ -205,7 +213,6 @@ revision = "04140366298a54a039076d798123ffa108fff46c" [[projects]] - branch = "master" digest = "1:2ed84e4d8015eaf56a29b3ce8512a91973084014061a452d112838351b42364c" name = "github.com/jrallison/go-workers" packages = ["."] @@ -398,10 +405,11 @@ version = "v1.2.0" [[projects]] - digest = "1:9d7b1f07e44cada28e6160cb0a808ad2f71f32bd7114793fb0baa302db0e6229" + digest = "1:450b7623b185031f3a456801155c8320209f75d0d4c4e633c6b1e59d44d6e392" name = "github.com/opentracing/opentracing-go" packages = [ ".", + "ext", "log", ] pruneopts = "UT" @@ -507,7 +515,7 @@ revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" [[projects]] - digest = "1:5deb18e60063a7156c41845e97669b57258064be5145211a4a663c7b8c093da1" + digest = "1:0f26fb9ec35872c6b9df645268f286045bb58534438f8cd463828a8930c81166" name = "github.com/topfreegames/extensions" packages = [ "dogstatsd", @@ -516,6 +524,7 @@ "gorp", "gorp/interfaces", "http", + "jaeger", "middleware", "mongo", "mongo/interfaces", @@ -555,6 +564,40 @@ pruneopts = "UT" revision = "d11d2851fcabcf03421c4dbdbc3146fc74eb1035" +[[projects]] + digest = "1:ac6f26e917fd2fb3194a7ebe2baf6fb32de2f2fbfed130c18aac0e758a6e1d22" + name = "github.com/uber/jaeger-client-go" + packages = [ + ".", + "config", + "internal/baggage", + "internal/baggage/remote", + "internal/spanlog", + "internal/throttler", + "internal/throttler/remote", + "log", + "rpcmetrics", + "thrift", + "thrift-gen/agent", + "thrift-gen/baggage", + "thrift-gen/jaeger", + "thrift-gen/sampling", + "thrift-gen/zipkincore", + "transport", + "utils", + ] + pruneopts = "UT" + revision = "1a782e2da844727691fef1757c72eb190c2909f0" + version = "v2.15.0" + +[[projects]] + digest = "1:0f09db8429e19d57c8346ad76fbbc679341fa86073d3b8fb5ac919f0357d8f4c" + name = "github.com/uber/jaeger-lib" + packages = ["metrics"] + pruneopts = "UT" + revision = "ed3a127ec5fef7ae9ea95b01b542c47fbd999ce5" + version = "v1.5.0" + [[projects]] branch = "master" digest = "1:c468422f334a6b46a19448ad59aaffdfc0a36b08fdcc1c749a0b29b6453d7e59" @@ -695,17 +738,6 @@ pruneopts = "UT" revision = "d805e540cd37b8aa6e2ad7d94f06f6537493e710" -[[projects]] - branch = "v2" - digest = "1:9e8ac8aa4f4c5557ba0b03a55d407b07b7030ed552b437a9dc7e5ec90f16ab39" - name = "gopkg.in/mgo.v2" - packages = [ - "bson", - "internal/json", - ] - pruneopts = "UT" - revision = "3f83fa5005286a7fe593b055f0d7771a7dce4655" - [[projects]] digest = "1:14aa12a4a23b14afe5731023e50b7ad98fd61ab4c17e6fcbf1ea62a151b8d02b" name = "gopkg.in/olivere/elastic.v5" @@ -740,6 +772,7 @@ "github.com/Pallinder/go-randomdata", "github.com/bluele/factory-go/factory", "github.com/getsentry/raven-go", + "github.com/globalsign/mgo/bson", "github.com/go-gorp/gorp", "github.com/golang/mock/gomock", "github.com/gosuri/uiprogress", @@ -767,10 +800,10 @@ "github.com/topfreegames/extensions/gorp", "github.com/topfreegames/extensions/gorp/interfaces", "github.com/topfreegames/extensions/http", + "github.com/topfreegames/extensions/jaeger", "github.com/topfreegames/extensions/middleware", "github.com/topfreegames/extensions/mongo", "github.com/topfreegames/extensions/mongo/interfaces", - "github.com/topfreegames/extensions/tracing", "github.com/topfreegames/extensions/worker/middleware", "github.com/topfreegames/goose/lib/goose", "github.com/uber-go/zap", @@ -778,7 +811,6 @@ "github.com/valyala/fasthttp/fasthttpadaptor", "github.com/valyala/fasttemplate", "gopkg.in/jarcoal/httpmock.v1", - "gopkg.in/mgo.v2/bson", "gopkg.in/olivere/elastic.v5", ] solver-name = "gps-cdcl" diff --git a/models/clan.go b/models/clan.go index 1c11cf7f..6c4aa10f 100644 --- a/models/clan.go +++ b/models/clan.go @@ -14,6 +14,7 @@ import ( "os" "strings" + "github.com/globalsign/mgo/bson" "github.com/go-gorp/gorp" workers "github.com/jrallison/go-workers" "github.com/mailru/easyjson/jlexer" @@ -24,7 +25,6 @@ import ( "github.com/topfreegames/khan/queues" "github.com/topfreegames/khan/util" "github.com/uber-go/zap" - "gopkg.in/mgo.v2/bson" ) // ClanByName allows sorting clans by name @@ -883,11 +883,13 @@ func SearchClan( } if err := db.Run(cmd, &res); err != nil { + fmt.Println("CACA", err, cmd) return []Clan{}, err } clans = make([]Clan, len(res.Cursor.FirstBatch)) for i, raw := range res.Cursor.FirstBatch { if err := raw.Unmarshal(&clans[i]); err != nil { + fmt.Println("CACA2", err) return []Clan{}, err } } From 344b89152f7769ab42be67ab5774f6dc8faf18be Mon Sep 17 00:00:00 2001 From: cscatolini Date: Wed, 27 Feb 2019 13:33:26 -0300 Subject: [PATCH 12/21] Catch SIGTERM and wait a few seconds before exiting. This should be enough for gracefully shutting down in kubernetes because when the pod is marked to terminate kubernetes will no longer forward requests to it. --- Makefile | 4 ++-- api/app.go | 27 ++++++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 4d21e9db..f1281d29 100644 --- a/Makefile +++ b/Makefile @@ -78,14 +78,14 @@ run-verbose: run: @echo "Khan running at http://localhost:8888/" - @go run main.go start -q -c ./config/local.yaml + @go run main.go start -c ./config/local.yaml worker: @echo "Khan Worker running at http://localhost:9999/" @go run main.go worker -d -c ./config/local.yaml run-fast: - @go run main.go start -q --fast -c ./config/local.yaml + @go run main.go start --fast -c ./config/local.yaml build-docker: @docker build -t khan . diff --git a/api/app.go b/api/app.go index 61ee71fe..8acc0af7 100644 --- a/api/app.go +++ b/api/app.go @@ -15,8 +15,10 @@ import ( "net/http" "net/http/pprof" "os" + "os/signal" "strconv" "strings" + "syscall" "time" "github.com/getsentry/raven-go" @@ -51,7 +53,6 @@ import ( type App struct { ID string Debug bool - Background bool Port int Host string ConfigPath string @@ -222,6 +223,7 @@ func (app *App) setConfigurationDefaults() { zap.String("source", "app"), zap.String("operation", "setConfigurationDefaults"), ) + app.Config.SetDefault("graceperiod.ms", 5000) app.Config.SetDefault("healthcheck.workingText", "WORKING") app.Config.SetDefault("postgres.host", "localhost") app.Config.SetDefault("postgres.user", "khan") @@ -685,15 +687,26 @@ func (app *App) Start() { ) defer app.finalizeApp() - log.D(l, "App started.", func(cm log.CM) { + log.I(l, "app started", func(cm log.CM) { cm.Write(zap.String("host", app.Host), zap.Int("port", app.Port)) }) - if app.Background { - go func() { - app.App.Run(app.Engine) - }() - } else { + go func() { app.App.Run(app.Engine) + }() + + sg := make(chan os.Signal) + signal.Notify(sg, syscall.SIGINT, syscall.SIGQUIT, syscall.SIGKILL, syscall.SIGTERM) + + // stop server + select { + case s := <-sg: + graceperiod := app.Config.GetInt("graceperiod.ms") + log.I(l, "shutting down", func(cm log.CM) { + cm.Write(zap.String("signal", fmt.Sprintf("%v", s)), + zap.Int("graceperiod", graceperiod)) + }) + time.Sleep(time.Duration(graceperiod) * time.Millisecond) } + log.I(l, "app stopped") } From 46b1528da472db6849252f1283d0bfa700a99115 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Wed, 27 Feb 2019 14:47:00 -0300 Subject: [PATCH 13/21] Update go-workers version to remove unnecessary logs --- Gopkg.lock | 11 +++++++---- Gopkg.toml | 5 +++++ api/api_suite_test.go | 5 +++++ api/app.go | 16 ++++++++++------ models/helpers_test.go | 10 ++++------ 5 files changed, 31 insertions(+), 16 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 3959f575..af0244fc 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -70,14 +70,15 @@ revision = "f12c6236fe7b5cf6bcf30e5935d08cb079d78334" [[projects]] - digest = "1:aba919ea5e7edff31f1bad7f69dbf29aa4a86a0205b1c6dc8f73a35e6a8babeb" + digest = "1:0594af97b2f4cec6554086eeace6597e20a4b69466eb4ada25adf9f4300dddd2" name = "github.com/garyburd/redigo" packages = [ "internal", "redis", ] pruneopts = "UT" - revision = "80f7de34463b0ed3d7c61303e5619efe1b227f92" + revision = "a69d19351219b6dd56f274f96d85a7014a2ec34e" + version = "v1.6.0" [[projects]] digest = "1:86182b0a05e928b62775e9012b613dbb59bd41ed1c80375bcca40a4ccdea29ba" @@ -213,11 +214,13 @@ revision = "04140366298a54a039076d798123ffa108fff46c" [[projects]] - digest = "1:2ed84e4d8015eaf56a29b3ce8512a91973084014061a452d112838351b42364c" + digest = "1:04e729cc283db01695b1df9e036ecf1feb06988ec9e4ca5f6406a3f374967ea5" name = "github.com/jrallison/go-workers" packages = ["."] pruneopts = "UT" - revision = "dbf81d0b75bbe2fd90ef66a07643dd70cb42a88a" + revision = "d1be24994f2aef33257256035eaba70a112bf57f" + source = "github.com/topfreegames/go-workers" + version = "v1.0.0" [[projects]] digest = "1:88a87d3a1fa2b355e78bde9de127b5e63b3f2d16f1acc7b3d0200b686ce7d608" diff --git a/Gopkg.toml b/Gopkg.toml index f6ee860b..0e18e9e4 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -34,6 +34,11 @@ name = "github.com/mailru/easyjson" branch = "master" +[[override]] + name = "github.com/jrallison/go-workers" + source = "github.com/topfreegames/go-workers" + version = "1.0.0" + [[override]] name = "github.com/spf13/viper" revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" diff --git a/api/api_suite_test.go b/api/api_suite_test.go index a01656ff..c1ac4f90 100644 --- a/api/api_suite_test.go +++ b/api/api_suite_test.go @@ -8,13 +8,18 @@ package api_test import ( + workers "github.com/jrallison/go-workers" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/sirupsen/logrus" "testing" ) func TestApi(t *testing.T) { + wl := logrus.New() + wl.Level = logrus.FatalLevel + workers.SetLogger(wl) RegisterFailHandler(Fail) RunSpecs(t, "Khan - API Suite") } diff --git a/api/app.go b/api/app.go index 8acc0af7..a8421081 100644 --- a/api/app.go +++ b/api/app.go @@ -8,10 +8,8 @@ package api import ( - "bytes" "context" "fmt" - dlog "log" "net/http" "net/http/pprof" "os" @@ -31,6 +29,7 @@ import ( newrelic "github.com/newrelic/go-agent" "github.com/rcrowley/go-metrics" "github.com/satori/go.uuid" + "github.com/sirupsen/logrus" "github.com/spf13/viper" eecho "github.com/topfreegames/extensions/echo" extechomiddleware "github.com/topfreegames/extensions/echo/middleware" @@ -513,10 +512,16 @@ func (app *App) configureGoWorkers() { } l.Debug("Configuring workers...") workers.Configure(opts) - if app.Config.GetBool("webhooks.logToBuf") { - var buf bytes.Buffer - workers.Logger = dlog.New(&buf, "test: ", 0) + + // TODO: replace zap with logrus so we don't need two loggers + wl := logrus.New() + wl.Formatter = new(logrus.JSONFormatter) + if app.Debug { + wl.Level = logrus.DebugLevel + } else { + wl.Level = logrus.InfoLevel } + workers.SetLogger(wl) workers.Middleware.Append(extworkermiddleware.NewResponseTimeMetricsMiddleware(app.DDStatsD)) workers.Process(queues.KhanQueue, app.Dispatcher.PerformDispatchHook, workerCount) @@ -537,7 +542,6 @@ func (app *App) StartWorkers() { jobsStatsPort := app.Config.GetInt("webhooks.statsPort") go workers.StatsServer(jobsStatsPort) } - workers.Run() } diff --git a/models/helpers_test.go b/models/helpers_test.go index 708583a8..cdd5538c 100644 --- a/models/helpers_test.go +++ b/models/helpers_test.go @@ -8,13 +8,12 @@ package models_test import ( - "bytes" "fmt" - dlog "log" "strconv" workers "github.com/jrallison/go-workers" uuid "github.com/satori/go.uuid" + "github.com/sirupsen/logrus" "github.com/spf13/viper" "github.com/topfreegames/extensions/mongo/interfaces" "github.com/topfreegames/khan/models" @@ -83,10 +82,9 @@ func ConfigureAndStartGoWorkers() error { opts["password"] = redisPass } workers.Configure(opts) - if config.GetBool("webhooks.logToBuf") { - var buf bytes.Buffer - workers.Logger = dlog.New(&buf, "test: ", 0) - } + wl := logrus.New() + wl.Level = logrus.FatalLevel + workers.SetLogger(wl) l := kt.NewMockLogger() mongoWorker := models.NewMongoWorker(l, config) From 7d277236628639680fcaad4cde957c14b3ff1c9c Mon Sep 17 00:00:00 2001 From: cscatolini Date: Wed, 27 Feb 2019 14:49:55 -0300 Subject: [PATCH 14/21] Release v4.2.0 --- util/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/version.go b/util/version.go index 4fadb3ab..947f83b1 100644 --- a/util/version.go +++ b/util/version.go @@ -8,4 +8,4 @@ package util // VERSION identifies Khan's current version -var VERSION = "4.1.4" +var VERSION = "4.2.0" From 33c376fcd78ce54f6ed3e4c62cb93d434b915799 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Wed, 27 Feb 2019 15:30:25 -0300 Subject: [PATCH 15/21] Fix logging. --- api/api_suite_test.go | 5 ----- api/app.go | 8 ++++++-- api/app_test.go | 2 +- api/helpers_test.go | 2 +- cmd/start.go | 1 + cmd/worker.go | 1 + models/helpers_test.go | 4 ---- 7 files changed, 10 insertions(+), 13 deletions(-) diff --git a/api/api_suite_test.go b/api/api_suite_test.go index c1ac4f90..a01656ff 100644 --- a/api/api_suite_test.go +++ b/api/api_suite_test.go @@ -8,18 +8,13 @@ package api_test import ( - workers "github.com/jrallison/go-workers" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/sirupsen/logrus" "testing" ) func TestApi(t *testing.T) { - wl := logrus.New() - wl.Level = logrus.FatalLevel - workers.SetLogger(wl) RegisterFailHandler(Fail) RunSpecs(t, "Khan - API Suite") } diff --git a/api/app.go b/api/app.go index a8421081..d1c628e5 100644 --- a/api/app.go +++ b/api/app.go @@ -51,6 +51,7 @@ import ( // App is a struct that represents a Khan API Application type App struct { ID string + Test bool Debug bool Port int Host string @@ -74,9 +75,10 @@ type App struct { } // GetApp returns a new Khan API Application -func GetApp(host string, port int, configPath string, debug bool, logger zap.Logger, fast bool) *App { +func GetApp(host string, port int, configPath string, debug bool, logger zap.Logger, fast, test bool) *App { app := &App{ ID: "default", + Test: test, Fast: fast, Host: host, Port: port, @@ -516,7 +518,9 @@ func (app *App) configureGoWorkers() { // TODO: replace zap with logrus so we don't need two loggers wl := logrus.New() wl.Formatter = new(logrus.JSONFormatter) - if app.Debug { + if app.Test { + wl.Level = logrus.FatalLevel + } else if app.Debug { wl.Level = logrus.DebugLevel } else { wl.Level = logrus.InfoLevel diff --git a/api/app_test.go b/api/app_test.go index 10eb2382..c9dbc75e 100644 --- a/api/app_test.go +++ b/api/app_test.go @@ -66,7 +66,7 @@ var _ = Describe("API Application", func() { Describe("App Struct", func() { It("should create app with custom arguments", func() { l := kt.NewMockLogger() - app := GetApp("127.0.0.1", 9999, "../config/test.yaml", false, l, false) + app := GetApp("127.0.0.1", 9999, "../config/test.yaml", false, l, false, true) Expect(app.Port).To(Equal(9999)) Expect(app.Host).To(Equal("127.0.0.1")) }) diff --git a/api/helpers_test.go b/api/helpers_test.go index 62ae7d69..b2b43819 100644 --- a/api/helpers_test.go +++ b/api/helpers_test.go @@ -68,7 +68,7 @@ func GetFaultyTestDB() models.DB { // GetDefaultTestApp returns a new Khan API Application bound to 0.0.0.0:8888 for test func GetDefaultTestApp() *api.App { l := kt.NewMockLogger() - app := api.GetApp("0.0.0.0", 8888, "../config/test.yaml", true, l, false) + app := api.GetApp("0.0.0.0", 8888, "../config/test.yaml", true, l, false, true) app.Configure() return app } diff --git a/cmd/start.go b/cmd/start.go index 597e3f85..5b50a382 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -55,6 +55,7 @@ environment variables to override configuration keys.`, debug, l, fast, + false, ) log.D(cmdL, "Application created successfully.") diff --git a/cmd/worker.go b/cmd/worker.go index ac7afd20..400c9acb 100644 --- a/cmd/worker.go +++ b/cmd/worker.go @@ -52,6 +52,7 @@ environment variables to override configuration keys.`, debug, l, false, + false, ) log.D(cmdL, "Application created successfully.") diff --git a/models/helpers_test.go b/models/helpers_test.go index cdd5538c..3560c595 100644 --- a/models/helpers_test.go +++ b/models/helpers_test.go @@ -13,7 +13,6 @@ import ( workers "github.com/jrallison/go-workers" uuid "github.com/satori/go.uuid" - "github.com/sirupsen/logrus" "github.com/spf13/viper" "github.com/topfreegames/extensions/mongo/interfaces" "github.com/topfreegames/khan/models" @@ -82,9 +81,6 @@ func ConfigureAndStartGoWorkers() error { opts["password"] = redisPass } workers.Configure(opts) - wl := logrus.New() - wl.Level = logrus.FatalLevel - workers.SetLogger(wl) l := kt.NewMockLogger() mongoWorker := models.NewMongoWorker(l, config) From bc66e1e416c66d1d77af49aecac58c4fbb242701 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Thu, 28 Feb 2019 12:00:50 -0300 Subject: [PATCH 16/21] Add jaeger to worker and improve http calls configuration. --- Gopkg.lock | 7 ++- Gopkg.toml | 4 -- Sidecarfile | 25 ++++++++- api/app.go | 8 +-- api/app_test.go | 3 +- api/clan.go | 2 +- api/dispatcher.go | 120 ++++++++++++++++++++++++++++++++--------- config/default.yaml | 2 +- config/local.yaml | 2 +- config/test.yaml | 2 +- es/es_client.go | 7 +-- models/es_worker.go | 14 +++-- models/mongo_worker.go | 11 +++- 13 files changed, 160 insertions(+), 47 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index af0244fc..01d8bc88 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -518,12 +518,13 @@ revision = "25b30aa063fc18e48662b86996252eabdcf2f0c7" [[projects]] - digest = "1:0f26fb9ec35872c6b9df645268f286045bb58534438f8cd463828a8930c81166" + digest = "1:4102c0f538fcd1b36853878a0f3e33288618fe0e769d059b0be6424991e850dc" name = "github.com/topfreegames/extensions" packages = [ "dogstatsd", "echo", "echo/middleware", + "elastic", "gorp", "gorp/interfaces", "http", @@ -794,12 +795,15 @@ "github.com/newrelic/go-agent", "github.com/onsi/ginkgo", "github.com/onsi/gomega", + "github.com/opentracing/opentracing-go", "github.com/rcrowley/go-metrics", "github.com/satori/go.uuid", + "github.com/sirupsen/logrus", "github.com/spf13/cobra", "github.com/spf13/viper", "github.com/topfreegames/extensions/echo", "github.com/topfreegames/extensions/echo/middleware", + "github.com/topfreegames/extensions/elastic", "github.com/topfreegames/extensions/gorp", "github.com/topfreegames/extensions/gorp/interfaces", "github.com/topfreegames/extensions/http", @@ -807,6 +811,7 @@ "github.com/topfreegames/extensions/middleware", "github.com/topfreegames/extensions/mongo", "github.com/topfreegames/extensions/mongo/interfaces", + "github.com/topfreegames/extensions/tracing", "github.com/topfreegames/extensions/worker/middleware", "github.com/topfreegames/goose/lib/goose", "github.com/uber-go/zap", diff --git a/Gopkg.toml b/Gopkg.toml index 0e18e9e4..04cd4102 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -26,10 +26,6 @@ name = "github.com/go-gorp/gorp" version = "2.1" -[[constraint]] - name = "gopkg.in/olivere/elastic.v5" - version = "5.0.39" - [[constraint]] name = "github.com/mailru/easyjson" branch = "master" diff --git a/Sidecarfile b/Sidecarfile index d51333c3..00d0f444 100644 --- a/Sidecarfile +++ b/Sidecarfile @@ -3,7 +3,30 @@ cmd: command: - /go/bin/agent-linux - --collector.host-port=jaeger-collector.jaeger.svc.cluster.local:14267 - image: jaegertracing/jaeger-agent:1.1 + image: jaegertracing/jaeger-agent:1.10 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 5775 + protocol: UDP + - containerPort: 5778 + protocol: TCP + - containerPort: 6831 + protocol: UDP + - containerPort: 6832 + protocol: UDP + resources: + limits: + cpu: 50m + memory: 50Mi + requests: + cpu: 25m + memory: 15Mi +worker: +- name: jaeger-agent + command: + - /go/bin/agent-linux + - --collector.host-port=jaeger-collector.jaeger.svc.cluster.local:14267 + image: jaegertracing/jaeger-agent:1.10 imagePullPolicy: IfNotPresent ports: - containerPort: 5775 diff --git a/api/app.go b/api/app.go index d1c628e5..a91c3120 100644 --- a/api/app.go +++ b/api/app.go @@ -231,7 +231,9 @@ func (app *App) setConfigurationDefaults() { app.Config.SetDefault("postgres.dbName", "khan") app.Config.SetDefault("postgres.port", 5432) app.Config.SetDefault("postgres.sslMode", "disable") - app.Config.SetDefault("webhooks.timeout", 2) + app.Config.SetDefault("webhooks.timeout", 500) + app.Config.SetDefault("webhooks.maxIdleConnsPerHost", http.DefaultMaxIdleConnsPerHost) + app.Config.SetDefault("webhooks.maxIdleConns", 100) app.Config.SetDefault("elasticsearch.host", "localhost") app.Config.SetDefault("elasticsearch.port", 9234) app.Config.SetDefault("elasticsearch.sniff", true) @@ -416,7 +418,7 @@ func (app *App) addError() { } //GetHooks returns all available hooks -func (app *App) GetHooks() map[string]map[int][]*models.Hook { +func (app *App) GetHooks(ctx context.Context) map[string]map[int][]*models.Hook { l := app.Logger.With( zap.String("source", "app"), zap.String("operation", "GetHooks"), @@ -424,7 +426,7 @@ func (app *App) GetHooks() map[string]map[int][]*models.Hook { start := time.Now() log.D(l, "Retrieving hooks...") - dbHooks, err := models.GetAllHooks(app.db) + dbHooks, err := models.GetAllHooks(app.Db(ctx)) if err != nil { log.E(l, "Retrieve hooks failed.", func(cm log.CM) { cm.Write(zap.String("error", err.Error())) diff --git a/api/app_test.go b/api/app_test.go index c9dbc75e..08b55a52 100644 --- a/api/app_test.go +++ b/api/app_test.go @@ -8,6 +8,7 @@ package api_test import ( + "context" "encoding/json" "fmt" "io/ioutil" @@ -110,7 +111,7 @@ var _ = Describe("API Application", func() { app := GetDefaultTestApp() app.NonblockingStartWorkers() - hooks := app.GetHooks() + hooks := app.GetHooks(context.Background()) Expect(len(hooks[gameID])).To(Equal(2)) Expect(len(hooks[gameID][0])).To(Equal(2)) Expect(len(hooks[gameID][1])).To(Equal(2)) diff --git a/api/clan.go b/api/clan.go index 4b0b7176..af0b0057 100644 --- a/api/clan.go +++ b/api/clan.go @@ -651,7 +651,7 @@ func SearchClansHandler(app *App) func(c echo.Context) error { err = WithSegment("clans-search", c, func() error { log.D(l, "Searching clans...") clans, err = models.SearchClan( - app.MongoDB, + app.MongoDB.WithContext(c.StdContext()), gameID, term, pageSize, diff --git a/api/dispatcher.go b/api/dispatcher.go index 2f66e786..71a589e9 100644 --- a/api/dispatcher.go +++ b/api/dispatcher.go @@ -8,39 +8,92 @@ package api import ( + "bytes" + "context" "encoding/base64" "encoding/json" "fmt" "io" + "io/ioutil" + "net" + "net/http" "net/url" "strings" + "sync" "time" workers "github.com/jrallison/go-workers" + opentracing "github.com/opentracing/opentracing-go" "github.com/satori/go.uuid" + ehttp "github.com/topfreegames/extensions/http" + "github.com/topfreegames/extensions/tracing" "github.com/topfreegames/khan/log" "github.com/topfreegames/khan/queues" - "github.com/topfreegames/khan/util" "github.com/uber-go/zap" - "github.com/valyala/fasthttp" "github.com/valyala/fasttemplate" ) +var once sync.Once +var httpClient *http.Client + const hookInternalFailures = "hook_internal_failures" const requestingHookMilliseconds = "requesting_hook_milliseconds" const requestingHookURLStatus = "requesting_hook_status" //Dispatcher is responsible for sending web hooks to workers type Dispatcher struct { - app *App + app *App + httpClient *http.Client } //NewDispatcher creates a new dispatcher available to our app func NewDispatcher(app *App) (*Dispatcher, error) { d := &Dispatcher{app: app} + d.configureHTTPClient() + d.httpClient = httpClient return d, nil } +func getHTTPTransport( + maxIdleConns, maxIdleConnsPerHost int, +) http.RoundTripper { + if _, ok := http.DefaultTransport.(*http.Transport); !ok { + return http.DefaultTransport // tests use a mock transport + } + + // We can't get http.DefaultTransport here and update its + // fields since it's an exported variable, so other libs could + // also change it and overwrite. This hardcoded values are copied + // from http.DefaultTransport but could be configurable too. + return &http.Transport{ + Proxy: http.ProxyFromEnvironment, + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + DualStack: true, + }).DialContext, + MaxIdleConns: maxIdleConns, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ExpectContinueTimeout: 1 * time.Second, + MaxIdleConnsPerHost: maxIdleConnsPerHost, + } +} + +func (d *Dispatcher) configureHTTPClient() { + timeout := time.Duration(d.app.Config.GetInt("webhooks.timeout")) * time.Millisecond + maxIdleConns := d.app.Config.GetInt("webhooks.maxIdleConns") + maxIdleConnsPerHost := d.app.Config.GetInt("webhooks.maxIdleConnsPerHost") + + once.Do(func() { + httpClient = &http.Client{ + Transport: getHTTPTransport(maxIdleConns, maxIdleConnsPerHost), + Timeout: timeout, + } + ehttp.Instrument(httpClient) + }) +} + //DispatchHook dispatches an event hook for eventType to gameID with the specified payload func (d *Dispatcher) DispatchHook(gameID string, eventType int, payload map[string]interface{}) { payload["type"] = eventType @@ -107,6 +160,11 @@ func basicAuth(username, password string) string { // PerformDispatchHook dispatches web hooks for a specific game and event type func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { + tags := opentracing.Tags{"component": "go-workers"} + span := opentracing.StartSpan("PerformDispatchHook", tags) + defer span.Finish() + defer tracing.LogPanic(span) + ctx := opentracing.ContextWithSpan(context.Background(), span) app := d.app statsd := app.DDStatsD @@ -124,7 +182,7 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { zap.Int64("eventType", eventType), ) - hooks := app.GetHooks() + hooks := app.GetHooks(ctx) if _, ok := hooks[gameID]; !ok { log.D(l, "No hooks found for game.") return @@ -134,17 +192,11 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { return } - timeout := time.Duration(app.Config.GetInt("webhooks.timeout")) * time.Second - for _, hook := range hooks[gameID][int(eventType)] { - log.I(app.Logger, "Sending webhook...", func(cm log.CM) { + log.D(app.Logger, "Sending webhook...", func(cm log.CM) { cm.Write(zap.String("url", hook.URL)) }) - client := fasthttp.Client{ - Name: fmt.Sprintf("khan-%s", util.VERSION), - } - requestURL, err := d.interpolateURL(hook.URL, payload) if err != nil { app.addError() @@ -165,9 +217,19 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { log.D(l, "Requesting Hook URL...", func(cm log.CM) { cm.Write(zap.String("requestURL", requestURL)) }) - req := fasthttp.AcquireRequest() - req.Header.SetMethod("POST") - req.AppendBody(payloadJSON) + + req, err := http.NewRequest("POST", requestURL, bytes.NewBuffer(payloadJSON)) + if err != nil { + log.E(l, "failed to create webhook request", func(cm log.CM) { + cm.Write( + zap.String("requestURL", hook.URL), + zap.Error(err), + ) + }) + continue + } + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(ctx) parsedURL, err := url.Parse(requestURL) if err != nil { @@ -189,14 +251,11 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { password = "" } requestURL = fmt.Sprintf("%s://%s%s", parsedURL.Scheme, parsedURL.Host, parsedURL.RequestURI()) - req.Header.Add("Authorization", "Basic "+basicAuth(username, password)) + req.SetBasicAuth(username, password) } - req.SetRequestURI(requestURL) - resp := fasthttp.AcquireResponse() - start := time.Now() - err = client.DoTimeout(req, resp, timeout) + resp, err := d.httpClient.Do(req) if err != nil { app.addError() tags := []string{"error:timeout"} @@ -207,32 +266,41 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { }) continue } + defer resp.Body.Close() + + body, respErr := ioutil.ReadAll(resp.Body) + if respErr != nil { + log.E(l, "failed to read webhook response", func(cm log.CM) { + cm.Write(zap.String("requestURL", hook.URL), zap.Error(respErr)) + }) + continue + } // elapsed must be set after checking the error // in order avoid noising the avg time with timeouts elapsed := time.Since(start) statsd.Timing(requestingHookMilliseconds, elapsed) - tags := []string{fmt.Sprintf("status:%d", resp.StatusCode())} + tags := []string{fmt.Sprintf("status:%d", resp.StatusCode)} statsd.Increment(requestingHookURLStatus, tags...) - if resp.StatusCode() > 399 { + if resp.StatusCode > 399 { app.addError() log.E(l, "Could not request webhook.", func(cm log.CM) { cm.Write( zap.String("requestURL", hook.URL), - zap.Int("statusCode", resp.StatusCode()), - zap.String("body", string(resp.Body())), + zap.Int("statusCode", resp.StatusCode), + zap.String("body", string(body)), ) }) continue } - log.I(l, "Webhook requested successfully.", func(cm log.CM) { + log.D(l, "Webhook requested successfully.", func(cm log.CM) { cm.Write( - zap.Int("statusCode", resp.StatusCode()), + zap.Int("statusCode", resp.StatusCode), zap.String("requestURL", requestURL), - zap.String("body", string(resp.Body())), + zap.String("body", string(body)), ) }) } diff --git a/config/default.yaml b/config/default.yaml index 07cb1d54..e62a1e26 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -30,7 +30,7 @@ healthcheck: workingText: "WORKING" webhooks: - timeout: 2 + timeout: 500 workers: 5 statsPort: 9999 runStats: true diff --git a/config/local.yaml b/config/local.yaml index a639a140..474662d7 100644 --- a/config/local.yaml +++ b/config/local.yaml @@ -22,7 +22,7 @@ search: pageSize: 50 webhooks: - timeout: 2 + timeout: 500 workers: 5 statsPort: 9999 runStats: true diff --git a/config/test.yaml b/config/test.yaml index 68dfa815..9db2df02 100644 --- a/config/test.yaml +++ b/config/test.yaml @@ -30,7 +30,7 @@ healthcheck: workingText: "WORKING" webhooks: - timeout: 2 + timeout: 500 newrelic: key: "" diff --git a/es/es_client.go b/es/es_client.go index c4eadef9..c4f56b44 100644 --- a/es/es_client.go +++ b/es/es_client.go @@ -5,11 +5,11 @@ import ( "os" "sync" - "gopkg.in/olivere/elastic.v5" - newrelic "github.com/newrelic/go-agent" + eelastic "github.com/topfreegames/extensions/elastic" "github.com/topfreegames/khan/log" "github.com/uber-go/zap" + "gopkg.in/olivere/elastic.v5" ) // Client is the struct of an elasticsearch client @@ -93,10 +93,11 @@ func (es *Client) configureClient() { ) }) var err error - es.Client, err = elastic.NewClient( + es.Client, err = eelastic.NewClient( elastic.SetURL(fmt.Sprintf("http://%s:%d", es.Host, es.Port)), elastic.SetSniff(es.Sniff), ) + if err != nil { log.E(l, "Failed to connect to elasticsearch!", func(cm log.CM) { cm.Write( diff --git a/models/es_worker.go b/models/es_worker.go index 6ceb0bdf..276ff175 100644 --- a/models/es_worker.go +++ b/models/es_worker.go @@ -6,6 +6,8 @@ import ( "time" "github.com/jrallison/go-workers" + opentracing "github.com/opentracing/opentracing-go" + "github.com/topfreegames/extensions/tracing" "github.com/topfreegames/khan/es" "github.com/uber-go/zap" ) @@ -31,6 +33,12 @@ func (w *ESWorker) configureESWorker() { // PerformUpdateES updates the clan into elasticsearc func (w *ESWorker) PerformUpdateES(m *workers.Msg) { + tags := opentracing.Tags{"component": "go-workers"} + span := opentracing.StartSpan("PerformUpdateES", tags) + defer span.Finish() + defer tracing.LogPanic(span) + ctx := opentracing.ContextWithSpan(context.Background(), span) + item := m.Args() data := item.MustMap() @@ -60,7 +68,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { Type("clan"). Id(clanID). BodyString(string(body)). - Do(context.TODO()) + Do(ctx) if err != nil { l.Error("Failed to index clan into Elastic Search") return @@ -74,7 +82,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { Type("clan"). Id(clanID). Doc(clan). - Do(context.TODO()) + Do(ctx) if err != nil { l.Error("Failed to update clan from Elastic Search.", zap.Error(err)) } @@ -86,7 +94,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { Index(index). Type("clan"). Id(clanID). - Do(context.TODO()) + Do(ctx) if err != nil { l.Error("Failed to delete clan from Elastic Search.", zap.Error(err)) diff --git a/models/mongo_worker.go b/models/mongo_worker.go index 7bbe7360..ea5d481d 100644 --- a/models/mongo_worker.go +++ b/models/mongo_worker.go @@ -1,11 +1,14 @@ package models import ( + "context" "fmt" "github.com/jrallison/go-workers" + opentracing "github.com/opentracing/opentracing-go" "github.com/spf13/viper" "github.com/topfreegames/extensions/mongo/interfaces" + "github.com/topfreegames/extensions/tracing" "github.com/topfreegames/khan/mongo" "github.com/uber-go/zap" ) @@ -33,6 +36,12 @@ func (w *MongoWorker) configureMongoWorker(config *viper.Viper) { // PerformUpdateMongo updates the clan into elasticsearc func (w *MongoWorker) PerformUpdateMongo(m *workers.Msg) { + tags := opentracing.Tags{"component": "go-workers"} + span := opentracing.StartSpan("PerformUpdateMongo", tags) + defer span.Finish() + defer tracing.LogPanic(span) + ctx := opentracing.ContextWithSpan(context.Background(), span) + item := m.Args() data := item.MustMap() game := data["game"].(string) @@ -48,7 +57,7 @@ func (w *MongoWorker) PerformUpdateMongo(m *workers.Msg) { ) if w.MongoDB != nil { - mongoCol, mongoSess := w.MongoDB.C(fmt.Sprintf(w.MongoCollectionTemplate, game)) + mongoCol, mongoSess := w.MongoDB.WithContext(ctx).C(fmt.Sprintf(w.MongoCollectionTemplate, game)) defer mongoSess.Close() if op == "update" { From 67270f51a7501ba5a35040420e2ab1c47e71ea57 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Thu, 28 Feb 2019 13:24:58 -0300 Subject: [PATCH 17/21] Clean up logs. --- api/app.go | 6 +++--- api/clan.go | 4 ++-- api/dispatcher.go | 2 +- api/player.go | 2 +- models/es_worker.go | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/api/app.go b/api/app.go index a91c3120..22f44ebc 100644 --- a/api/app.go +++ b/api/app.go @@ -117,7 +117,7 @@ func (app *App) configureSentry() { zap.String("operation", "configureSentry"), ) sentryURL := app.Config.GetString("sentry.url") - log.I(l, fmt.Sprintf("Configuring sentry with URL %s", sentryURL)) + log.D(l, fmt.Sprintf("Configuring sentry with URL %s", sentryURL)) raven.SetDSN(sentryURL) raven.SetRelease(util.VERSION) } @@ -433,7 +433,7 @@ func (app *App) GetHooks(ctx context.Context) map[string]map[int][]*models.Hook }) return nil } - log.I(l, "Hooks retrieved successfully.", func(cm log.CM) { + log.D(l, "Hooks retrieved successfully.", func(cm log.CM) { cm.Write(zap.Duration("hookRetrievalDuration", time.Now().Sub(start))) }) @@ -612,7 +612,7 @@ func (app *App) DispatchHooks(gameID string, eventType int, payload map[string]i start := time.Now() log.D(l, "Dispatching hook...") app.Dispatcher.DispatchHook(gameID, eventType, payload) - log.I(l, "Hook dispatched successfully.", func(cm log.CM) { + log.D(l, "Hook dispatched successfully.", func(cm log.CM) { cm.Write(zap.Duration("hookDispatchDuration", time.Now().Sub(start))) }) return nil diff --git a/api/clan.go b/api/clan.go index af0b0057..b81469ae 100644 --- a/api/clan.go +++ b/api/clan.go @@ -415,7 +415,7 @@ func LeaveClanHandler(app *App) func(c echo.Context) error { return FailWith(500, err.Error(), c) } - log.D(l, "Clan left successfully.", func(cm log.CM) { + log.I(l, "Clan left successfully.", func(cm log.CM) { cm.Write(fields...) }) @@ -552,7 +552,7 @@ func TransferOwnershipHandler(app *App) func(c echo.Context) error { return FailWith(500, err.Error(), c) } - log.D(l, "Clan ownership transfer completed successfully.", func(cm log.CM) { + log.I(l, "Clan ownership transfer completed successfully.", func(cm log.CM) { cm.Write( zap.String("previousOwnerPublicID", previousOwner.PublicID), zap.String("newOwnerPublicID", newOwner.PublicID), diff --git a/api/dispatcher.go b/api/dispatcher.go index 71a589e9..bd82b153 100644 --- a/api/dispatcher.go +++ b/api/dispatcher.go @@ -101,7 +101,7 @@ func (d *Dispatcher) DispatchHook(gameID string, eventType int, payload map[stri payload["timestamp"] = time.Now().Format(time.RFC3339) // Push the work onto the queue. - log.I(d.app.Logger, "Pushing work into dispatch queue.", func(cm log.CM) { + log.D(d.app.Logger, "Pushing work into dispatch queue.", func(cm log.CM) { cm.Write( zap.String("source", "dispatcher"), zap.String("operation", "DispatchHook"), diff --git a/api/player.go b/api/player.go index 8f276e09..7130152f 100644 --- a/api/player.go +++ b/api/player.go @@ -241,7 +241,7 @@ func RetrievePlayerHandler(app *App) func(c echo.Context) error { if err != nil { if err.Error() == fmt.Sprintf("Player was not found with id: %s", publicID) { - log.W(l, "Player was not found.", func(cm log.CM) { + log.D(l, "Player was not found.", func(cm log.CM) { cm.Write(zap.Error(err)) }) return FailWith(http.StatusNotFound, err.Error(), c) diff --git a/models/es_worker.go b/models/es_worker.go index 276ff175..0db798a5 100644 --- a/models/es_worker.go +++ b/models/es_worker.go @@ -74,7 +74,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { return } - l.Info("Successfully indexed clan into Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) + l.Debug("Successfully indexed clan into Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) } else if op == "update" { _, err := w.ES.Client. Update(). @@ -87,7 +87,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { l.Error("Failed to update clan from Elastic Search.", zap.Error(err)) } - l.Info("Successfully updated clan from Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) + l.Debug("Successfully updated clan from Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) } else if op == "delete" { _, err := w.ES.Client. Delete(). @@ -100,7 +100,7 @@ func (w *ESWorker) PerformUpdateES(m *workers.Msg) { l.Error("Failed to delete clan from Elastic Search.", zap.Error(err)) } - l.Info("Successfully deleted clan from Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) + l.Debug("Successfully deleted clan from Elastic Search.", zap.Duration("latency", time.Now().Sub(start))) } } From 0045f164319c3850ee3996fc48dc477b6186f1f0 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Thu, 28 Feb 2019 13:47:49 -0300 Subject: [PATCH 18/21] Fix panic when promoting/demoting member. --- api/membership.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/membership.go b/api/membership.go index 0ae137ea..d5a0f414 100644 --- a/api/membership.go +++ b/api/membership.go @@ -747,7 +747,7 @@ func PromoteOrDemoteMembershipHandler(app *App, action string) func(c echo.Conte }) if err != nil { - return FailWithError(err, c) + return err } err = WithSegment("player-retrieve", c, func() error { From 4c14803a22951889f944b39da6f626ee2f30dc48 Mon Sep 17 00:00:00 2001 From: cscatolini Date: Thu, 28 Feb 2019 14:19:11 -0300 Subject: [PATCH 19/21] Improve hook dispatcher metrics. --- api/dispatcher.go | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/api/dispatcher.go b/api/dispatcher.go index bd82b153..c4c62a3d 100644 --- a/api/dispatcher.go +++ b/api/dispatcher.go @@ -38,7 +38,6 @@ var httpClient *http.Client const hookInternalFailures = "hook_internal_failures" const requestingHookMilliseconds = "requesting_hook_milliseconds" -const requestingHookURLStatus = "requesting_hook_status" //Dispatcher is responsible for sending web hooks to workers type Dispatcher struct { @@ -160,8 +159,8 @@ func basicAuth(username, password string) string { // PerformDispatchHook dispatches web hooks for a specific game and event type func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { - tags := opentracing.Tags{"component": "go-workers"} - span := opentracing.StartSpan("PerformDispatchHook", tags) + jtags := opentracing.Tags{"component": "go-workers"} + span := opentracing.StartSpan("PerformDispatchHook", jtags) defer span.Finish() defer tracing.LogPanic(span) ctx := opentracing.ContextWithSpan(context.Background(), span) @@ -200,7 +199,11 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { requestURL, err := d.interpolateURL(hook.URL, payload) if err != nil { app.addError() - tags := []string{"error:interpolateurl"} + tags := []string{ + "error:true", + fmt.Sprintf("url:%s", hook.URL), + fmt.Sprintf("game:%s", gameID), + } statsd.Increment(hookInternalFailures, tags...) log.E(l, "Could not interpolate webhook.", func(cm log.CM) { @@ -234,7 +237,11 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { parsedURL, err := url.Parse(requestURL) if err != nil { app.addError() - tags := []string{"error:parserequesturl"} + tags := []string{ + "error:true", + fmt.Sprintf("url:%s", hook.URL), + fmt.Sprintf("game:%s", gameID), + } statsd.Increment(hookInternalFailures, tags...) log.E(l, "Could not parse request requestURL.", func(cm log.CM) { @@ -258,7 +265,14 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { resp, err := d.httpClient.Do(req) if err != nil { app.addError() - tags := []string{"error:timeout"} + tags := []string{ + "error:true", + fmt.Sprintf("url:%s", hook.URL), + fmt.Sprintf("game:%s", gameID), + fmt.Sprintf("status:500"), + } + elapsed := time.Since(start) + statsd.Timing(requestingHookMilliseconds, elapsed, tags...) statsd.Increment(hookInternalFailures, tags...) log.E(l, "Could not request webhook.", func(cm log.CM) { @@ -276,13 +290,14 @@ func (d *Dispatcher) PerformDispatchHook(m *workers.Msg) { continue } - // elapsed must be set after checking the error - // in order avoid noising the avg time with timeouts + tags := []string{ + fmt.Sprintf("error:%t", resp.StatusCode > 399), + fmt.Sprintf("url:%s", hook.URL), + fmt.Sprintf("game:%s", gameID), + fmt.Sprintf("status:%d", resp.StatusCode), + } elapsed := time.Since(start) - statsd.Timing(requestingHookMilliseconds, elapsed) - - tags := []string{fmt.Sprintf("status:%d", resp.StatusCode)} - statsd.Increment(requestingHookURLStatus, tags...) + statsd.Timing(requestingHookMilliseconds, elapsed, tags...) if resp.StatusCode > 399 { app.addError() From be8a159f1cc0248c54475e380de4ecad5f8ba29b Mon Sep 17 00:00:00 2001 From: cscatolini Date: Thu, 28 Feb 2019 14:31:14 -0300 Subject: [PATCH 20/21] Release v4.3.0 --- util/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/version.go b/util/version.go index 947f83b1..a933497c 100644 --- a/util/version.go +++ b/util/version.go @@ -8,4 +8,4 @@ package util // VERSION identifies Khan's current version -var VERSION = "4.2.0" +var VERSION = "4.3.0" From 78d57f6cfe5ed9d485d499798b785bbf98081c7d Mon Sep 17 00:00:00 2001 From: Andre Hahn Date: Wed, 5 Jun 2019 11:13:06 -0300 Subject: [PATCH 21/21] Remove db reference from webhook sender --- .travis.yml | 2 +- Gopkg.lock | 1 - api/clan.go | 8 ++++---- api/clan_helpers.go | 2 +- api/membership.go | 2 +- models/clan.go | 2 -- 6 files changed, 7 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index c0131a15..f4633446 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - - 1.9.2 + - 1.9.7 sudo: false diff --git a/Gopkg.lock b/Gopkg.lock index 01d8bc88..419a917f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -815,7 +815,6 @@ "github.com/topfreegames/extensions/worker/middleware", "github.com/topfreegames/goose/lib/goose", "github.com/uber-go/zap", - "github.com/valyala/fasthttp", "github.com/valyala/fasthttp/fasthttpadaptor", "github.com/valyala/fasttemplate", "gopkg.in/jarcoal/httpmock.v1", diff --git a/api/clan.go b/api/clan.go index b81469ae..01197d09 100644 --- a/api/clan.go +++ b/api/clan.go @@ -366,7 +366,7 @@ func LeaveClanHandler(app *App) func(c echo.Context) error { } err = WithSegment("hook-dispatch", c, func() error { - err = dispatchClanOwnershipChangeHook(app, tx, models.ClanLeftHook, clan, previousOwner, newOwner) + err = dispatchClanOwnershipChangeHook(app, models.ClanLeftHook, clan, previousOwner, newOwner) if err != nil { txErr := rb(err) if txErr == nil { @@ -410,12 +410,12 @@ func LeaveClanHandler(app *App) func(c echo.Context) error { return nil }) - err = app.Commit(tx, "Clan left", c, l) + err = app.Commit(tx, "Left clan", c, l) if err != nil { return FailWith(500, err.Error(), c) } - log.I(l, "Clan left successfully.", func(cm log.CM) { + log.I(l, "Left clan successfully.", func(cm log.CM) { cm.Write(fields...) }) @@ -518,7 +518,7 @@ func TransferOwnershipHandler(app *App) func(c echo.Context) error { err = WithSegment("hook-dispatch", c, func() error { err = dispatchClanOwnershipChangeHook( - app, tx, models.ClanOwnershipTransferredHook, + app, models.ClanOwnershipTransferredHook, clan, previousOwner, newOwner, ) if err != nil { diff --git a/api/clan_helpers.go b/api/clan_helpers.go index 2dc55b93..7776c7cf 100644 --- a/api/clan_helpers.go +++ b/api/clan_helpers.go @@ -15,7 +15,7 @@ import ( "github.com/uber-go/zap" ) -func dispatchClanOwnershipChangeHook(app *App, db models.DB, hookType int, clan *models.Clan, previousOwner *models.Player, newOwner *models.Player) error { +func dispatchClanOwnershipChangeHook(app *App, hookType int, clan *models.Clan, previousOwner *models.Player, newOwner *models.Player) error { newOwnerPublicID := "" if newOwner != nil { newOwnerPublicID = newOwner.PublicID diff --git a/api/membership.go b/api/membership.go index d5a0f414..8a7ee5a2 100644 --- a/api/membership.go +++ b/api/membership.go @@ -622,7 +622,7 @@ func DeleteMembershipHandler(app *App) func(c echo.Context) error { if err != nil { return err } - log.D(l, "DB Tx begun successful.") + log.D(l, "DB Tx began successfully.") log.D(l, "Deleting membership...") membership, err = models.DeleteMembership( diff --git a/models/clan.go b/models/clan.go index 6c4aa10f..d82f4205 100644 --- a/models/clan.go +++ b/models/clan.go @@ -559,7 +559,6 @@ func TransferClanOwnership(db DB, gameID, clanPublicID, playerPublicID string, l } } - //Update new owner memberships _, err = deleteMembershipHelper(db, newOwnerMembership, newOwnerMembership.PlayerID) if err != nil { return nil, nil, nil, err @@ -575,7 +574,6 @@ func TransferClanOwnership(db DB, gameID, clanPublicID, playerPublicID string, l return nil, nil, nil, err } - //Update old owner membership err = UpdatePlayerOwnershipCount(db, oldOwnerID) if err != nil { return nil, nil, nil, err