app/vmselect/promql: follow-up for 79e1c6a6fc

- Document the fix at docs/CHANGELOG.md
- Add tests with multiple adjancent zero buckets
- Simplify the fix a bit

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/296
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4021

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),

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

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