cmd/scollector: fastly collector #1728

Merged
merged 1 commit into from May 11, 2016

Projects

None yet

3 participants

@kylebrandt
Member

No description provided.

@alienth alienth commented on an outdated diff May 11, 2016
metadata/metadata.go
@@ -118,6 +118,7 @@ const (
Thread = "threads"
Timestamp = "timestamp"
Transition = "transitions"
+ USD = "us dollars"
@alienth
alienth May 11, 2016 Contributor

nitpick: "US dollars", like "RPM"

@alienth alienth commented on an outdated diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ }
+ switch value := value.(type) {
+ case int64, float64:
+ var v float64
+ if f, found := value.(float64); found {
+ v = f
+ } else {
+ v = float64(value.(int64))
+ }
+ if shouldDiv {
+ v /= 60.0
+ }
+ AddTS(md, prefix+"."+metricName+suffix, timeStamp, v, ts, metadata.RateType(rateTag), metadata.Unit(unitTag), descTag)
+ case string:
+ // Floats in strings, I know not why, precision perhaps?
+ if f, err := strconv.ParseFloat(value, 64); err != nil {
@alienth
alienth May 11, 2016 edited Contributor

Unhandled error. Seems like we should at least log this?

@alienth alienth commented on the diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ divTag = field.Tag.Get("div")
+ descTag = field.Tag.Get("desc")
+ exclude = field.Tag.Get("exclude") != ""
+ )
+ if exclude || descTag == "" {
+ continue
+ }
+ metricName := jsonTag
+ if metricTag != "" {
+ metricName = metricTag
+ }
+ if metricName == "" {
+ slog.Errorf("Unable to determine metric name for field %s. Skipping.", field.Name)
+ continue
+ }
+ shouldDiv := divTag != ""
@alienth
alienth May 11, 2016 Contributor

Weak check which may cause bugs if things get extended. Why not check to see if it is == 'true'?

@alienth alienth commented on an outdated diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ Status204 int64 `json:"status_204" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 204 response code (No Content)."`
+ Status2xx int64 `json:"status_2xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 2xx response code (Success)."`
+ Status301 int64 `json:"status_301" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 301 response code (Moved Permanently)."`
+ Status302 int64 `json:"status_302" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 302 response code (Found)."`
+ Status304 int64 `json:"status_304" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 304 response code."`
+ Status3xx int64 `json:"status_3xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 3xx response code (Redirection)."`
+ Status4xx int64 `json:"status_4xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 4xx response code (Client Error)."`
+ Status503 int64 `json:"status_503" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 503 response code. (Service Unavailable)"`
+ Status5xx int64 `json:"status_5xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a xxx response code."`
+ Synth int64 `json:"synth"`
+ TLS int64 `json:"tls"`
+ Uncacheable int64 `json:"uncacheable" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests that were uncacheable."`
+ Video int64 `json:"video"`
+}
+
+const fastlyDivDesc = " This metric is collected per minute, but we divide it by 60 seconds in order to normalize the rate to per second instead of per minute."
@alienth
alienth May 11, 2016 Contributor

Intentional leading whitespace?

@alienth alienth and 1 other commented on an outdated diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ Pci int64 `json:"pci"`
+ Requests int64 `json:"requests" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of requests processed"`
+ Shield int64 `json:"shield"`
+ StartTime int64 `json:"start_time" exclude:""`
+ Status1xx int64 `json:"status_1xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 1xx response code (Informational)."`
+ Status200 int64 `json:"status_200" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 200 response code (Success)."`
+ Status204 int64 `json:"status_204" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 204 response code (No Content)."`
+ Status2xx int64 `json:"status_2xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 2xx response code (Success)."`
+ Status301 int64 `json:"status_301" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 301 response code (Moved Permanently)."`
+ Status302 int64 `json:"status_302" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 302 response code (Found)."`
+ Status304 int64 `json:"status_304" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 304 response code."`
+ Status3xx int64 `json:"status_3xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 3xx response code (Redirection)."`
+ Status4xx int64 `json:"status_4xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 4xx response code (Client Error)."`
+ Status503 int64 `json:"status_503" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 503 response code. (Service Unavailable)"`
+ Status5xx int64 `json:"status_5xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a xxx response code."`
+ Synth int64 `json:"synth"`
@alienth
alienth May 11, 2016 edited Contributor

Would add description: 'Synthetic response sent from Varnish. Typically used to send edge-generated error pages.'

@kylebrandt
kylebrandt May 11, 2016 Member

@alienth What would the unit be?

@captncraig captncraig commented on an outdated diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ slog.Errorf("couldn't get stats for region %v: %v", region, err)
+ continue
+ }
+ for _, stats := range statsCollection {
+ FastlyReflectAdd(&md, "fastly", "_by_region", stats, stats.StartTime, region.TagSet())
+ }
+ }
+ return md, nil
+}
+
+type FastlyClient struct {
+ key string
+ client *http.Client
+}
+
+func NewFastlyClient(key string) FastlyClient {
@captncraig
captncraig May 11, 2016 Contributor

don't export anything you don't have to.

@captncraig captncraig commented on the diff May 11, 2016
cmd/scollector/conf/conf.go
@@ -99,6 +100,10 @@ type GoogleAnalytics struct {
Sites []GoogleAnalyticsSite
}
+type Fastly struct {
@captncraig
captncraig May 11, 2016 Contributor

unexport

@alienth
alienth May 11, 2016 Contributor

That's directly referenced in the collectors package.

@captncraig
captncraig May 11, 2016 Contributor

my bad on this one.

@captncraig captncraig commented on an outdated diff May 11, 2016
cmd/scollector/collectors/fastly.go
+ Status301 int64 `json:"status_301" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 301 response code (Moved Permanently)."`
+ Status302 int64 `json:"status_302" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 302 response code (Found)."`
+ Status304 int64 `json:"status_304" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 304 response code."`
+ Status3xx int64 `json:"status_3xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 3xx response code (Redirection)."`
+ Status4xx int64 `json:"status_4xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 4xx response code (Client Error)."`
+ Status503 int64 `json:"status_503" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a 503 response code. (Service Unavailable)"`
+ Status5xx int64 `json:"status_5xx" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests with a xxx response code."`
+ Synth int64 `json:"synth"`
+ TLS int64 `json:"tls"`
+ Uncacheable int64 `json:"uncacheable" div:"true" rate:"gauge" unit:"requests per second" desc:"The number of http requests that were uncacheable."`
+ Video int64 `json:"video"`
+}
+
+const fastlyDivDesc = " This metric is collected per minute, but we divide it by 60 seconds in order to normalize the rate to per second instead of per minute."
+
+func FastlyReflectAdd(md *opentsdb.MultiDataPoint, prefix, suffix string, st interface{}, timeStamp int64, ts opentsdb.TagSet) {
@captncraig
captncraig May 11, 2016 Contributor

unexport

@captncraig
Contributor

lgtm if you unexport anything that doesn't need to be.

@kylebrandt kylebrandt cmd/scollector: fastly collector
9a73372
@kylebrandt kylebrandt merged commit 07a9d92 into master May 11, 2016

2 checks passed

bosun All checks Passed!
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kylebrandt kylebrandt deleted the fastly branch May 18, 2016
@simnv simnv added a commit to simnv/bosun that referenced this pull request May 18, 2016
@kylebrandt @simnv kylebrandt + simnv cmd/scollector: fastly collector (#1728) c8b0d1f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment