Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Commit

Permalink
Fix for min/max start/end timestamp values for remote read
Browse files Browse the repository at this point in the history
When start/end timestamp values are not sent to a query, Prometheus sets them
min/max timestamp values. These values are outside of Prometheus date/time
ranges and cause errors when used in SQL queries. This fix limits those values
to year 1970 for min and year 3000 for max.
  • Loading branch information
antekresic committed Aug 4, 2020
1 parent c8777f8 commit a9c6b1b
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 19 deletions.
24 changes: 19 additions & 5 deletions pkg/api/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@ package api

import (
"encoding/json"
"github.com/pkg/errors"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/promql/parser"
"github.com/prometheus/prometheus/storage"
"github.com/timescale/timescale-prometheus/pkg/promql"
"fmt"
"io"
"math"
"net/http"
"strconv"
"time"

"github.com/pkg/errors"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/promql/parser"
"github.com/prometheus/prometheus/storage"
"github.com/timescale/timescale-prometheus/pkg/promql"
)

var (
Expand Down Expand Up @@ -101,6 +103,18 @@ func marshalMatrixResponse(writer io.Writer, data promql.Matrix, warnings []stri
return out.err
}

func parseTimeParam(r *http.Request, paramName string, defaultValue time.Time) (time.Time, error) {
val := r.FormValue(paramName)
if val == "" {
return defaultValue, nil
}
result, err := parseTime(val)
if err != nil {
return time.Time{}, fmt.Errorf("Invalid time value for '%s': %w", paramName, err)
}
return result, nil
}

func parseTime(s string) (time.Time, error) {
if t, err := strconv.ParseFloat(s, 64); err == nil {
s, ns := math.Modf(t)
Expand Down
16 changes: 6 additions & 10 deletions pkg/api/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,12 @@ import (
func Query(queryEngine *promql.Engine, queryable *query.Queryable) http.Handler {
hf := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var ts time.Time
if t := r.FormValue("time"); t != "" {
var err error
ts, err = parseTime(t)
if err != nil {
log.Error("msg", "Query error", "err", err.Error())
respondError(w, http.StatusBadRequest, err, "bad_data")
return
}
} else {
ts = time.Now()
var err error
ts, err = parseTimeParam(r, "time", time.Now())
if err != nil {
log.Error("msg", "Query error", "err", err.Error())
respondError(w, http.StatusBadRequest, err, "bad_data")
return
}

ctx := r.Context()
Expand Down
9 changes: 5 additions & 4 deletions pkg/api/series.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package api

import (
"encoding/json"
"net/http"
"strings"

"github.com/NYTimes/gziphandler"
"github.com/pkg/errors"
"github.com/prometheus/prometheus/pkg/labels"
Expand All @@ -11,8 +14,6 @@ import (
"github.com/timescale/timescale-prometheus/pkg/log"
"github.com/timescale/timescale-prometheus/pkg/promql"
"github.com/timescale/timescale-prometheus/pkg/query"
"net/http"
"strings"
)

func Series(queryable *query.Queryable) http.Handler {
Expand All @@ -27,13 +28,13 @@ func Series(queryable *query.Queryable) http.Handler {
return
}

start, err := parseTime(r.FormValue("start"))
start, err := parseTimeParam(r, "start", minTime)
if err != nil {
log.Info("msg", "Query bad request:"+err.Error())
respondError(w, http.StatusBadRequest, err, "bad_data")
return
}
end, err := parseTime(r.FormValue("end"))
end, err := parseTimeParam(r, "end", maxTime)
if err != nil {
log.Info("msg", "Query bad request:"+err.Error())
respondError(w, http.StatusBadRequest, err, "bad_data")
Expand Down
27 changes: 27 additions & 0 deletions pkg/pgmodel/end_to_end_tests/promql_endpoint_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,27 @@ func genSeriesRequest(apiURL string, matchers []string, start, end time.Time) (*
)
}

func genSeriesNoTimeRequest(apiURL string, matchers []string) (*http.Request, error) {
u, err := url.Parse(fmt.Sprintf("%s/series", apiURL))

if err != nil {
return nil, err
}

val := url.Values{}

for _, m := range matchers {
val.Add("match[]", m)
}
u.RawQuery = val.Encode()

return http.NewRequest(
"GET",
u.String(),
nil,
)
}

type samples []sample

func (s samples) Len() int { return len(s) }
Expand Down Expand Up @@ -487,6 +508,12 @@ func TestPromQLSeriesEndpoint(t *testing.T) {
testMethod := testRequest(req, series, client, seriesResultComparator)
tester.Run(fmt.Sprintf("%s (instant query)", c.name), testMethod)

req, err = genSeriesNoTimeRequest(apiURL, c.matchers)
if err != nil {
t.Fatalf("unable to create PromQL query request: %s", err)
}
testMethod = testRequest(req, series, client, seriesResultComparator)
tester.Run(fmt.Sprintf("%s (instant query)", c.name), testMethod)
}
})
}
Expand Down
61 changes: 61 additions & 0 deletions pkg/pgmodel/end_to_end_tests/query_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"fmt"
"io/ioutil"
"math"
"net/http"
"net/url"
"reflect"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/jackc/pgx/v4/pgxpool"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/pkg/labels"
"github.com/prometheus/prometheus/pkg/timestamp"
"github.com/prometheus/prometheus/tsdb"
"github.com/prometheus/prometheus/util/testutil"
"github.com/timescale/timescale-prometheus/pkg/internal/testhelpers"
Expand Down Expand Up @@ -519,6 +521,65 @@ func TestSQLQuery(t *testing.T) {
}),
},
},
// https://github.com/timescale/timescale-prometheus/issues/125
{
name: "min start and max end timestamps",
readRequest: prompb.ReadRequest{
Queries: []*prompb.Query{
{
Matchers: []*prompb.LabelMatcher{
{
Type: prompb.LabelMatcher_RE,
Name: "empty",
Value: ".*",
},
{
Type: prompb.LabelMatcher_EQ,
Name: "common",
Value: "tag",
},
},
// Prometheus setting start and end timestamp to min/max values when missing:
// https://github.com/prometheus/prometheus/blob/master/web/api/v1/api.go#L555-L556
StartTimestampMs: timestamp.FromTime(time.Unix(math.MinInt64/1000+62135596801, 0).UTC()),
EndTimestampMs: timestamp.FromTime(time.Unix(math.MaxInt64/1000-62135596801, 999999999).UTC()),
},
},
},
expectResponse: prompb.ReadResponse{
Results: createQueryResult([]*prompb.TimeSeries{
{
Labels: []prompb.Label{
{Name: MetricNameLabelName, Value: "firstMetric"},
{Name: "common", Value: "tag"},
{Name: "empty", Value: ""},
{Name: "foo", Value: "bar"},
},
Samples: []prompb.Sample{
{Timestamp: 1, Value: 0.1},
{Timestamp: 2, Value: 0.2},
{Timestamp: 3, Value: 0.3},
{Timestamp: 4, Value: 0.4},
{Timestamp: 5, Value: 0.5},
},
},
{
Labels: []prompb.Label{
{Name: MetricNameLabelName, Value: "secondMetric"},
{Name: "common", Value: "tag"},
{Name: "foo", Value: "baz"},
},
Samples: []prompb.Sample{
{Timestamp: 1, Value: 1.1},
{Timestamp: 2, Value: 1.2},
{Timestamp: 3, Value: 1.3},
{Timestamp: 4, Value: 1.4},
{Timestamp: 5, Value: 1.5},
},
},
}),
},
},
}

withDB(t, *testDatabase, func(db *pgxpool.Pool, t testing.TB) {
Expand Down
13 changes: 13 additions & 0 deletions pkg/pgmodel/query_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ package pgmodel

import (
"fmt"
"math"
"sort"
"strings"
"time"

"github.com/jackc/pgx/v4"
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/pkg/labels"
"github.com/prometheus/prometheus/pkg/timestamp"
"github.com/prometheus/prometheus/promql/parser"
"github.com/prometheus/prometheus/storage"
"github.com/timescale/timescale-prometheus/pkg/prompb"
Expand Down Expand Up @@ -55,6 +57,11 @@ const (
GROUP BY s.id`
)

var (
minTime = timestamp.FromTime(time.Unix(math.MinInt64/1000+62135596801, 0).UTC())
maxTime = timestamp.FromTime(time.Unix(math.MaxInt64/1000-62135596801, 999999999).UTC())
)

func buildSubQueries(matchers []*labels.Matcher) (string, []string, []interface{}, error) {
var err error
metric := ""
Expand Down Expand Up @@ -390,6 +397,12 @@ func toMilis(t time.Time) int64 {
}

func toRFC3339Nano(milliseconds int64) string {
if milliseconds == minTime {
return "-Infinity"
}
if milliseconds == maxTime {
return "Infinity"
}
sec := milliseconds / 1000
nsec := (milliseconds - (sec * 1000)) * 1000000
return time.Unix(sec, nsec).UTC().Format(time.RFC3339Nano)
Expand Down

0 comments on commit a9c6b1b

Please sign in to comment.