Skip to content

Commit

Permalink
server: log x-request-id header if present
Browse files Browse the repository at this point in the history
The logging middleware isn't able to show the requestId, which is how
we trace requests through all systems. Instead we needed to pull that
header off and pass it around to each endpoint function.

Our timing metrics were always 0ms as ach currently does everything
inmem.
  • Loading branch information
adamdecaf committed Dec 13, 2018
1 parent a28fd40 commit 96eb5d3
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 179 deletions.
1 change: 0 additions & 1 deletion cmd/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ func main() {
// Setup underlying ach service
r := server.NewRepositoryInMemory()
svc = server.NewService(r)
svc = server.LoggingMiddleware(logger)(svc)

// Create HTTP server
handler = server.MakeHTTPHandler(svc, r, log.With(logger, "component", "HTTP"))
Expand Down
55 changes: 46 additions & 9 deletions server/batches.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,18 @@ import (
"net/http"

"github.com/moov-io/ach"
moovhttp "github.com/moov-io/base/http"

"github.com/go-kit/kit/endpoint"
"github.com/go-kit/kit/log"
"github.com/gorilla/mux"
)

type createBatchRequest struct {
FileID string
Batch *ach.Batch

requestId string
}

type createBatchResponse struct {
Expand All @@ -28,19 +32,26 @@ type createBatchResponse struct {

func (r createBatchResponse) error() error { return r.Err }

func createBatchEndpoint(s Service) endpoint.Endpoint {
func createBatchEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(createBatchRequest)
id, e := s.CreateBatch(req.FileID, req.Batch)
id, err := s.CreateBatch(req.FileID, req.Batch)

if req.requestId != "" && logger != nil {
logger.Log("batches", "createBatch", "file", req.FileID, "requestId", req.requestId, "error", err)
}

return createBatchResponse{
ID: id,
Err: e,
Err: err,
}, nil
}
}

func decodeCreateBatchRequest(_ context.Context, r *http.Request) (interface{}, error) {
var req createBatchRequest
req.requestId = moovhttp.GetRequestId(r)

vars := mux.Vars(r)
id, ok := vars["fileID"]
if !ok {
Expand All @@ -58,6 +69,8 @@ func decodeCreateBatchRequest(_ context.Context, r *http.Request) (interface{},

type getBatchesRequest struct {
fileID string

requestId string
}

type getBatchesResponse struct {
Expand All @@ -73,6 +86,8 @@ func (r getBatchesResponse) error() error { return r.Err }

func decodeGetBatchesRequest(_ context.Context, r *http.Request) (interface{}, error) {
var req getBatchesRequest
req.requestId = moovhttp.GetRequestId(r)

vars := mux.Vars(r)
id, ok := vars["fileID"]
if !ok {
Expand All @@ -82,9 +97,12 @@ func decodeGetBatchesRequest(_ context.Context, r *http.Request) (interface{}, e
return req, nil
}

func getBatchesEndpoint(s Service) endpoint.Endpoint {
func getBatchesEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(getBatchesRequest)
if req.requestId != "" && logger != nil {
logger.Log("batches", "getBatches", "file", req.fileID, "requestId", req.requestId)
}
return getBatchesResponse{
Batches: s.GetBatches(req.fileID),
Err: nil,
Expand All @@ -95,6 +113,8 @@ func getBatchesEndpoint(s Service) endpoint.Endpoint {
type getBatchRequest struct {
fileID string
batchID string

requestId string
}

type getBatchResponse struct {
Expand All @@ -106,6 +126,8 @@ func (r getBatchResponse) error() error { return r.Err }

func decodeGetBatchRequest(_ context.Context, r *http.Request) (interface{}, error) {
var req getBatchRequest
req.requestId = moovhttp.GetRequestId(r)

vars := mux.Vars(r)
fileID, ok := vars["fileID"]
if !ok {
Expand All @@ -121,20 +143,27 @@ func decodeGetBatchRequest(_ context.Context, r *http.Request) (interface{}, err
return req, nil
}

func getBatchEndpoint(s Service) endpoint.Endpoint {
func getBatchEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(getBatchRequest)
batch, e := s.GetBatch(req.fileID, req.batchID)
batch, err := s.GetBatch(req.fileID, req.batchID)

if req.requestId != "" && logger != nil {
logger.Log("batches", "getBatche", "file", req.fileID, "requestId", req.requestId, "error", err)
}

return getBatchResponse{
Batch: batch,
Err: e,
Err: err,
}, nil
}
}

type deleteBatchRequest struct {
fileID string
batchID string

requestId string
}

type deleteBatchResponse struct {
Expand All @@ -145,6 +174,8 @@ func (r deleteBatchResponse) error() error { return r.Err }

func decodeDeleteBatchRequest(_ context.Context, r *http.Request) (interface{}, error) {
var req deleteBatchRequest
req.requestId = moovhttp.GetRequestId(r)

vars := mux.Vars(r)
fileID, ok := vars["fileID"]
if !ok {
Expand All @@ -160,11 +191,17 @@ func decodeDeleteBatchRequest(_ context.Context, r *http.Request) (interface{},
return req, nil
}

func deleteBatchEndpoint(s Service) endpoint.Endpoint {
func deleteBatchEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(deleteBatchRequest)
err := s.DeleteBatch(req.fileID, req.batchID)

if req.requestId != "" && logger != nil {
logger.Log("batches", "deleteBatch", "file", req.fileID, "requestId", req.requestId, "error", err)
}

return deleteBatchResponse{
Err: s.DeleteBatch(req.fileID, req.batchID),
Err: err,
}, nil
}
}
90 changes: 73 additions & 17 deletions server/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import (
"strings"

"github.com/moov-io/ach"
moovhttp "github.com/moov-io/base/http"

"github.com/go-kit/kit/endpoint"
"github.com/go-kit/kit/log"
"github.com/go-kit/kit/metrics/prometheus"
"github.com/gorilla/mux"
stdprometheus "github.com/prometheus/client_golang/prometheus"
Expand All @@ -29,6 +31,8 @@ var (

type createFileRequest struct {
File *ach.File

requestId string
}

type createFileResponse struct {
Expand All @@ -38,7 +42,7 @@ type createFileResponse struct {

func (r createFileResponse) error() error { return r.Err }

func createFileEndpoint(s Service, r Repository) endpoint.Endpoint {
func createFileEndpoint(s Service, r Repository, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(createFileRequest)

Expand All @@ -52,9 +56,14 @@ func createFileEndpoint(s Service, r Repository) endpoint.Endpoint {
req.File.ID = NextID()
}

err := r.StoreFile(req.File)
if req.requestId != "" && logger != nil {
logger.Log("files", "createFile", "requestId", req.requestId, "error", err)
}

return createFileResponse{
ID: req.File.ID,
Err: r.StoreFile(req.File),
Err: err,
}, nil
}
}
Expand All @@ -63,6 +72,8 @@ func decodeCreateFileRequest(_ context.Context, request *http.Request) (interfac
var r io.Reader
var req createFileRequest

req.requestId = moovhttp.GetRequestId(request)

// Sets default values
req.File = ach.NewFile()
bs, err := ioutil.ReadAll(request.Body)
Expand Down Expand Up @@ -90,7 +101,9 @@ func decodeCreateFileRequest(_ context.Context, request *http.Request) (interfac
return req, nil
}

type getFilesRequest struct{}
type getFilesRequest struct {
requestId string
}

type getFilesResponse struct {
Files []*ach.File `json:"files"`
Expand All @@ -111,11 +124,15 @@ func getFilesEndpoint(s Service) endpoint.Endpoint {
}

func decodeGetFilesRequest(_ context.Context, r *http.Request) (interface{}, error) {
return getFilesRequest{}, nil
return getFilesRequest{
requestId: moovhttp.GetRequestId(r),
}, nil
}

type getFileRequest struct {
ID string

requestId string
}

type getFileResponse struct {
Expand All @@ -125,13 +142,18 @@ type getFileResponse struct {

func (r getFileResponse) error() error { return r.Err }

func getFileEndpoint(s Service) endpoint.Endpoint {
func getFileEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(getFileRequest)
f, e := s.GetFile(req.ID)
f, err := s.GetFile(req.ID)

if req.requestId != "" && logger != nil {
logger.Log("files", "getFile", "requestId", req.requestId, "error", err)
}

return getFileResponse{
File: f,
Err: e,
Err: err,
}, nil
}
}
Expand All @@ -142,11 +164,16 @@ func decodeGetFileRequest(_ context.Context, r *http.Request) (interface{}, erro
if !ok {
return nil, ErrBadRouting
}
return getFileRequest{ID: id}, nil
return getFileRequest{
ID: id,
requestId: moovhttp.GetRequestId(r),
}, nil
}

type deleteFileRequest struct {
ID string

requestId string
}

type deleteFileResponse struct {
Expand All @@ -155,11 +182,17 @@ type deleteFileResponse struct {

func (r deleteFileResponse) error() error { return r.Err }

func deleteFileEndpoint(s Service) endpoint.Endpoint {
func deleteFileEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(deleteFileRequest)
err := s.DeleteFile(req.ID)

if req.requestId != "" && logger != nil {
logger.Log("files", "deleteFile", "requestId", req.requestId, "error", err)
}

return deleteFileResponse{
Err: s.DeleteFile(req.ID),
Err: err,
}, nil
}
}
Expand All @@ -170,11 +203,16 @@ func decodeDeleteFileRequest(_ context.Context, r *http.Request) (interface{}, e
if !ok {
return nil, ErrBadRouting
}
return deleteFileRequest{ID: id}, nil
return deleteFileRequest{
ID: id,
requestId: moovhttp.GetRequestId(r),
}, nil
}

type getFileContentsRequest struct {
ID string

requestId string
}

type getFileContentsResponse struct {
Expand All @@ -183,14 +221,18 @@ type getFileContentsResponse struct {

func (v getFileContentsResponse) error() error { return v.Err }

func getFileContentsEndpoint(s Service) endpoint.Endpoint {
func getFileContentsEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(getFileContentsRequest)
r, err := s.GetFileContents(req.ID)

if req.requestId != "" && logger != nil {
logger.Log("files", "getFileContents", "requestId", req.requestId, "error", err)
}
if err != nil {
// TODO(adam): log? if requestId != ""
return getFileContentsResponse{Err: err}, nil
}

return r, nil
}
}
Expand All @@ -201,11 +243,16 @@ func decodeGetFileContentsRequest(_ context.Context, r *http.Request) (interface
if !ok {
return nil, ErrBadRouting
}
return getFileContentsRequest{ID: id}, nil
return getFileContentsRequest{
ID: id,
requestId: moovhttp.GetRequestId(r),
}, nil
}

type validateFileRequest struct {
ID string

requestId string
}

type validateFileResponse struct {
Expand All @@ -214,11 +261,17 @@ type validateFileResponse struct {

func (v validateFileResponse) error() error { return v.Err }

func validateFileEndpoint(s Service) endpoint.Endpoint {
func validateFileEndpoint(s Service, logger log.Logger) endpoint.Endpoint {
return func(_ context.Context, request interface{}) (interface{}, error) {
req := request.(validateFileRequest)
err := s.ValidateFile(req.ID)

if req.requestId != "" && logger != nil {
logger.Log("files", "validateFile", "requestId", req.requestId, "error", err)
}

return validateFileResponse{
Err: s.ValidateFile(req.ID),
Err: err,
}, nil
}
}
Expand All @@ -229,5 +282,8 @@ func decodeValidateFileRequest(_ context.Context, r *http.Request) (interface{},
if !ok {
return nil, ErrBadRouting
}
return validateFileRequest{ID: id}, nil
return validateFileRequest{
ID: id,
requestId: moovhttp.GetRequestId(r),
}, nil
}
Loading

0 comments on commit 96eb5d3

Please sign in to comment.