You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/08/31 14:58:14 UTC

[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #4993: Adds support for CSV parsing in astats

zrhoffman commented on a change in pull request #4993:
URL: https://github.com/apache/trafficcontrol/pull/4993#discussion_r480168290



##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {

Review comment:
       `pollCTX` is unused

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]
+		} else {
+			value, err := strconv.ParseFloat(line[delim+1:len(line)], 64)
+
+			// Skip values that dont parse
+			if err != nil {
+				continue
+			}
+			atsData.Ats[line[0:delim]] = value
+		}
+	}
+
+	if len(atsData.Ats) < 1 {
+		return stats, nil, errors.New("No 'global' data object found in stats_over_http payload")
+	}
+
+	statMap := atsData.Ats
+
+	// Handle system specific values and remove them from the map for precomputing to not have issues
+	if stats.Loadavg, err = LoadavgFromRawLine(statMap["proc.loadavg"].(string)); err != nil {
+		return stats, nil, fmt.Errorf("Error parsing loadavg for cache '%s': %v", cacheName, err)
+	} else {
+		delete(statMap, "proc.loadavg")
+	}
+
+	if err := stats.AddInterfaceFromRawLine(statMap["proc.net.dev"].(string)); err != nil {
+		return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)
+	} else {
+		delete(statMap, "proc.net.dev")
+	}
+
+	if inf, ok := stats.Interfaces[statMap["inf.name"].(string)]; !ok {
+		return stats, nil, errors.New("/proc/net/dev line didn't match reported interface line")
+	} else {
+		inf.Speed = int64(statMap["inf.speed"].(float64)) //strconv.ParseInt(statMap["inf.speed"].(string), 10, 64)
+		stats.Interfaces[statMap["inf.name"].(string)] = inf
+		delete(statMap, "inf.speed")
+		delete(statMap, "inf.name")
+
+	}
+
+	// Clean up other non-stats entries
+	delete(statMap, "astatsLoad")
+	delete(statMap, "lastReloadRequest")
+	delete(statMap, "version")
+	delete(statMap, "something")
+	delete(statMap, "lastReload")
+	delete(statMap, "configReloadRequests")
+	delete(statMap, "configReloads")

Review comment:
       Can `delete()` be inside a loop through these strings?

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]
+		} else {
+			value, err := strconv.ParseFloat(line[delim+1:len(line)], 64)
+
+			// Skip values that dont parse
+			if err != nil {
+				continue
+			}
+			atsData.Ats[line[0:delim]] = value
+		}
+	}
+
+	if len(atsData.Ats) < 1 {
+		return stats, nil, errors.New("No 'global' data object found in stats_over_http payload")
+	}
+
+	statMap := atsData.Ats
+
+	// Handle system specific values and remove them from the map for precomputing to not have issues
+	if stats.Loadavg, err = LoadavgFromRawLine(statMap["proc.loadavg"].(string)); err != nil {
+		return stats, nil, fmt.Errorf("Error parsing loadavg for cache '%s': %v", cacheName, err)

Review comment:
       In Go, error messages should not be capitalized.

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]
+		} else {
+			value, err := strconv.ParseFloat(line[delim+1:len(line)], 64)
+
+			// Skip values that dont parse
+			if err != nil {
+				continue
+			}
+			atsData.Ats[line[0:delim]] = value
+		}
+	}
+
+	if len(atsData.Ats) < 1 {
+		return stats, nil, errors.New("No 'global' data object found in stats_over_http payload")
+	}
+
+	statMap := atsData.Ats
+
+	// Handle system specific values and remove them from the map for precomputing to not have issues
+	if stats.Loadavg, err = LoadavgFromRawLine(statMap["proc.loadavg"].(string)); err != nil {
+		return stats, nil, fmt.Errorf("Error parsing loadavg for cache '%s': %v", cacheName, err)
+	} else {
+		delete(statMap, "proc.loadavg")
+	}
+
+	if err := stats.AddInterfaceFromRawLine(statMap["proc.net.dev"].(string)); err != nil {
+		return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)

Review comment:
       In Go, error messages should not be capitalized.

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]

