fix: time series aggregation inconsistency across different time windows

[marcsanmi]

Problem

Time series visualization showed inconsistent values when querying the same workload across different time ranges:

• 1-hour queries: Used 5-second aggregation windows → lower totals
• 12-hour queries: Used 30-second aggregation windows → higher totals

This created misleading graphs where users thought their applications consumed more resources during longer time periods, when it was actually just an aggregation window artifact.

Root Cause

All profile types used simple sum aggregation regardless of their semantic meaning:

• Cumulative profiles (CPU time, allocations) accumulated values over different window sizes without normalization
• Instant profiles (current memory usage, goroutine counts) were summed instead of averaged

Solution

1. Profile Type Classification

Implemented profile type registry following frontend schema:

Cumulative Types (rate normalized):

• cpu, alloc_*, delay, lock_time - accumulated over time

Instant Types:

• Gauges (averaged): inuse_*, goroutine - current state snapshots
• Events (summed): samples, contentions, exceptions - event counts

2. Rate Normalization

Added rateTimeSeriesAggregator that divides cumulative values by step duration:

Solves #3587

[kolesnikovae]

Let's delegate the decision about which aggregation to use to the caller – I believe it should be the client, specifically the frontend. IIRC, we already have some form of ProfilesTypeRegistry there (or at least we used to). Otherwise, the client wouldn't know what units to use or what the numbers represent. We also need to ensure compatibility with existing versions of the UI

[simonswine]

I think this PR is maybe not the right time, but I really would love to centralise this ProfileTypesRegistry in the backend, but until them it makes sense to shift this to the UI

[kolesnikovae]

It's cool if the registry is implemented in the backend and it's managed by the client/frontend – e.g., adding new types, altering existing ones, etc. It just should not be hardcoded in the backend.

[marcsanmi]

I think we're not doing any profile-type-aware aggregation... If I'm not wrong should it happen around here https://github.com/grafana/grafana/blob/ecf793ea05189af1c79bd87bbdba4cb7b5cba93d/pkg/tsdb/grafana-pyroscope-datasource/pyroscopeClient.go#L105?

[simonswine]

Yes you are right we are not passing any profile Type aggregation parameter today in Grafana and Drilldown, but the only way to modify our current behaviour without breaking existing API users is to make the UIs set that parameter, so we know they are new enough that they would also change the units in the Y axis at the same time.

pyroscope/api/querier/v1/querier.proto

Line 162 in 4542317

optional types.v1.TimeSeriesAggregationType aggregation = 7;

[kolesnikovae]

Please note that sum aggregation is enforced in v2 and is not exposed through the query API. The biggest difference is here: we need to move the RangeSeries call to the frontend, somewhere here (rate also needs this), and make sure that we do not call NewTimeSeriesMerger(true) anywhere.

Another issue is that the denominator is not always correct for average. If we count "points" for an aggregate, we count rows in the profile table, not profiles themselves. It will break if a profile has multiple rows: e.g., Go goroutines profile uses avg. aggregation and it may have multiple points per profile (due to pprof labels). I can't see a simple solution to the problem.

I'm not sure if it works as intended in v1, since it's not currently in use. We need to make sure that we take into account that the average operation is not associative.

[kolesnikovae]

I'm not very sure this is what users expect from rate and normalization – in the test I see that reported rate changes depending on the step duration, which does not address the problem:

Time series visualization showed inconsistent values when querying the same workload across different time ranges

By definition, rate measures how a metric is changing over time (ΔV/Δt, and the Δt is fixed to 1s in our case): https://sourcegraph.com/github.com/prometheus/prometheus/-/blob/promql/functions.go?L71-73 is a good example of how we can calculate this

[simonswine]

I am not too sure if we can do this much better, as we don't reliable have the duration (DurationNanos not set for every counter profile) of the profile and we also don't take it into account when selecting the time window.

I think fixing both of that is a massive change, that I don't think I want this tasl to turn into.

Let's say we have, we have 3 pods, two of them are using a scrape interval of 30s and one is using 15s. All 3 are using exactly one core all the time.

