client_golang icon indicating copy to clipboard operation
client_golang copied to clipboard

API passes model.Earliest incorrectly

Open bboreham opened this issue 3 years ago • 3 comments

If you pass model.Earliest.Time() to prometheus/client_golang it sends -9223372036854776, which is 192 milliseconds less than the original. This value will cause an overflow when read by prometheus/web/api/v1, turning it into a very large positive time.

I am using github.com/prometheus/client_golang v1.11.0 and github.com/prometheus/client_model v0.2.0.

To illustrate: https://go.dev/play/p/b_75bSN2Gyr

package main

import (
	"fmt"
	"strconv"
	"time"

	"github.com/prometheus/common/model"
)

// As found in github.com/prometheus/common/model
func formatTime(t time.Time) string {
	return strconv.FormatFloat(float64(t.Unix())+float64(t.Nanosecond())/1e9, 'f', -1, 64)
}

// As found in github.com/prometheus/prometheus/model/timestamp
func FromTime(t time.Time) int64 {
	return t.Unix()*1000 + int64(t.Nanosecond())/int64(time.Millisecond)
}

func main() {
	t1 := model.Earliest.Time()
	fmt.Println(formatTime(t1))
	t2 := time.Unix(-9223372036854776, 0)  // this is the number output by the previous line
	fmt.Println(t1, FromTime(t1))
	fmt.Println(t2, FromTime(t2))
}

I believe the problem arises because strconv.FormatFloat() is asked to round the number as if it came from a float64, and model.Earliest is the largest negative int64 so it doesn't fit in a float64, and the milliseconds get dropped.

This arose while trying to work round #621

bboreham avatar Dec 20 '21 16:12 bboreham

Thanks for reporting @bboreham. Help wanted if anyone is interested for a chance to contribute.

kakkoyun avatar Jan 05 '22 10:01 kakkoyun

I couldn't think of a clean and painless way to fix it. Maybe special-case the exact numbers/strings?

Conceptually I'd like model.Earliest to be this value: https://github.com/prometheus/prometheus/blob/d26fd5c97b0c505f5cb36f719abd6ccc8130bdc9/web/api/v1/api.go#L689

but it seems unlikely we can make that change without breaking people. Maybe add that as a new model constant and deprecate Earliest ?

bboreham avatar Jan 05 '22 13:01 bboreham