Review comment:
       Right-hand value can be just `line[delim+1:]`

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]
+		} else {
+			value, err := strconv.ParseFloat(line[delim+1:len(line)], 64)

Review comment:
       Slice can be `line[delim+1:]`

##########
File path: traffic_monitor/cache/astats_csv.go
##########
@@ -0,0 +1,114 @@
+package cache
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strconv"
+	"strings"
+
+	"github.com/apache/trafficcontrol/lib/go-log"
+)
+
+type astatsDataCsv struct {
+	Ats map[string]interface{}
+}
+
+func astatsCsvParseCsv(cacheName string, data io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
+	var stats Statistics
+	var err error
+	if data == nil {
+		log.Warnf("Cannot read stats data for cache '%s' - nil data reader", cacheName)
+		return stats, nil, errors.New("handler got nil reader")
+	}
+
+	var atsData astatsDataCsv
+	atsData.Ats = make(map[string]interface{})
+	scanner := bufio.NewScanner(data)
+
+	for scanner.Scan() {
+
+		line := scanner.Text()
+		delim := strings.IndexByte(line, ',')
+
+		// No delimiter found, skip this line as invalid
+		if delim < 0 {
+			continue
+		}
+		// Special cases where we just want the string value
+		if strings.Contains(line[0:delim], "proc.") || strings.Contains(line[0:delim], "inf.name") {
+			atsData.Ats[line[0:delim]] = line[delim+1 : len(line)]
+		} else {
+			value, err := strconv.ParseFloat(line[delim+1:len(line)], 64)
+
+			// Skip values that dont parse
+			if err != nil {
+				continue
+			}
+			atsData.Ats[line[0:delim]] = value
+		}
+	}
+
+	if len(atsData.Ats) < 1 {
+		return stats, nil, errors.New("No 'global' data object found in stats_over_http payload")

Review comment:
       In Go, error messages should not be capitalized.

##########
File path: traffic_monitor/cache/astats.go
##########
@@ -68,50 +69,60 @@ type Astats struct {
 	System AstatsSystem           `json:"system"`
 }
 
-func astatsParse(cacheName string, rdr io.Reader) (Statistics, map[string]interface{}, error) {
+func astatsParse(cacheName string, rdr io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
 	var stats Statistics
 	if rdr == nil {
 		log.Warnf("%s handle reader nil", cacheName)
 		return stats, nil, errors.New("handler got nil reader")
 	}
 
-	var astats Astats
-	json := jsoniter.ConfigFastest
-	if err := json.NewDecoder(rdr).Decode(&astats); err != nil {
-		return stats, nil, err
-	}
+	ctx := pollCTX.(*poller.HTTPPollCtx)
 
-	if err := stats.AddInterfaceFromRawLine(astats.System.ProcNetDev); err != nil {
-		return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)
-	}
-	if inf, ok := stats.Interfaces[astats.System.InfName]; !ok {
-		return stats, nil, errors.New("/proc/net/dev line didn't match reported interface line")
-	} else {
-		inf.Speed = int64(astats.System.InfSpeed)
-		stats.Interfaces[astats.System.InfName] = inf
-	}
+	ctype := ctx.HTTPHeader.Get("Content-Type")
 
-	if load, err := LoadavgFromRawLine(astats.System.ProcLoadavg); err != nil {
-		return stats, nil, fmt.Errorf("Failed to parse loadavg line for cache '%s': %v", cacheName, err)
-	} else {
-		stats.Loadavg = load
-	}
+	if ctype == "text/json" || ctype == "text/javascript" || ctype == "" {
+		var astats Astats
+		json := jsoniter.ConfigFastest
+		if err := json.NewDecoder(rdr).Decode(&astats); err != nil {
+			return stats, nil, err
+		}
 
-	stats.NotAvailable = astats.System.NotAvailable
+		if err := stats.AddInterfaceFromRawLine(astats.System.ProcNetDev); err != nil {
+			return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)

Review comment:
       This seems like as good of a time as ever to remove the capitalization from this error message.

##########
File path: traffic_monitor/cache/stats_over_http.go
##########
@@ -19,17 +19,19 @@ package cache
  * under the License.
  */
 
-import "errors"
-import "fmt"
-import "io"
-import "math"
-import "strings"
-import "strconv"
+import (
+	"errors"
+	"fmt"
+	"io"
+	"math"
+	"strconv"
+	"strings"
 
-import "github.com/apache/trafficcontrol/lib/go-log"
-import "github.com/apache/trafficcontrol/traffic_monitor/todata"
+	"github.com/apache/trafficcontrol/lib/go-log"
 
-import "github.com/json-iterator/go"
+	"github.com/apache/trafficcontrol/traffic_monitor/todata"
+	jsoniter "github.com/json-iterator/go"
+)

Review comment:
       These imports should be split into (at least) 3 groups:
   - core package imports
   - ATC imports
   - external library imports

##########
File path: traffic_monitor/cache/astats.csv
##########
@@ -0,0 +1,527 @@
+proxy.process.http.completed_requests,26220072200

Review comment:
       If a license header is added to this file, we don't need to re-add a `.csv` exception to `.dependency_license`, right?

##########
File path: traffic_monitor/cache/astats.go
##########
@@ -68,50 +69,60 @@ type Astats struct {
 	System AstatsSystem           `json:"system"`
 }
 
-func astatsParse(cacheName string, rdr io.Reader) (Statistics, map[string]interface{}, error) {
+func astatsParse(cacheName string, rdr io.Reader, pollCTX interface{}) (Statistics, map[string]interface{}, error) {
 	var stats Statistics
 	if rdr == nil {
 		log.Warnf("%s handle reader nil", cacheName)
 		return stats, nil, errors.New("handler got nil reader")
 	}
 
-	var astats Astats
-	json := jsoniter.ConfigFastest
-	if err := json.NewDecoder(rdr).Decode(&astats); err != nil {
-		return stats, nil, err
-	}
+	ctx := pollCTX.(*poller.HTTPPollCtx)
 
-	if err := stats.AddInterfaceFromRawLine(astats.System.ProcNetDev); err != nil {
-		return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)
-	}
-	if inf, ok := stats.Interfaces[astats.System.InfName]; !ok {
-		return stats, nil, errors.New("/proc/net/dev line didn't match reported interface line")
-	} else {
-		inf.Speed = int64(astats.System.InfSpeed)
-		stats.Interfaces[astats.System.InfName] = inf
-	}
+	ctype := ctx.HTTPHeader.Get("Content-Type")
 
-	if load, err := LoadavgFromRawLine(astats.System.ProcLoadavg); err != nil {
-		return stats, nil, fmt.Errorf("Failed to parse loadavg line for cache '%s': %v", cacheName, err)
-	} else {
-		stats.Loadavg = load
-	}
+	if ctype == "text/json" || ctype == "text/javascript" || ctype == "" {
+		var astats Astats
+		json := jsoniter.ConfigFastest
+		if err := json.NewDecoder(rdr).Decode(&astats); err != nil {
+			return stats, nil, err
+		}
 
-	stats.NotAvailable = astats.System.NotAvailable
+		if err := stats.AddInterfaceFromRawLine(astats.System.ProcNetDev); err != nil {
+			return stats, nil, fmt.Errorf("Failed to parse interface line for cache '%s': %v", cacheName, err)
+		}
+		if inf, ok := stats.Interfaces[astats.System.InfName]; !ok {
+			return stats, nil, errors.New("/proc/net/dev line didn't match reported interface line")
+		} else {
+			inf.Speed = int64(astats.System.InfSpeed)
+			stats.Interfaces[astats.System.InfName] = inf
+		}
+
+		if load, err := LoadavgFromRawLine(astats.System.ProcLoadavg); err != nil {
+			return stats, nil, fmt.Errorf("Failed to parse loadavg line for cache '%s': %v", cacheName, err)
+		} else {
+			stats.Loadavg = load
+		}
 
-	// TODO: what's using these?? Can we get rid of them?
-	astats.Ats["system.astatsLoad"] = float64(astats.System.AstatsLoad)
-	astats.Ats["system.configReloadRequests"] = float64(astats.System.ConfigLoadRequest)
-	astats.Ats["system.configReloads"] = float64(astats.System.ConfigReloads)
-	astats.Ats["system.inf.name"] = astats.System.InfName
-	astats.Ats["system.inf.speed"] = float64(astats.System.InfSpeed)
-	astats.Ats["system.lastReload"] = float64(astats.System.LastReload)
-	astats.Ats["system.lastReloadRequest"] = float64(astats.System.LastReloadRequest)
-	astats.Ats["system.notAvailable"] = stats.NotAvailable
-	astats.Ats["system.proc.loadavg"] = astats.System.ProcLoadavg
-	astats.Ats["system.proc.net.dev"] = astats.System.ProcNetDev
+		stats.NotAvailable = astats.System.NotAvailable
 
-	return stats, astats.Ats, nil
+		// TODO: what's using these?? Can we get rid of them?
+		astats.Ats["system.astatsLoad"] = float64(astats.System.AstatsLoad)
+		astats.Ats["system.configReloadRequests"] = float64(astats.System.ConfigLoadRequest)
+		astats.Ats["system.configReloads"] = float64(astats.System.ConfigReloads)
+		astats.Ats["system.inf.name"] = astats.System.InfName
+		astats.Ats["system.inf.speed"] = float64(astats.System.InfSpeed)
+		astats.Ats["system.lastReload"] = float64(astats.System.LastReload)
+		astats.Ats["system.lastReloadRequest"] = float64(astats.System.LastReloadRequest)
+		astats.Ats["system.notAvailable"] = stats.NotAvailable
+		astats.Ats["system.proc.loadavg"] = astats.System.ProcLoadavg
+		astats.Ats["system.proc.net.dev"] = astats.System.ProcNetDev
+
+		return stats, astats.Ats, nil
+	} else if ctype == "text/csv" {
+		return astatsCsvParseCsv(cacheName, rdr, pollCTX)
+	} else {
+		return stats, nil, fmt.Errorf("Stats Content-Type (%s) can not be parsed by astats", ctype)

Review comment:
       In Go, error messages should not be capitalized.

##########
File path: traffic_monitor/cache/stats_types.go
##########
@@ -105,7 +105,7 @@ type StatsDecoder struct {
 // whatever miscellaneous data was in the payload but not represented by
 // the properties of a Statistics object, so that it can be used in later
 // calculations if necessary.
-type StatisticsParser func(string, io.Reader) (Statistics, map[string]interface{}, error)
+type StatisticsParser func(string, io.Reader, interface{}) (Statistics, map[string]interface{}, error)

Review comment:
       It looks like this is always typecast to `*poller.HTTPPollCtx` type. Any reason we can't skip the typecast and make this `*poller.HTTPPollCtx` directly (though I understand, separately, that `handler.Handler` uses `interface{}` type to avoid an import cycle)?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org