Skip to content

Commit

Permalink
add RequestInfo to GetAnnotations and metrics (#247)
Browse files Browse the repository at this point in the history 
* add RequestInfo to GetAnnotations and metrics

* Add info field to tests

* fix missing reqInfo
  • Loading branch information
@gfr10598
gfr10598 authored May 22, 2019
1 parent 7986dd2 commit a9c2c4c
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 36 deletions.
13 changes: 9 additions & 4 deletions api/v2/api-v2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

/*************************************************************************
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions api/v2/api-v2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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.")
}
Expand Down
54 changes: 27 additions & 27 deletions handler/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -219,37 +219,37 @@ 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()

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
Expand All @@ -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
}

Expand All @@ -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 {
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -331,16 +331,16 @@ 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
}

// No need to validate IP addresses, as they are net.IP
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
}
}
Expand All @@ -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)
}
}

Expand All @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit a9c2c4c

Please sign in to comment.