Default query response time to P999

metrics/query.response-time/response_time.go

@@ -7,6 +7,7 @@ import (
 	"database/sql"
 	"fmt"
 	"math"
+	"strconv"
 	"strings"
 	"time"
 
@@ -24,6 +25,8 @@ const (
 	OPT_TRUNCATE_TABLE   = "truncate-table"
 	OPT_TRUNCATE_TIMEOUT = "truncate-timeout"
 
+	defaultPercentile = "p999"
+
 	ERR_NO_TABLE        = "table-not-exist"
 	ERR_TRUNCATE_FAILED = "truncate-timeout"
 
@@ -63,6 +66,22 @@ func NewResponseTime(db *sql.DB) *ResponseTime {
 	}
 }
 
+func percentileMetrics(metrics []string) ([]sqlutil.P, error) {
+	if len(metrics) == 0 {
+		metrics = []string{defaultPercentile}
+	}
+	for _, metric := range metrics {
+		if len(metric) < 2 || (metric[0] != 'p' && metric[0] != 'P') {
+			return nil, fmt.Errorf("invalid percentile metric %q: expected pN where N is an integer from 1 through 999", metric)
+		}
+		n, err := strconv.Atoi(metric[1:])
+		if err != nil || n < 1 || n > 999 {
+			return nil, fmt.Errorf("invalid percentile metric %q: expected pN where N is an integer from 1 through 999", metric)
+		}
+	}
+	return sqlutil.PercentileMetrics(metrics)
+}
+
 // Domain returns the Blip metric domain name (DOMAIN const).
 func (c *ResponseTime) Domain() string {
 	return DOMAIN
@@ -165,8 +184,9 @@ LEVEL:
 			config.lockWaitQuery = fmt.Sprintf(LOCKWAIT_QUERY, int64(lockWaitTimeout))
 		}
 
-		// Process list of percentiles metrics into a list of names and values
-		p, err := sqlutil.PercentileMetrics(dom.Metrics)
+		// Process the configured percentile metrics into names and values,
+		// applying and validating the documented default during preparation.
+		p, err := percentileMetrics(dom.Metrics)
 		if err != nil {
 			return nil, err
 		}

metrics/query.response-time/response_time_test.go

@@ -4,12 +4,119 @@ package queryresponsetime
 
 import (
 	"context"
+	"strings"
 	"testing"
 
+	"github.com/cashapp/blip/v2"
 	"github.com/cashapp/blip/v2/sqlutil"
 	"github.com/cashapp/blip/v2/test"
 )
 
+func TestPrepareDefaultsToP999(t *testing.T) {
+	c := NewResponseTime(nil)
+	plan := blip.Plan{
+		Levels: map[string]blip.Level{
+			"kpi": {
+				Name: "kpi",
+				Collect: map[string]blip.Domain{
+					DOMAIN: {},
+				},
+			},
+		},
+	}
+
+	_, err := c.Prepare(context.Background(), plan)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	percentiles := c.atLevel["kpi"].percentiles
+	if len(percentiles) != 1 {
+		t.Fatalf("prepared %d percentiles, expected 1: %+v", len(percentiles), percentiles)
+	}
+	if percentiles[0].formatted != defaultPercentile {
+		t.Errorf("prepared percentile %q, expected %q", percentiles[0].formatted, defaultPercentile)
+	}
+	wantQuery := BASE_QUERY + " WHERE bucket_quantile >= 0.999000 ORDER BY bucket_number LIMIT 1"
+	if percentiles[0].query != wantQuery {
+		t.Errorf("prepared query %q, expected %q", percentiles[0].query, wantQuery)
+	}
+}
+
+func TestPrepareRejectsInvalidPercentileMetrics(t *testing.T) {
+	for _, metric := range []string{"99", "p0", "p1000", "p99.9", "pfoo"} {
+		t.Run(metric, func(t *testing.T) {
+			c := NewResponseTime(nil)
+			plan := blip.Plan{
+				Levels: map[string]blip.Level{
+					"kpi": {
+						Name: "kpi",
+						Collect: map[string]blip.Domain{
+							DOMAIN: {Metrics: []string{metric}},
+						},
+					},
+				},
+			}
+
+			_, err := c.Prepare(context.Background(), plan)
+			if err == nil {
+				t.Fatalf("Prepare accepted invalid percentile metric %q", metric)
+			}
+			if !strings.Contains(err.Error(), "expected pN where N is an integer from 1 through 999") {
+				t.Errorf("Prepare error %q does not explain the percentile metric format", err)
+			}
+		})
+	}
+}
+
+func TestCollectDefaultsToP999(t *testing.T) {
+	_, db, err := test.Connection("mysql80")
+	if err != nil {
+		t.Skip("mysql80 not running")
+	}
+	defer db.Close()
+
+	c := NewResponseTime(db)
+	plan := blip.Plan{
+		Levels: map[string]blip.Level{
+			"kpi": {
+				Name: "kpi",
+				Collect: map[string]blip.Domain{
+					DOMAIN: {
+						Options: map[string]string{OPT_TRUNCATE_TABLE: "no"},
+					},
+				},
+			},
+		},
+	}
+
+	_, err = c.Prepare(context.Background(), plan)
+	if err != nil {
+		t.Fatal(err)
+	}
+	metrics, err := c.Collect(context.Background(), "kpi")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(metrics) != 1 {
+		t.Fatalf("collected %d metrics, expected 1: %+v", len(metrics), metrics)
+	}
+	if metrics[0].Name != defaultPercentile {
+		t.Errorf("collected metric %q, expected %q", metrics[0].Name, defaultPercentile)
+	}
+	realPercentile, ok := metrics[0].Meta[defaultPercentile]
+	if !ok {
+		t.Fatalf("metric meta does not have key %q: %+v", defaultPercentile, metrics[0].Meta)
+	}
+	value, err := sqlutil.ParsePercentileStr(realPercentile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if value < 0.999 {
+		t.Errorf("real percentile %s is %f, expected at least 0.999", realPercentile, value)
+	}
+}
+
 func TestCollectP(t *testing.T) {
 	_, db, err := test.Connection("mysql80")
 	if err != nil {