Skip to content

Commit 54d0133

Browse files
authored
profilecli: exclude pre-window boundary point from query top totals (grafana#5359)
SelectSeries fetches one extra step before the window so the boundary point at start renders as a complete bucket in charts; that point aggregates (start-step, start]. query top uses step = window size and summed it, roughly doubling reported totals. Skip points at or before the window start; totals now match SelectMergeProfile exactly.
1 parent 2b10d62 commit 54d0133

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

cmd/profilecli/query-top.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/olekukonko/tablewriter"
1414

1515
querierv1 "github.com/grafana/pyroscope/api/gen/proto/go/querier/v1"
16+
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
1617
"github.com/grafana/pyroscope/v2/pkg/model"
1718
)
1819

@@ -74,11 +75,9 @@ func queryTop(ctx context.Context, params *queryTopParams) error {
7475
}
7576

7677
totals := make([]seriesTotal, 0, len(series))
78+
startMs := from.UnixMilli()
7779
for _, s := range series {
78-
var total float64
79-
for _, p := range s.Points {
80-
total += p.Value
81-
}
80+
total := sumPointsAfter(s.Points, startMs)
8281
lbls := model.Labels(s.Labels)
8382
vals := make([]string, len(params.LabelNames))
8483
for i, name := range params.LabelNames {
@@ -159,6 +158,22 @@ func queryTop(ctx context.Context, params *queryTopParams) error {
159158
return nil
160159
}
161160

161+
// sumPointsAfter sums point values with timestamps strictly after startMs.
162+
// SelectSeries fetches one extra step before the window so that the boundary
163+
// point at `start` renders as a complete bucket in charts; that point
164+
// aggregates (start-step, start], which lies entirely before the requested
165+
// window. With step = window size, counting it roughly doubles the total.
166+
func sumPointsAfter(points []*typesv1.Point, startMs int64) float64 {
167+
var total float64
168+
for _, p := range points {
169+
if p.Timestamp <= startMs {
170+
continue
171+
}
172+
total += p.Value
173+
}
174+
return total
175+
}
176+
162177
func formatUnit(v float64, unit string) string {
163178
switch unit {
164179
case "nanoseconds":

cmd/profilecli/query-top_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
9+
)
10+
11+
func TestSumPointsAfter(t *testing.T) {
12+
const startMs = int64(1_784_130_300_000) // window start
13+
const stepMs = int64(3_600_000)
14+
15+
// SelectSeries with step = window size returns two points: the boundary
16+
// point at `start` holding pre-window data, and the point at `end`
17+
// holding the actual window total.
18+
points := []*typesv1.Point{
19+
{Timestamp: startMs, Value: 8_778_052_796_311},
20+
{Timestamp: startMs + stepMs, Value: 4_005_116_752_500},
21+
}
22+
require.Equal(t, float64(4_005_116_752_500), sumPointsAfter(points, startMs))
23+
24+
t.Run("all points within window", func(t *testing.T) {
25+
points := []*typesv1.Point{
26+
{Timestamp: startMs + 1, Value: 1},
27+
{Timestamp: startMs + 2, Value: 2},
28+
}
29+
require.Equal(t, float64(3), sumPointsAfter(points, startMs))
30+
})
31+
32+
t.Run("no points", func(t *testing.T) {
33+
require.Equal(t, float64(0), sumPointsAfter(nil, startMs))
34+
})
35+
}

0 commit comments

Comments
 (0)