Reduce cardinality

Dockerfile

@@ -1,4 +1,4 @@
-FROM golang:1.12
+FROM golang:1.13
 
 ENV CGO_ENABLED=0
 
@@ -19,13 +19,10 @@ RUN go mod download
 
 COPY *.go ./
 
+RUN go test -short -v ./...
+RUN go test -run SetupModule -v ./...
+
 RUN gofmt -e -s -d . 2>&1 | tee /gofmt.out && test ! -s /gofmt.out
 RUN go vet .
 RUN golint -set_exit_status
-
-RUN errcheck ./...
-
-RUN go install ./...
-
-RUN go test -short -v ./...
-RUN go test -run SetupModule -v ./...
+RUN errcheck ./...

go.mod

@@ -1,6 +1,6 @@
 module github.com/section-io/module-metrics
 
-go 1.12
+go 1.13
 
 require (
 	github.com/pkg/errors v0.8.1

metrics.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 	"syscall"
@@ -16,31 +17,65 @@ import (
 const maxLabelValueLength = 80
 
 var (
-	filepath string
+	filepath          string
+	isValidHostHeader = regexp.MustCompile(`^[a-z0-9.-]+$`).MatchString
 )
 
-func sanitizeValue(label string, value interface{}) string {
+func sanitizeLabel(label string, value interface{}) (string, string) {
 
-	// Convert to a string, no matter what underlying type it is
-	var labelValue string
-	if value != nil {
-		labelValue = fmt.Sprintf("%v", value)
+	if value == nil || value == "" || value == "-" {
+		return label, ""
 	}
 
+	// Convert to a string, no matter what underlying type it is
+	labelValue := fmt.Sprintf("%v", value)
+	labelValue = strings.TrimSpace(labelValue)
+
 	switch label {
 	case "content_type":
-		labelValue = strings.Split(labelValue, ";")[0]
+		label = "content_type_bucket"
+
+		labelValue = strings.ToLower(labelValue)
+		if strings.HasPrefix(labelValue, "image/") {
+			labelValue = "image"
+		} else if strings.HasPrefix(labelValue, "text/html") {
+			labelValue = "html"
+		} else if strings.HasPrefix(labelValue, "text/css") {
+			labelValue = "css"
+		} else if strings.Contains(labelValue, "javascript") {
+			labelValue = "javascript"
+		} else {
+			labelValue = "other"
+		}
+
 	case "hostname":
 		labelValue = strings.Split(labelValue, ":")[0]
-	}
+		labelValue = strings.ToLower(labelValue)
+		if !isValidHostHeader(labelValue) {
+			labelValue = ""
+		}
 
-	labelValue = strings.TrimSpace(labelValue)
+	case "status":
+		statusInt, _ := strconv.Atoi(labelValue)
+		switch {
+		case statusInt >= 100 && statusInt <= 103:
+		case statusInt >= 200 && statusInt <= 208:
+		case statusInt >= 300 && statusInt <= 308:
+		case statusInt >= 400 && statusInt <= 431:
+		case statusInt == 499:
+		case statusInt >= 500 && statusInt <= 511:
+		default:
+			// If it matches any of the above cases, do nothing (leave labelValue as is)
+			// otherwise set to blank
+			labelValue = ""
+		}
+	}
 
 	if len(labelValue) > maxLabelValueLength {
 		labelValue = labelValue[0:maxLabelValueLength]
 	}
 
-	return labelValue
+	return label, labelValue
 }
 
 func getBytes(l map[string]interface{}) int {
@@ -117,8 +152,9 @@ func StartReader(file io.Reader, output io.Writer, errorWriter io.Writer) {
 			} else {
 				labelValues := map[string]string{}
 
-				for _, label := range p8sLabels {
-					labelValues[label] = sanitizeValue(label, logline[label])
+				for _, label := range logFieldNames {
+					label, value := sanitizeLabel(label, logline[label])
+					labelValues[label] = value
 				}
 				addRequest(labelValues, getBytes(logline))
 			}

metrics_test.go

@@ -30,72 +30,159 @@ func TestCreateLogFifoFails(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestSanitizeContentType(t *testing.T) {
-	const expected = "text/html"
-	actual := sanitizeValue("content_type", "text/html; charset=iso-8859-1")
+func TestUnknownLabelSanitize(t *testing.T) {
+	const expectedLabel = "some_other_field"
+	const expectedValue = "foobar"
+	actual, _ := sanitizeLabel("some_other_field", "foobar")
+	assert.Equal(t, expectedLabel, actual)
+}
+
+func TestSanitizeContentTypeLabelName(t *testing.T) {
+	const expectedLabel = "content_type_bucket"
+	actual, _ := sanitizeLabel("content_type", "text/html")
+	assert.Equal(t, expectedLabel, actual)
+}
 
+func TestSanitizeContentTypeHTML(t *testing.T) {
+	const expected = "html"
+	_, actual := sanitizeLabel("content_type", "text/html; charset=iso-8859-1")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("content_type", "text/html")
 	assert.Equal(t, expected, actual)
 }
 
-func TestSanitizeContentTypeNoSemiColon(t *testing.T) {
-	const expected = "text/html"
-	actual := sanitizeValue("content_type", "text/html")
+func TestSanitizeContentTypeImage(t *testing.T) {
+	const expected = "image"
+	_, actual := sanitizeLabel("content_type", "image/jpg")
+	assert.Equal(t, expected, actual)
 
+	_, actual = sanitizeLabel("content_type", "image/gif")
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeContentTypeCSS(t *testing.T) {
+	const expected = "css"
+	_, actual := sanitizeLabel("content_type", "text/css")
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeContentTypeJavascript(t *testing.T) {
+	const expected = "javascript"
+	_, actual := sanitizeLabel("content_type", "text/javascript")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("content_type", "application/javascript")
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeContentTypeEmpty(t *testing.T) {
 	const expected = ""
-	actual := sanitizeValue("content_type", "")
+	_, actual := sanitizeLabel("content_type", "")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("content_type", "-")
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeContentTypeOther(t *testing.T) {
+	const expected = "other"
+	_, actual := sanitizeLabel("content_type", "foobar")
+	assert.Equal(t, expected, actual)
 
+	_, actual = sanitizeLabel("content_type", "text/rtf")
 	assert.Equal(t, expected, actual)
 }
 
 func TestUnsanitizedLabel(t *testing.T) {
 	const expected = "fooooo3iwac"
-	actual := sanitizeValue("some_unknown_type", " fooooo3iwac ")
+	_, actual := sanitizeLabel("some_unknown_type", " fooooo3iwac ")
 
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeNil(t *testing.T) {
 	const expected = ""
-	actual := sanitizeValue("foo", nil)
+	_, actual := sanitizeLabel("foo", nil)
 
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeHostname(t *testing.T) {
 	const expected = "www.foo.com"
-	actual := sanitizeValue("hostname", "www.foo.com")
+	_, actual := sanitizeLabel("hostname", "www.foo.com")
 
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeHostnameWithPort(t *testing.T) {
 	const expected = "www.foo.com"
-	actual := sanitizeValue("hostname", "www.foo.com:80")
+	_, actual := sanitizeLabel("hostname", "www.foo.com:80")
 
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeHostnameWithSpaces(t *testing.T) {
 	const expected = "www.foo.com"
-	actual := sanitizeValue("hostname", "   www.foo.com   ")
+	_, actual := sanitizeLabel("hostname", "   www.foo.com   ")
 
 	assert.Equal(t, expected, actual)
 }
 
 func TestSanitizeHostnameMissing(t *testing.T) {
 	const expected = ""
-	actual := sanitizeValue("hostname", nil)
 
+	_, actual := sanitizeLabel("hostname", nil)
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("hostname", "-")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("hostname", "    ")
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeHostnameCasing(t *testing.T) {
+	const expected = "www.foo.com"
+	_, actual := sanitizeLabel("hostname", "WWw.FOo.COm")
+
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeHostnameInvalidChars(t *testing.T) {
+	const expected = ""
+
+	_, actual := sanitizeLabel("hostname", "www.fi$h.com")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("hostname", "%(+ ")
 	assert.Equal(t, expected, actual)
 }
 
+func TestSanitizeStatus(t *testing.T) {
+	const expected = "200"
+	_, actual := sanitizeLabel("status", "200")
+
+	assert.Equal(t, expected, actual)
+}
+
+func TestSanitizeStatusInvalid(t *testing.T) {
+	const expected = ""
+
+	_, actual := sanitizeLabel("status", "220")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("status", "foobar")
+	assert.Equal(t, expected, actual)
+
+	_, actual = sanitizeLabel("status", "220foo")
+	assert.Equal(t, expected, actual)
+
+}
+
 func TestSanitizeMaxLength(t *testing.T) {
 	const expected = "01234567890123456789012345678901234567890123456789012345678901234567890123456789"
-	actual := sanitizeValue("hostname", "012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789")
+	_, actual := sanitizeLabel("hostname", "012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789")
 
 	assert.Equal(t, expected, actual)
 }

p8s.go

@@ -26,8 +26,9 @@ var (
 	registry            *prometheus.Registry
 	httpServer          *http.Server
 
-	defaultP8sLabels = []string{"hostname", "status"}
-	p8sLabels        []string
+	defaultP8sLabels   = []string{"hostname"}
+	logFieldNames      []string
+	sanitizedP8sLabels []string
 
 	p8sHTTPServerStarted = false
 
@@ -44,7 +45,13 @@ func addRequest(labels map[string]string, bytes int) {
 // will reset any collected metrics. Returns the registry so additional metrics can be registered.
 func InitMetrics(additionalLabels ...string) *prometheus.Registry {
 
-	p8sLabels = append(defaultP8sLabels, additionalLabels...)
+	logFieldNames = append(defaultP8sLabels, additionalLabels...)
+
+	sanitizedP8sLabels = defaultP8sLabels
+	for _, label := range additionalLabels {
+		label, _ = sanitizeLabel(label, "dummy")
+		sanitizedP8sLabels = append(sanitizedP8sLabels, label)
+	}
 
 	const promeNamespace = "section"
 	registry = prometheus.NewRegistry()
@@ -54,14 +61,14 @@ func InitMetrics(additionalLabels ...string) *prometheus.Registry {
 		Subsystem: promeSubsystem,
 		Name:      "request_count_total",
 		Help:      "Total count of HTTP requests.",
-	}, p8sLabels)
+	}, sanitizedP8sLabels)
 
 	bytesTotal = prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: promeNamespace,
 		Subsystem: promeSubsystem,
 		Name:      "bytes_total",
 		Help:      "Total sum of response bytes.",
-	}, p8sLabels)
+	}, sanitizedP8sLabels)
 
 	jsonParseErrorTotal = prometheus.NewCounter(prometheus.CounterOpts{
 		Namespace: promeNamespace,