From 7b891d10ee420617cb82c7960b86c315fdd30cbe Mon Sep 17 00:00:00 2001
From: Carl Bergquist <carl@grafana.com>
Date: Wed, 30 Sep 2020 20:12:57 +0200
Subject: [PATCH] Instrumentation: Removes invalid chars from label names
 (#27921)

---
 pkg/infra/metrics/metrics.go      | 27 ++++++++++++++++++++++++++
 pkg/infra/metrics/metrics_test.go | 32 +++++++++++++++++++++++++++++++
 pkg/models/datasource_cache.go    | 10 +++++++++-
 3 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 pkg/infra/metrics/metrics_test.go

diff --git a/pkg/infra/metrics/metrics.go b/pkg/infra/metrics/metrics.go
index 31d2fcb4182..f4cbc6bcac3 100644
--- a/pkg/infra/metrics/metrics.go
+++ b/pkg/infra/metrics/metrics.go
@@ -1,7 +1,10 @@
 package metrics
 
 import (
+	"errors"
+	"fmt"
 	"runtime"
+	"strings"
 
 	"github.com/prometheus/client_golang/prometheus"
 
@@ -589,3 +592,27 @@ func newCounterStartingAtZero(opts prometheus.CounterOpts, labelValues ...string
 
 	return counter
 }
+
+// SanitizeLabelName removes all invalid chars from the label name.
+// If the label name is empty or contains only invalid chars, it
+// will return an error.
+func SanitizeLabelName(name string) (string, error) {
+	if len(name) == 0 {
+		return "", errors.New("label name cannot be empty")
+	}
+
+	out := strings.Builder{}
+	for i, b := range name {
+		if (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || (b >= '0' && b <= '9' && i > 0) {
+			out.WriteRune(b)
+		} else if b == ' ' {
+			out.WriteRune('_')
+		}
+	}
+
+	if out.Len() == 0 {
+		return "", fmt.Errorf("label name only contains invalid chars: %q", name)
+	}
+
+	return out.String(), nil
+}
diff --git a/pkg/infra/metrics/metrics_test.go b/pkg/infra/metrics/metrics_test.go
new file mode 100644
index 00000000000..a523960f103
--- /dev/null
+++ b/pkg/infra/metrics/metrics_test.go
@@ -0,0 +1,32 @@
+package metrics
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLabelNameSanitization(t *testing.T) {
+	testcases := []struct {
+		input    string
+		expected string
+		err      bool
+	}{
+		{input: "job", expected: "job"},
+		{input: "job._loal['", expected: "job_loal"},
+		{input: "", expected: "", err: true},
+		{input: ";;;", expected: "", err: true},
+		{input: "Data source", expected: "Data_source"},
+	}
+
+	for _, tc := range testcases {
+		got, err := SanitizeLabelName(tc.input)
+		if tc.err {
+			assert.Error(t, err)
+		} else {
+			require.NoError(t, err)
+			assert.Equal(t, tc.expected, got)
+		}
+	}
+}
diff --git a/pkg/models/datasource_cache.go b/pkg/models/datasource_cache.go
index 8991423b457..128bde860b5 100644
--- a/pkg/models/datasource_cache.go
+++ b/pkg/models/datasource_cache.go
@@ -10,6 +10,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/grafana/grafana/pkg/infra/metrics"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -72,7 +73,14 @@ type dataSourceTransport struct {
 
 func instrumentRoundtrip(datasourceName string, next http.RoundTripper) promhttp.RoundTripperFunc {
 	return promhttp.RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
-		datasourceLabel := prometheus.Labels{"datasource": datasourceName}
+		datasourceLabelName, err := metrics.SanitizeLabelName(datasourceName)
+		// if the datasource named cannot be turned into a prometheus
+		// label we will skip instrumenting these metrics.
+		if err != nil {
+			return next.RoundTrip(r)
+		}
+
+		datasourceLabel := prometheus.Labels{"datasource": datasourceLabelName}
 
 		requestCounter := datasourceRequestCounter.MustCurryWith(datasourceLabel)
 		requestSummary := datasourceRequestSummary.MustCurryWith(datasourceLabel)