Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid timeseries aggregation for range_query #4939

Closed
dernasherbrezon opened this Issue Dec 1, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@dernasherbrezon
Copy link

dernasherbrezon commented Dec 1, 2018

Bug Report

When I execute range_query I'm getting 1millisecond mismatch in the response.

curl "http://0.0.0.0:9090/api/v1/query_range?query=scrape_duration_seconds&start=1543578564.705&end=1543664964.705&step=300"
{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"scrape_duration_seconds","instance":"localhost:9090","job":"prometheus"},"values":[[1543603764.704,"0.008642759"],[1543604064.704,"0.02045981"],[1543604364.704,"0.015498824"]

Start is 1543578564.705. Values in the result are xxxx.704. This makes impossible to generate gaps in the sparse data.

curl "http://0.0.0.0:9090/api/v1/query_range?query=scrape_duration_seconds&start=1543578564.705&end=1543664964.705&step=300"
{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"scrape_duration_seconds","instance":"localhost:9090","job":"prometheus"},"values":[[1543603764.705,"0.008642759"],[1543604064.705,"0.02045981"],[1543604364.705,"0.015498824"]

I guess double is incorrectly rounded somewhere inside.

  • System information:

    Darwin 18.2.0 x86_64

  • Prometheus version:

    prometheus, version 2.5.0 (branch: HEAD, revision: 67dc912)
    build user: root@578ab108d0b9
    build date: 20181106-11:45:24
    go version: go1.11.1

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Dec 3, 2018

In the API code we do:

prometheus/web/api/v1/api.go

Lines 1150 to 1159 in 8b91d39

func parseTime(s string) (time.Time, error) {
if t, err := strconv.ParseFloat(s, 64); err == nil {
s, ns := math.Modf(t)
return time.Unix(int64(s), int64(ns*float64(time.Second))), nil
}
if t, err := time.Parse(time.RFC3339Nano, s); err == nil {
return t, nil
}
return time.Time{}, fmt.Errorf("cannot parse %q to a valid timestamp", s)
}

This takes the string 1543578564.705 and makes the time.Time 2018-11-30 11:49:24.704999923 +0000 UTC out of it. Then later, in the query layer, we convert that time.Time into an int64 TSDB timestamp by simply cutting off any sub-milliseconds digits: https://github.com/prometheus/prometheus/blob/master/pkg/timestamp/timestamp.go#L19-L21

Should we round there instead? Would that be sufficient for covering all reasonable conversion cases? Is this accuracy something we should guarantee?

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Dec 3, 2018

Minimal reproduction case: https://play.golang.org/p/rhOD8UUdkwY

juliusv added a commit that referenced this issue Dec 3, 2018

Better rounding for incoming query timestamps
Fixes #4939

Signed-off-by: Julius Volz <julius.volz@gmail.com>
@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Dec 3, 2018

Here's a suggestion for how to solve this on the API layer (should cover the relevant cases): #4941

@juliusv juliusv closed this in #4941 Dec 3, 2018

juliusv added a commit that referenced this issue Dec 3, 2018

Better rounding for incoming query timestamps (#4941)
Fixes #4939

Signed-off-by: Julius Volz <julius.volz@gmail.com>
@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Dec 3, 2018

@dernasherbrezon Fix is merged. Btw., even before a new version is released, you could try using an rfc3339 timestamp like 2015-07-01T20:10:51.781Z instead of a float, if you control the thing that queries Prometheus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.