With normalising it to the step size, the result will be with step size 15s roughly this:

Pod    | 1 | 2 | 3 |
t+0s   | 0 | 2 | 1 | 
t+15s  | 2 | 0 | 1 |
t+30s  | 0 | 2 | 1 |
t+45s  | 2 | 0 | 1 |

total 3*4*15 = 180

And with step size 30s

Pod    | 1 | 2 | 3 |
t+0s   | 1 | 1 | 1 |
t+30s  | 1 | 1 | 1 |

total 3 * 2 * 30 = 180

I do think this looks very close to the correct data, it obviously is incorrect if you step size gets smaller than your collection window, that also the problem that prometheus has.

[kolesnikovae]

Yeah, I often use this approach to estimate average core usage – for example, if you get an aggregate profile worth 10 hours of CPU time and the query time range is 1 hour, that's essentially 10h/1h = 10 cores on average. The "total" itself isn't the issue. Problems arise when we have gaps in the data – which is common with ingest sampling. Some users also implement their own strategies, like the classic "10s every 60s" sampling.

I'm also curious how this works with very narrow query time ranges, like when a user tries to fetch an individual profile. I might be mistaken, but IIRC we can receive a step as small as 1ms or so.

This approach works in many cases and could already improve the situation. I'd say we should try it out and see – it's likely we'll need to go through a few iterations. However, as you mentioned, the proper solution lies much closer to the data itself, and can't really be implemented without DurationNanos, which is a must for delta profiles (otherwise, we're stuck calculating deltas ourselves).

I also find it interesting that many continuous profiling solutions don't even have a timeline. In many cases, it doesn't make much sense, it's hard to get right, and even harder to build correctly if sampling takes place (and it should take place).

[simonswine]

Fully agree with this we need to deploy this and find out 🙂

Btw: The minimum step size is configured in Grafana data source settings and will 15s by default: https://grafana.com/docs/grafana/latest/datasources/pyroscope/configure-pyroscope-data-source/#querying.

[marcsanmi]

It makes sense. Let's test this out to see how it works and fix this separately #4192. Before that deploying this to dev, I want to address the other comment and remove from this PR the ProfileTypesRegistry in the backend for now.

[marcsanmi]

@kolesnikovae I addressed your suggestions in 9a3ed0f in case you want to take a look. I'll deploy this to dev first to find out any unexpected behaviour.

Edit: I've deploy it to dev, you can check it in ops in the grafanacloud-dev-profiles data source. Right now everything is defaulting to rate aggregation for the sake of testing, but it seems that is consistently calculating the values... (e.g. take a look at the diff view) PLMKWYT 🙂

[kolesnikovae]

I think it improves what we have right now!

But the timeline still looks quirky to me:

Screen.Recording.2025-06-20.at.10.43.51.mov

The diff problem isn't fully resolved yet. We'll likely get better results if both queries share the same step. We could pick the largest, the smallest, something in between, or use a smarter heuristic; it would be cool if users could choose the step. This logic seems best handled in the frontend.

Likewise, our current treatment of "rate" feels like a presentation detail. Storage only sums the data; calculating the rate just means dividing by step, which the frontend can do easily. Moving this to the UI would also let us switch between sum and "rate" views without another fetch.

---

The issue you referenced (#4192) is related in that it also involves DurationNanos, which we need for exporting pprof profiles. However, the scope is quite different and won't help with aggregation. I've explained the context in more detail in this comment – please have a look when you get a chance.

---

Also, please take my comment on the average aggregation seriously – it isn't possible to implement it correctly at the moment; the current implementation will break on profiles that have sample-level (e.g., pprof) labels, such as Go goroutines:

// A single profile containing samples
1 time=1 v=1 {pod="a", endpoint="foo"} // Bock A
2 time=1 v=1 {pod="a", endpoint="bar"} // Bock B
3 time=1 v=1 {pod="a"} // Bock C

// Another profile
4 time=1 v=3 {pod="b"} // Block D