From dcf20a4a7117e2f9f167d4f7c06d68e1263cbf52 Mon Sep 17 00:00:00 2001 From: Desislava Asenova Date: Fri, 4 Oct 2019 13:07:28 +0300 Subject: [PATCH] Fix logger data enrichment (#84) * Wrap data creation * Apply comments --- handlers/bind.go | 13 +++++------ handlers/deprovision.go | 11 ++++------ handlers/get_binding.go | 13 +++++------ handlers/get_instance.go | 11 ++++------ handlers/last_binding_operation.go | 11 ++++------ handlers/last_operation.go | 11 ++++------ handlers/provision.go | 10 +++------ handlers/unbind.go | 10 ++++----- handlers/update.go | 11 ++++------ utils/context.go | 12 ++++++++++ utils/context_test.go | 35 ++++++++++++++++++++++++++++++ 11 files changed, 84 insertions(+), 64 deletions(-) diff --git a/handlers/bind.go b/handlers/bind.go index 83453d93..4f7ff40a 100644 --- a/handlers/bind.go +++ b/handlers/bind.go @@ -4,12 +4,12 @@ import ( "encoding/json" "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const ( @@ -22,13 +22,10 @@ func (h APIHandler) Bind(w http.ResponseWriter, req *http.Request) { instanceID := vars["instance_id"] bindingID := vars["binding_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(bindLogKey, lager.Data{ - instanceIDLogKey: instanceID, - bindingIDLogKey: bindingID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + bindingIDLogKey: bindingID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) version := getAPIVersion(req) asyncAllowed := false diff --git a/handlers/deprovision.go b/handlers/deprovision.go index 13260316..e8612a24 100644 --- a/handlers/deprovision.go +++ b/handlers/deprovision.go @@ -3,12 +3,12 @@ package handlers import ( "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const deprovisionLogKey = "deprovision" @@ -17,12 +17,9 @@ func (h APIHandler) Deprovision(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(deprovisionLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) details := domain.DeprovisionDetails{ PlanID: req.FormValue("plan_id"), diff --git a/handlers/get_binding.go b/handlers/get_binding.go index 4ac27239..966bf201 100644 --- a/handlers/get_binding.go +++ b/handlers/get_binding.go @@ -4,11 +4,11 @@ import ( "errors" "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const getBindLogKey = "getBinding" @@ -18,13 +18,10 @@ func (h APIHandler) GetBinding(w http.ResponseWriter, req *http.Request) { instanceID := vars["instance_id"] bindingID := vars["binding_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(getBindLogKey, lager.Data{ - instanceIDLogKey: instanceID, - bindingIDLogKey: bindingID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + bindingIDLogKey: bindingID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) version := getAPIVersion(req) if version.Minor < 14 { diff --git a/handlers/get_instance.go b/handlers/get_instance.go index ce9dce7e..f9ea3a4b 100644 --- a/handlers/get_instance.go +++ b/handlers/get_instance.go @@ -4,11 +4,11 @@ import ( "errors" "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const getInstanceLogKey = "getInstance" @@ -17,12 +17,9 @@ func (h APIHandler) GetInstance(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(getInstanceLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) version := getAPIVersion(req) if version.Minor < 14 { diff --git a/handlers/last_binding_operation.go b/handlers/last_binding_operation.go index 3b0c1b1e..f9bbecb9 100644 --- a/handlers/last_binding_operation.go +++ b/handlers/last_binding_operation.go @@ -4,12 +4,12 @@ import ( "errors" "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const lastBindingOperationLogKey = "lastBindingOperation" @@ -24,12 +24,9 @@ func (h APIHandler) LastBindingOperation(w http.ResponseWriter, req *http.Reques OperationData: req.FormValue("operation"), } - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(lastBindingOperationLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) version := getAPIVersion(req) if version.Minor < 14 { diff --git a/handlers/last_operation.go b/handlers/last_operation.go index f994aa97..d185d7cf 100644 --- a/handlers/last_operation.go +++ b/handlers/last_operation.go @@ -3,12 +3,12 @@ package handlers import ( "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const lastOperationLogKey = "lastOperation" @@ -22,12 +22,9 @@ func (h APIHandler) LastOperation(w http.ResponseWriter, req *http.Request) { OperationData: req.FormValue("operation"), } - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(lastOperationLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) logger.Info("starting-check-for-operation") diff --git a/handlers/provision.go b/handlers/provision.go index ef290f27..859a6414 100644 --- a/handlers/provision.go +++ b/handlers/provision.go @@ -4,12 +4,11 @@ import ( "encoding/json" "net/http" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" "github.com/pivotal-cf/brokerapi/utils" ) @@ -26,12 +25,9 @@ func (h *APIHandler) Provision(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(provisionLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) var details domain.ProvisionDetails if err := json.NewDecoder(req.Body).Decode(&details); err != nil { diff --git a/handlers/unbind.go b/handlers/unbind.go index 18518651..ba19faa9 100644 --- a/handlers/unbind.go +++ b/handlers/unbind.go @@ -9,6 +9,7 @@ import ( "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const unbindLogKey = "unbind" @@ -18,13 +19,10 @@ func (h APIHandler) Unbind(w http.ResponseWriter, req *http.Request) { instanceID := vars["instance_id"] bindingID := vars["binding_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(unbindLogKey, lager.Data{ - instanceIDLogKey: instanceID, - bindingIDLogKey: bindingID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + bindingIDLogKey: bindingID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) version := getAPIVersion(req) asyncAllowed := req.FormValue("accepts_incomplete") == "true" diff --git a/handlers/update.go b/handlers/update.go index 809a4501..bebd212e 100644 --- a/handlers/update.go +++ b/handlers/update.go @@ -5,12 +5,12 @@ import ( "net/http" "strconv" - "github.com/pivotal-cf/brokerapi/middlewares" - "code.cloudfoundry.org/lager" "github.com/gorilla/mux" "github.com/pivotal-cf/brokerapi/domain" "github.com/pivotal-cf/brokerapi/domain/apiresponses" + "github.com/pivotal-cf/brokerapi/middlewares" + "github.com/pivotal-cf/brokerapi/utils" ) const updateLogKey = "update" @@ -19,12 +19,9 @@ func (h APIHandler) Update(w http.ResponseWriter, req *http.Request) { vars := mux.Vars(req) instanceID := vars["instance_id"] - correlationID := req.Context().Value(middlewares.CorrelationIDKey).(string) - logger := h.logger.Session(updateLogKey, lager.Data{ - instanceIDLogKey: instanceID, - middlewares.CorrelationIDKey: correlationID, - }) + instanceIDLogKey: instanceID, + }, utils.DataForContext(req.Context(), middlewares.CorrelationIDKey)) var details domain.UpdateDetails if err := json.NewDecoder(req.Body).Decode(&details); err != nil { diff --git a/utils/context.go b/utils/context.go index 5e3f697d..78fabb7f 100644 --- a/utils/context.go +++ b/utils/context.go @@ -3,6 +3,7 @@ package utils import ( "context" + "code.cloudfoundry.org/lager" "github.com/pivotal-cf/brokerapi/domain" ) @@ -40,3 +41,14 @@ func RetrieveServicePlanFromContext(ctx context.Context) *domain.ServicePlan { } return nil } + +func DataForContext(context context.Context, dataKeys ...string) lager.Data { + data := lager.Data{} + for _, key := range dataKeys { + if value := context.Value(key); value != nil { + data[key] = value + } + } + + return data +} diff --git a/utils/context_test.go b/utils/context_test.go index 764a8031..c8aeb84a 100644 --- a/utils/context_test.go +++ b/utils/context_test.go @@ -73,4 +73,39 @@ var _ = Describe("Context", func() { }) }) }) + + Describe("Log data for context", func() { + const testKey = "test-key" + + Context("the provided key is present in the context", func() { + It("returns data containing the key", func() { + expectedValue := "123" + ctx = context.WithValue(ctx, testKey, expectedValue) + + data := utils.DataForContext(ctx, testKey) + value, ok := data[testKey] + Expect(ok).To(BeTrue()) + Expect(value).Should(Equal(expectedValue)) + }) + Context("the key value is a struct", func() { + It("returns data containing the key", func() { + type testType struct{} + expectedValue := testType{} + ctx = context.WithValue(ctx, testKey, expectedValue) + + data := utils.DataForContext(ctx, testKey) + value, ok := data[testKey] + Expect(ok).To(BeTrue()) + Expect(value).Should(Equal(expectedValue)) + }) + }) + }) + Context("the provided key is not in the context", func() { + It("returns data without the key", func() { + data := utils.DataForContext(ctx, testKey) + _, ok := data[testKey] + Expect(ok).To(BeFalse()) + }) + }) + }) })