Prometheus' data model requires more characters be switched (#87)

Follow prometheus' data model for metric names better, allow for flexible replace of '.'.
Update README correctly and ensure periodCharacter is used in queries.

Co-authored-by: John Seekins <[email protected]>

Author: johnseekins

README.md

@@ -73,6 +73,11 @@ Geras additionally listens on a HTTP port for Prometheus `/metrics` queries and
   -metrics-name-response-rewriting
         Rewrite '.' to ':' and '-' to '_' in all responses (Prometheus
         remote_read won't accept these, while Thanos will) (default true)
+  -metrics-name-response-rewriting
+        Rewrite '.' to a defined character and other bad characters to '_' in all responses (Prometheus
+        remote_read won't accept these, while Thanos will) (default true)
+  -period-character-replace
+		Rewrite '.' to a defined charater that Prometheus will handle better. (default ':')
 ```
 
 When specifying multiple labels, you will need to repeat the argument name, e.g:
@@ -86,6 +91,6 @@ When specifying multiple labels, you will need to repeat the argument name, e.g:
 * PromQL supports queries without `__name__`. This is not possible in OpenTSDB and no results will be returned if the query doesn't match on a metric name.
 * Geras periodically loads metric names from OpenTSDB and keeps them in memory to support queries like `{__name__=~"regexp"}`.
 * Thanos' primary timeseries backend is Prometheus, which doesn't support unquoted dots in metric names. However OpenTSDB metrics generally use `.` as a seperator within names. In order to query names containing a `.` you will need to either:
-  * Replace all `.` with `:` in your query; or
+  * Replace all `.` with another character (we like `:`).
   * Use the `__name__` label to specify the metric name, e.g. `{__name__="cpu.percent"}`
   * Also watch out for `-` (dashes) in your metric names

cmd/geras/main.go

@@ -114,7 +114,8 @@ func main() {
 	allowedMetricNamesRE := flag.String("metrics-allowed-regexp", ".*", "Regexp of metrics to allow")
 	blockedMetricNamesRE := flag.String("metrics-blocked-regexp", "", "Regexp of metrics to block (empty disables blocking)")
 	enableMetricSuggestions := flag.Bool("metrics-suggestions", true, "Enable metric suggestions (can be expensive)")
-	enableMetricNameRewriting := flag.Bool("metrics-name-response-rewriting", true, "Rewrite '.' to ':' and '-' to '_' in all responses (Prometheus remote_read won't accept these, while Thanos will)")
+	periodCharacter := flag.String("period-character-replace", ":", "Replace any periods in metric name with this character")
+	enableMetricNameRewriting := flag.Bool("metrics-name-response-rewriting", true, "Rewrite any character not in '[^a-zA-Z0-9_:.]' to '_' in all responses and '.' to a defined charater (Prometheus remote_read won't accept these, while Thanos will)")
 	var labels multipleStringFlags
 	flag.Var(&labels, "label", "Label to expose on the Store API, of the form '<key>=<value>'. May be repeated.")
 	flag.Parse()
@@ -185,7 +186,7 @@ func main() {
 	prometheus.DefaultRegisterer.MustRegister(version.NewCollector("geras"))
 
 	// create openTSDBStore and expose its api on a grpc server
-	srv := store.NewOpenTSDBStore(logger, client, prometheus.DefaultRegisterer, *refreshInterval, *refreshTimeout, storeLabels, allowedMetricNames, blockedMetricNames, *enableMetricSuggestions, *enableMetricNameRewriting, *healthcheckMetric)
+	srv := store.NewOpenTSDBStore(logger, client, prometheus.DefaultRegisterer, *refreshInterval, *refreshTimeout, storeLabels, allowedMetricNames, blockedMetricNames, *enableMetricSuggestions, *enableMetricNameRewriting, *healthcheckMetric, *periodCharacter)
 	grpcSrv := grpc.NewServer(
 		grpc.StreamInterceptor(grpc_prometheus.StreamServerInterceptor),
 		grpc.UnaryInterceptor(grpc_prometheus.UnaryServerInterceptor),

go.mod

@@ -1,5 +1,7 @@
 module github.com/G-Research/geras
 
+go 1.15
+
 require (
 	github.com/G-Research/opentsdb-goclient v0.0.0-20191219203319-f9f2aa5b2624
 	github.com/go-kit/kit v0.9.0

pkg/store/store.go

@@ -41,6 +41,7 @@ type OpenTSDBStore struct {
 	storeLabels                            []storepb.Label
 	storeLabelsMap                         map[string]string
 	healthcheckMetric                      string
+	periodCharacter                        string
 }
 
 var (
@@ -52,6 +53,7 @@ var (
 		storepb.Aggr_COUNTER: "avg",
 	}
 	downsampleToAggregate map[string]storepb.Aggr
+	replaceChars          = regexp.MustCompile("[^a-zA-Z0-9_:.]")
 )
 
 func init() {
@@ -64,7 +66,7 @@ func init() {
 	}
 }
 
-func NewOpenTSDBStore(logger log.Logger, client opentsdb.ClientContext, reg prometheus.Registerer, refreshInterval, refreshTimeout time.Duration, storeLabels []storepb.Label, allowedMetricNames, blockedMetricNames *regexp.Regexp, enableMetricSuggestions, enableMetricNameRewriting bool, healthcheckMetric string) *OpenTSDBStore {
+func NewOpenTSDBStore(logger log.Logger, client opentsdb.ClientContext, reg prometheus.Registerer, refreshInterval, refreshTimeout time.Duration, storeLabels []storepb.Label, allowedMetricNames, blockedMetricNames *regexp.Regexp, enableMetricSuggestions, enableMetricNameRewriting bool, healthcheckMetric string, periodCharacter string) *OpenTSDBStore {
 	// Extract the store labels into a map for faster access later
 	storeLabelsMap := map[string]string{}
 	for _, l := range storeLabels {
@@ -83,6 +85,7 @@ func NewOpenTSDBStore(logger log.Logger, client opentsdb.ClientContext, reg prom
 		allowedMetricNames:        allowedMetricNames,
 		blockedMetricNames:        blockedMetricNames,
 		healthcheckMetric:         healthcheckMetric,
+		periodCharacter:           periodCharacter,
 	}
 	if client != nil {
 		store.updateMetrics(context.Background(), logger)
@@ -376,8 +379,8 @@ func (store *OpenTSDBStore) getMatchingMetricNames(matcher storepb.LabelMatcher)
 	if matcher.Name != "__name__" {
 		return nil, errors.New("getMatchingMetricNames must be called on __name__ matcher")
 	}
-	if matcher.Type == storepb.LabelMatcher_EQ {
-		value := strings.Replace(matcher.Value, ":", ".", -1)
+	if matcher.Type == storepb.LabelMatcher_EQ && store.periodCharacter != "" {
+		value := strings.Replace(matcher.Value, store.periodCharacter, ".", -1)
 		return []string{value}, nil
 	} else if matcher.Type == storepb.LabelMatcher_NEQ {
 		// we can support this, but we should not.
@@ -572,7 +575,10 @@ func (store *OpenTSDBStore) checkMetricNames(metricNames []string, fullBlock boo
 func (store *OpenTSDBStore) convertOpenTSDBResultsToSeriesResponse(respI *opentsdb.QueryRespItem) (*storepb.SeriesResponse, int, error) {
 	name := respI.Metric
 	if store.enableMetricNameRewriting {
-		name = strings.ReplaceAll(strings.ReplaceAll(name, ".", ":"), "-", "_")
+		name = replaceChars.ReplaceAllString(name, "_")
+		if store.periodCharacter != "" {
+			name = strings.ReplaceAll(name, ".", store.periodCharacter)
+		}
 	}
 	seriesLabels := make([]storepb.Label, 1+len(respI.Tags)+len(store.storeLabels))
 	i := 0

pkg/store/store_test.go

@@ -886,7 +886,7 @@ func TestComposeOpenTSDBQuery(t *testing.T) {
 			allowedMetrics = test.allowedMetrics
 		}
 		store := NewOpenTSDBStore(
-			log.NewJSONLogger(&testLogger{t}), nil, nil, time.Duration(0), 1*time.Minute, nil, allowedMetrics, test.blockedMetrics, false, false, "foo")
+			log.NewJSONLogger(&testLogger{t}), nil, nil, time.Duration(0), 1*time.Minute, nil, allowedMetrics, test.blockedMetrics, false, false, "foo", ":")
 		store.metricNames = test.knownMetrics
 		p, _, err := store.composeOpenTSDBQuery(&test.req)
 		if test.err != nil {
@@ -1187,7 +1187,7 @@ func TestConvertOpenTSDBResultsToSeriesResponse(t *testing.T) {
 	}
 	for i, test := range testCases {
 		store := NewOpenTSDBStore(
-			log.NewJSONLogger(&testLogger{t}), nil, nil, time.Duration(0), 1*time.Minute, test.storeLabels, nil, nil, false, true, "foo")
+			log.NewJSONLogger(&testLogger{t}), nil, nil, time.Duration(0), 1*time.Minute, test.storeLabels, nil, nil, false, true, "foo", ":")
 		converted, _, err := store.convertOpenTSDBResultsToSeriesResponse(&test.input)
 		if err != nil {
 			t.Errorf("unexpected error: %s", err.Error())
@@ -1343,6 +1343,7 @@ func TestGetMatchingMetricNames(t *testing.T) {
 				"tsd.rpc.received",
 				"tsd.rpc.unauthorized",
 			},
+			periodCharacter: ":",
 		}
 		output, err := store.getMatchingMetricNames(test.input)