diff --git a/app/vmselect/promql/transform.go b/app/vmselect/promql/transform.go index bcebe46dd..7cb7766b3 100644 --- a/app/vmselect/promql/transform.go +++ b/app/vmselect/promql/transform.go @@ -548,33 +548,32 @@ func vmrangeBucketsToLE(tss []*timeseries) []*timeseries { sort.Slice(xss, func(i, j int) bool { return xss[i].end < xss[j].end }) xssNew := make([]x, 0, len(xss)+2) var xsPrev x - hasNonEmpty := false uniqTs := make(map[string]*timeseries, len(xss)) for _, xs := range xss { ts := xs.ts if isZeroTS(ts) { - xsPrev = xs + // Skip buckets with zero values - they will be merged into a single bucket + // when the next non-zero bucket appears. - if uniqTs[xs.endStr] == nil { - uniqTs[xs.endStr] = xs.ts - xssNew = append(xssNew, x{ - endStr: xs.endStr, - end: xs.end, - ts: copyTS(ts, xs.endStr), - }) - } + // Do not store xs in xsPrev in order to properly create `le` time series + // for zero buckets. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4021 continue } - - hasNonEmpty = true - if xs.start != xsPrev.end && uniqTs[xs.startStr] == nil { - uniqTs[xs.startStr] = xs.ts - xssNew = append(xssNew, x{ - endStr: xs.startStr, - end: xs.start, - ts: copyTS(ts, xs.startStr), - }) + if xs.start != xsPrev.end { + // There is a gap between the previous bucket and the current bucket + // or the previous bucket is skipped because it was zero. + // Fill it with a time series with le=xs.start. + if uniqTs[xs.startStr] == nil { + uniqTs[xs.startStr] = xs.ts + xssNew = append(xssNew, x{ + endStr: xs.startStr, + end: xs.start, + ts: copyTS(ts, xs.startStr), + }) + } } + // Convert the current time series to a time series with le=xs.end ts.MetricName.AddTag("le", xs.endStr) prevTs := uniqTs[xs.endStr] if prevTs != nil { @@ -586,13 +585,7 @@ func vmrangeBucketsToLE(tss []*timeseries) []*timeseries { } xsPrev = xs } - - if !hasNonEmpty { - xssNew = []x{} - continue - } - - if !math.IsInf(xsPrev.end, 1) && !isZeroTS(xsPrev.ts) { + if xsPrev.ts != nil && !math.IsInf(xsPrev.end, 1) && !isZeroTS(xsPrev.ts) { xssNew = append(xssNew, x{ endStr: "+Inf", end: math.Inf(1), diff --git a/app/vmselect/promql/transform_test.go b/app/vmselect/promql/transform_test.go index 0261353ce..6a4b29d01 100644 --- a/app/vmselect/promql/transform_test.go +++ b/app/vmselect/promql/transform_test.go @@ -87,6 +87,27 @@ foo{le="8.799e+05"} 5 123 foo{le="+Inf"} 5 123`, ) + // Multiple adjacent empty vmrange bucket + f( + `foo{vmrange="7.743e+05...8.799e+05"} 5 123 +foo{vmrange="6.813e+05...7.743e+05"} 0 123 +foo{vmrange="5.813e+05...6.813e+05"} 0 123 +`, + `foo{le="7.743e+05"} 0 123 +foo{le="8.799e+05"} 5 123 +foo{le="+Inf"} 5 123`, + ) + f( + `foo{vmrange="8.799e+05...9.813e+05"} 0 123 +foo{vmrange="7.743e+05...8.799e+05"} 5 123 +foo{vmrange="6.813e+05...7.743e+05"} 0 123 +foo{vmrange="5.813e+05...6.813e+05"} 0 123 +`, + `foo{le="7.743e+05"} 0 123 +foo{le="8.799e+05"} 5 123 +foo{le="+Inf"} 5 123`, + ) + // Multiple non-empty vmrange buckets f( `foo{vmrange="4.084e+02...4.642e+02"} 2 123 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 60b1fb48b..98b269721 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -15,6 +15,8 @@ The following tip changes can be tested by building VictoriaMetrics components f ## v1.87.x long-time support release (LTS) +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly convert [VictoriaMetrics historgram buckets](https://valyala.medium.com/improving-histogram-usability-for-prometheus-and-grafana-bc7e5df0e350) to Prometheus histogram buckets when VictoriaMetrics histogram contain zero buckets. Previously these buckets were ignored, and this could lead to missing Prometheus histogram buckets after the conversion. Thanks to @zklapow for [the fix](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4021). + ## [v1.87.4](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.87.4) Released at 2023-03-25