From 5fb1b89678980b275322414e8236f189065919d0 Mon Sep 17 00:00:00 2001 From: David Nesting Date: Mon, 9 Nov 2015 08:36:26 -0500 Subject: [PATCH 1/3] Make metric sort stable and sort by timestamp --- prometheus/registry.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/prometheus/registry.go b/prometheus/registry.go index 5970aae..9000caa 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -32,6 +32,7 @@ import ( "sort" "strings" "sync" + "time" "github.com/golang/protobuf/proto" "github.com/prometheus/common/expfmt" @@ -722,5 +723,26 @@ func (s metricSorter) Less(i, j int) bool { return vi < vj } } - return true + if s[i].GetTimestampMs() != s[j].GetTimestampMs() { + // While this is not strictly supported, until we have + // an import API some people are having some success + // getting data imported by supplying the same metric + // with different timestamps. At the very least we + // shouldn't make things more difficult, so ensure that + // metrics get ordered by timestamp to make it possible + // for Prometheus to import them. + ti, tj := s[i].GetTimestampMs(), s[j].GetTimestampMs() + if ti == 0 { + ti = nowMillis() + } else if tj == 0 { + tj = nowMillis() + } + return ti < tj + } + return false +} + +func nowMillis() int64 { + now := time.Now() + return now.Unix()*1000 + int64(now.Nanosecond())/1000000 } From 544f65f7fc2ab8d5c7b0ec8f3409c8e95efd1868 Mon Sep 17 00:00:00 2001 From: David Nesting Date: Mon, 9 Nov 2015 17:14:07 -0500 Subject: [PATCH 2/3] metricSorter stops relying on time.Now and respects nil timestamps --- prometheus/registry.go | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/prometheus/registry.go b/prometheus/registry.go index 9000caa..65c534a 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -32,7 +32,6 @@ import ( "sort" "strings" "sync" - "time" "github.com/golang/protobuf/proto" "github.com/prometheus/common/expfmt" @@ -723,26 +722,22 @@ func (s metricSorter) Less(i, j int) bool { return vi < vj } } - if s[i].GetTimestampMs() != s[j].GetTimestampMs() { - // While this is not strictly supported, until we have - // an import API some people are having some success - // getting data imported by supplying the same metric - // with different timestamps. At the very least we - // shouldn't make things more difficult, so ensure that - // metrics get ordered by timestamp to make it possible - // for Prometheus to import them. - ti, tj := s[i].GetTimestampMs(), s[j].GetTimestampMs() - if ti == 0 { - ti = nowMillis() - } else if tj == 0 { - tj = nowMillis() - } - return ti < tj + + // While this is not strictly supported, until we have an + // import API some people are having some success getting + // data imported by supplying the same metric with different + // timestamps. At the very least we shouldn't make things + // more difficult, so ensure that metrics get ordered by + // timestamp so that Prometheus doesn't complain about + // out-of-order metrics. A missing timestamp (implies "now") + // will be ordered last. + if s[i].TimestampMs == nil && s[j].TimestampMs != nil { + return false + } else if s[i].TimestampMs != nil && s[j].TimestampMs == nil { + return true + } else if s[i].GetTimestampMs() != s[j].GetTimestampMs() { + return s[i].GetTimestampMs() < s[j].GetTimestampMs() } + return false } - -func nowMillis() int64 { - now := time.Now() - return now.Unix()*1000 + int64(now.Nanosecond())/1000000 -} From b0bd184e74108cc916ff2e35b94fb34e207c8355 Mon Sep 17 00:00:00 2001 From: David Nesting Date: Tue, 10 Nov 2015 09:37:58 -0500 Subject: [PATCH 3/3] Simplify metricSorter timestamp comparison and improve comment --- prometheus/registry.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/prometheus/registry.go b/prometheus/registry.go index 65c534a..7dbbe84 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -723,21 +723,17 @@ func (s metricSorter) Less(i, j int) bool { } } - // While this is not strictly supported, until we have an - // import API some people are having some success getting - // data imported by supplying the same metric with different - // timestamps. At the very least we shouldn't make things - // more difficult, so ensure that metrics get ordered by - // timestamp so that Prometheus doesn't complain about - // out-of-order metrics. A missing timestamp (implies "now") - // will be ordered last. - if s[i].TimestampMs == nil && s[j].TimestampMs != nil { + // We should never arrive here. Multiple metrics with the same + // label set in the same scrape will lead to undefined ingestion + // behavior. However, as above, we have to provide stable sorting + // here, even for inconsistent metrics. So sort equal metrics + // by their timestamp, with missing timestamps (implying "now") + // coming last. + if s[i].TimestampMs == nil { return false - } else if s[i].TimestampMs != nil && s[j].TimestampMs == nil { - return true - } else if s[i].GetTimestampMs() != s[j].GetTimestampMs() { - return s[i].GetTimestampMs() < s[j].GetTimestampMs() } - - return false + if s[j].TimestampMs == nil { + return true + } + return s[i].GetTimestampMs() < s[j].GetTimestampMs() }