Fixed query start/end timestamp rounding (#2990)

* Fixed query start/end timestamp rounding

Signed-off-by: Marco Pracucci <[email protected]>

* Added integration test

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed linter

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed integration test

Signed-off-by: Marco Pracucci <[email protected]>

* Improved tests

Signed-off-by: Marco Pracucci <[email protected]>

Author: pracucci

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## master / unreleased
 
 * [ENHANCEMENT] Query-tee: added a small tolerance to floating point sample values comparison. #2994
+* [BUGFIX] Query-frontend: Fixed rounding for incoming query timestamps, to be 100% Prometheus compatible. #2990
 
 ## 1.3.0 in progress
 

integration/e2ecortex/client.go

@@ -112,12 +112,22 @@ func (c *Client) Push(timeseries []prompb.TimeSeries) (*http.Response, error) {
 	return res, nil
 }
 
-// Query runs a query
+// Query runs an instant query.
 func (c *Client) Query(query string, ts time.Time) (model.Value, error) {
 	value, _, err := c.querierClient.Query(context.Background(), query, ts)
 	return value, err
 }
 
+// Query runs a query range.
+func (c *Client) QueryRange(query string, start, end time.Time, step time.Duration) (model.Value, error) {
+	value, _, err := c.querierClient.QueryRange(context.Background(), query, promv1.Range{
+		Start: start,
+		End:   end,
+		Step:  step,
+	})
+	return value, err
+}
+
 func (c *Client) QueryRaw(query string) (*http.Response, []byte, error) {
 	addr := fmt.Sprintf("http://%s/api/prom/api/v1/query?query=%s", c.querierAddress, url.QueryEscape(query))
 

integration/query_frontend_test.go

@@ -175,14 +175,30 @@ func runQueryFrontendTest(t *testing.T, testMissingMetricName bool, setup queryF
 		c, err := e2ecortex.NewClient("", queryFrontend.HTTPEndpoint(), "", "", fmt.Sprintf("user-%d", userID))
 		require.NoError(t, err)
 
-		// No need to repeat this test for each user.
+		// No need to repeat the test on missing metric name for each user.
 		if userID == 0 && testMissingMetricName {
 			res, body, err := c.QueryRaw("{instance=~\"hello.*\"}")
 			require.NoError(t, err)
 			require.Equal(t, 422, res.StatusCode)
 			require.Contains(t, string(body), "query must contain metric name")
 		}
 
+		// No need to repeat the test on start/end time rounding for each user.
+		if userID == 0 {
+			start := time.Unix(1595846748, 806*1e6)
+			end := time.Unix(1595846750, 806*1e6)
+
+			result, err := c.QueryRange("time()", start, end, time.Second)
+			require.NoError(t, err)
+			require.Equal(t, model.ValMatrix, result.Type())
+
+			matrix := result.(model.Matrix)
+			require.Len(t, matrix, 1)
+			require.Len(t, matrix[0].Values, 3)
+			assert.Equal(t, model.Time(1595846748806), matrix[0].Values[0].Timestamp)
+			assert.Equal(t, model.Time(1595846750806), matrix[0].Values[2].Timestamp)
+		}
+
 		for q := 0; q < numQueriesPerUser; q++ {
 			go func() {
 				defer wg.Done()
@@ -197,9 +213,9 @@ func runQueryFrontendTest(t *testing.T, testMissingMetricName bool, setup queryF
 
 	wg.Wait()
 
-	extra := float64(0)
+	extra := float64(1)
 	if testMissingMetricName {
-		extra = 1
+		extra++
 	}
 	require.NoError(t, queryFrontend.WaitSumMetrics(e2e.Equals(numUsers*numQueriesPerUser+extra), "cortex_query_frontend_queries_total"))
 

pkg/util/time.go

@@ -27,6 +27,7 @@ func TimeFromMillis(ms int64) time.Time {
 func ParseTime(s string) (int64, error) {
 	if t, err := strconv.ParseFloat(s, 64); err == nil {
 		s, ns := math.Modf(t)
+		ns = math.Round(ns*1000) / 1000
 		tm := time.Unix(int64(s), int64(ns*float64(time.Second)))
 		return TimeToMillis(tm), nil
 	}

pkg/util/time_test.go

@@ -38,3 +38,53 @@ func TestDurationWithJitter(t *testing.T) {
 func TestDurationWithJitter_ZeroInputDuration(t *testing.T) {
 	assert.Equal(t, time.Duration(0), DurationWithJitter(time.Duration(0), 0.5))
 }
+
+func TestParseTime(t *testing.T) {
+	var tests = []struct {
+		input  string
+		fail   bool
+		result time.Time
+	}{
+		{
+			input: "",
+			fail:  true,
+		}, {
+			input: "abc",
+			fail:  true,
+		}, {
+			input: "30s",
+			fail:  true,
+		}, {
+			input:  "123",
+			result: time.Unix(123, 0),
+		}, {
+			input:  "123.123",
+			result: time.Unix(123, 123000000),
+		}, {
+			input:  "2015-06-03T13:21:58.555Z",
+			result: time.Unix(1433337718, 555*time.Millisecond.Nanoseconds()),
+		}, {
+			input:  "2015-06-03T14:21:58.555+01:00",
+			result: time.Unix(1433337718, 555*time.Millisecond.Nanoseconds()),
+		}, {
+			// Test nanosecond rounding.
+			input:  "2015-06-03T13:21:58.56789Z",
+			result: time.Unix(1433337718, 567*1e6),
+		}, {
+			// Test float rounding.
+			input:  "1543578564.705",
+			result: time.Unix(1543578564, 705*1e6),
+		},
+	}
+
+	for _, test := range tests {
+		ts, err := ParseTime(test.input)
+		if test.fail {
+			require.Error(t, err)
+			continue
+		}
+
+		require.NoError(t, err)
+		assert.Equal(t, TimeToMillis(test.result), ts)
+	}
+}