From a9c2c4c49226df1cd4114df5b8f80309df11904f Mon Sep 17 00:00:00 2001 From: Gregory Russell Date: Wed, 22 May 2019 14:14:31 -0400 Subject: [PATCH] add RequestInfo to GetAnnotations and metrics (#247) * add RequestInfo to GetAnnotations and metrics * Add info field to tests * fix missing reqInfo --- api/v2/api-v2.go | 13 +++++++--- api/v2/api-v2_test.go | 6 ++--- handler/handler.go | 54 ++++++++++++++++++++--------------------- metrics/metrics.go | 2 +- metrics/metrics_test.go | 2 +- 5 files changed, 41 insertions(+), 36 deletions(-) diff --git a/api/v2/api-v2.go b/api/v2/api-v2.go index ccec2f2f..22ce0fbd 100644 --- a/api/v2/api-v2.go +++ b/api/v2/api-v2.go @@ -51,8 +51,10 @@ type Response struct { } // Annotator defines the GetAnnotations method used for annotating. +// info is an optional string to populate Request.RequestInfo type Annotator interface { - GetAnnotations(ctx context.Context, date time.Time, ips []string) (*Response, error) + // TODO - make info an regular parameter instead of vararg. + GetAnnotations(ctx context.Context, date time.Time, ips []string, info ...string) (*Response, error) } /************************************************************************* @@ -136,8 +138,11 @@ var decodeLogEvery = logx.NewLogEvery(nil, 30*time.Second) // GetAnnotations takes a url, and Request, makes remote call, and returns parsed ResponseV2 // TODO make this unexported once we have migrated all code to use GetAnnotator() -func GetAnnotations(ctx context.Context, url string, date time.Time, ips []string) (*Response, error) { +func GetAnnotations(ctx context.Context, url string, date time.Time, ips []string, info ...string) (*Response, error) { req := NewRequest(date, ips) + if len(info) > 0 { + req.RequestInfo = info[0] + } encodedData, err := json.Marshal(req) if err != nil { return nil, err @@ -203,8 +208,8 @@ type annotator struct { url string } -func (ann annotator) GetAnnotations(ctx context.Context, date time.Time, ips []string) (*Response, error) { - return GetAnnotations(ctx, ann.url, date, ips) +func (ann annotator) GetAnnotations(ctx context.Context, date time.Time, ips []string, info ...string) (*Response, error) { + return GetAnnotations(ctx, ann.url, date, ips, info...) } // GetAnnotator returns a v2.Annotator that uses the provided url to make v2 api requests. diff --git a/api/v2/api-v2_test.go b/api/v2/api-v2_test.go index 68a57a2e..24ffab64 100644 --- a/api/v2/api-v2_test.go +++ b/api/v2/api-v2_test.go @@ -38,13 +38,13 @@ func TestDoRequest(t *testing.T) { ips := []string{"8.8.8.8", "147.1.2.3"} ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - resp, err := api.GetAnnotations(ctx, url, time.Now(), ips) + resp, err := api.GetAnnotations(ctx, url, time.Now(), ips, "reqInfo") if err == nil { t.Fatal("Should have timed out") } ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - resp, err = api.GetAnnotations(ctx, url, time.Now(), ips) + resp, err = api.GetAnnotations(ctx, url, time.Now(), ips, "reqInfo") if err != nil { t.Fatal(err) } @@ -79,7 +79,7 @@ func TestSomeErrors(t *testing.T) { ips := []string{"8.8.8.8", "147.1.2.3"} ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - _, err := api.GetAnnotations(ctx, url, time.Now(), ips) + _, err := api.GetAnnotations(ctx, url, time.Now(), ips, "reqInfo") if callCount != 1 { t.Error("Should have been two calls to server.") } diff --git a/handler/handler.go b/handler/handler.go index e2d6490a..31f76ce8 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -57,24 +57,24 @@ func Annotate(w http.ResponseWriter, r *http.Request) { defer metrics.ActiveRequests.Dec() data, err := ValidateAndParse(r) - if checkError(err, w, 1, "single", tStart) { + if checkError(err, w, "", 1, "single", tStart) { return } result, err := GetMetadataForSingleIP(data) - if checkError(err, w, 1, "single", tStart) { + if checkError(err, w, "", 1, "single", tStart) { return } trackMissingResponses(&result) encodedResult, err := json.Marshal(result) - if checkError(err, w, 1, "single", tStart) { + if checkError(err, w, "", 1, "single", tStart) { return } fmt.Fprint(w, string(encodedResult)) - metrics.RequestTimeHistogramUsec.WithLabelValues("single", "success").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues("unknown", "single", "success").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) } // ValidateAndParse takes a request and validates the URL parameters, @@ -166,7 +166,7 @@ var v2errorLogger = logx.NewLogEvery(nil, time.Second) // AnnotateV2 finds an appropriate Annotator based on the requested Date, and creates a // response with annotations for all parseable IPs. -func AnnotateV2(date time.Time, ips []string) (v2.Response, error) { +func AnnotateV2(date time.Time, ips []string, reqInfo string) (v2.Response, error) { responseMap := make(map[string]*api.GeoData, len(ips)) ann, err := manager.GetAnnotator(date) @@ -219,7 +219,7 @@ func BatchAnnotate(w http.ResponseWriter, r *http.Request) { defer metrics.ActiveRequests.Dec() jsonBuffer, err := ioutil.ReadAll(r.Body) - if checkError(err, w, 0, "batch", tStart) { + if checkError(err, w, "batch-error", 0, "", tStart) { return } r.Body.Close() @@ -227,29 +227,29 @@ func BatchAnnotate(w http.ResponseWriter, r *http.Request) { handleNewOrOld(w, tStart, jsonBuffer) } -func latencyStats(label string, count int, tStart time.Time) { +func latencyStats(source string, label string, count int, tStart time.Time) { switch { case count >= 400: - metrics.RequestTimeHistogramUsec.WithLabelValues(label, "400+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(source, label, "400+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) case count >= 100: - metrics.RequestTimeHistogramUsec.WithLabelValues(label, "100+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(source, label, "100+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) case count >= 20: - metrics.RequestTimeHistogramUsec.WithLabelValues(label, "20+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(source, label, "20+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) case count >= 5: - metrics.RequestTimeHistogramUsec.WithLabelValues(label, "5+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(source, label, "5+").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) default: - metrics.RequestTimeHistogramUsec.WithLabelValues(label, "<5").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(source, label, "<5").Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) } } // TODO - is this now obsolete? -func checkError(err error, w http.ResponseWriter, ipCount int, label string, tStart time.Time) bool { +func checkError(err error, w http.ResponseWriter, reqInfo string, ipCount int, label string, tStart time.Time) bool { if err != nil { switch err { default: // If it isn't loading, client should probably give up instead of retrying. w.WriteHeader(http.StatusInternalServerError) - metrics.RequestTimeHistogramUsec.WithLabelValues(label, err.Error()).Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) + metrics.RequestTimeHistogramUsec.WithLabelValues(reqInfo, label, err.Error()).Observe(float64(time.Since(tStart).Nanoseconds()) / 1000) } fmt.Fprintf(w, err.Error()) return true @@ -260,7 +260,7 @@ func checkError(err error, w http.ResponseWriter, ipCount int, label string, tSt // TODO Leave this here for now to make review easier, rearrange later. func handleOld(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { dataSlice, err := BatchValidateAndParse(jsonBuffer) - if checkError(err, w, 0, "old", tStart) { + if checkError(err, w, "old", 0, "", tStart) { return } @@ -271,7 +271,7 @@ func handleOld(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { // For old request format, we use the date of the first RequestData date := dataSlice[0].Timestamp responseMap, _, err = AnnotateLegacy(date, dataSlice) - if checkError(err, w, len(dataSlice), "old", tStart) { + if checkError(err, w, "old", len(dataSlice), "", tStart) { return } } else { @@ -281,20 +281,20 @@ func handleOld(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { trackMissingResponses(anno) } encodedResult, err := json.Marshal(responseMap) - if checkError(err, w, len(dataSlice), "old", tStart) { + if checkError(err, w, "old", len(dataSlice), "", tStart) { return } fmt.Fprint(w, string(encodedResult)) if len(dataSlice) == 0 { // Don't know if this was legacy or geolite2, so just label it "old" - latencyStats("old", len(dataSlice), tStart) + latencyStats("old", "unknown", len(dataSlice), tStart) } else if geoloader.IsLegacy(dataSlice[0].Timestamp) { // Label this old (api) and legacy (dataset) - latencyStats("old-legacy", len(dataSlice), tStart) + latencyStats("old", "legacy", len(dataSlice), tStart) } else { // Label this old (api) and geolite2 (dataset) - latencyStats("old-geolite2", len(dataSlice), tStart) + latencyStats("old", "geolite2", len(dataSlice), tStart) } } @@ -331,7 +331,7 @@ func handleV2(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { request := v2.Request{} err := json.Unmarshal(jsonBuffer, &request) - if checkError(err, w, 0, "v2", tStart) { + if checkError(err, w, request.RequestInfo, 0, "v2", tStart) { return } @@ -339,8 +339,8 @@ func handleV2(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { response := v2.Response{} if len(request.IPs) > 0 { - response, err = AnnotateV2(request.Date, request.IPs) - if checkError(err, w, len(request.IPs), "v2", tStart) { + response, err = AnnotateV2(request.Date, request.IPs, request.RequestInfo) + if checkError(err, w, request.RequestInfo, len(request.IPs), "v2", tStart) { return } } @@ -349,16 +349,16 @@ func handleV2(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) { } encodedResult, err := json.Marshal(response) - if checkError(err, w, len(request.IPs), "v2", tStart) { + if checkError(err, w, request.RequestInfo, len(request.IPs), "v2", tStart) { return } fmt.Fprint(w, string(encodedResult)) if geoloader.IsLegacy(request.Date) { // Label this v2 (api) and legacy (dataset) - latencyStats("v2-legacy", len(request.IPs), tStart) + latencyStats(request.RequestInfo, "v2-legacy", len(request.IPs), tStart) } else { // Label this v2 (api) and geolite2 (dataset) - latencyStats("v2-geolite2", len(request.IPs), tStart) + latencyStats(request.RequestInfo, "v2-geolite2", len(request.IPs), tStart) } } @@ -373,7 +373,7 @@ func handleNewOrOld(w http.ResponseWriter, tStart time.Time, jsonBuffer []byte) case v2.RequestTag: handleV2(w, tStart, jsonBuffer) default: - if checkError(errors.New("Unknown Request Type"), w, 0, "newOrOld", tStart) { + if checkError(errors.New("Unknown Request Type"), w, "newOrOld", 0, "", tStart) { return } } diff --git a/metrics/metrics.go b/metrics/metrics.go index 1c686be0..b611d50b 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -30,7 +30,7 @@ var ( 10000000, }, }, - []string{"type", "detail"}) + []string{"source", "type", "detail"}) // Note the batch annotate request counted as 1 as well. TotalRequests = promauto.NewCounter(prometheus.CounterOpts{ Name: "annotator_Annotation_Requests_total", diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index b622bbc3..b19e23b5 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -13,7 +13,7 @@ import ( // through the linter. func TestPrometheusMetrics(t *testing.T) { api.RequestTimeHistogram.WithLabelValues("x") - metrics.RequestTimeHistogramUsec.WithLabelValues("x", "x") + metrics.RequestTimeHistogramUsec.WithLabelValues("x", "y", "z") metrics.ErrorTotal.WithLabelValues("x") metrics.RejectionCount.WithLabelValues("x") promtest.LintMetrics(t)