From cb77e97996fc2c5267c270020f04bbf2606a3a7b Mon Sep 17 00:00:00 2001 From: Marc Sanmiquel Date: Wed, 3 Sep 2025 09:15:06 +0200 Subject: [PATCH] Pyroscope: Fix incorrect rate calculation from flamegraph totals (#110470) * fix(pyroscope): remove incorrect rate calculation from flamegraph totals * update CHANGELOG.md --- CHANGELOG.md | 1 + .../grafana-pyroscope-datasource/query.go | 24 ++++--------------- .../query_test.go | 24 ++++++++----------- 3 files changed, 15 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 596ca47fe22..153314d98d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - **Alerting:** Fix active time intervals when time interval is renamed [#108547](https://github.com/grafana/grafana/pull/108547), [@yuri-tceretian](https://github.com/yuri-tceretian) - **Alerting:** Fix subpath handling in the alerting package [#109505](https://github.com/grafana/grafana/pull/109505), [@konrad147](https://github.com/konrad147) - **Config:** Fix date_formats options being moved to a different section [#109366](https://github.com/grafana/grafana/pull/109366), [@joshhunt](https://github.com/joshhunt) +- **Pyroscope:** Fix flamegraph totals showing incorrect values after rate aggregation changes [#110470](https://github.com/grafana/grafana/pull/110470), [@marcsanmiquel](https://github.com/marcsanmiquel) diff --git a/pkg/tsdb/grafana-pyroscope-datasource/query.go b/pkg/tsdb/grafana-pyroscope-datasource/query.go index 88d1d216956..a9d3449c7ed 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/query.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/query.go @@ -357,15 +357,7 @@ type CustomMeta struct { // dataFrame to again basically walking depth first over the tree/profile. func treeToNestedSetDataFrame(tree *ProfileTree, unit string, stepDurationSec float64, profileTypeID string) *data.Frame { frame := data.NewFrame("response") - frameMeta := &data.FrameMeta{PreferredVisualization: "flamegraph"} - - // Add metadata when rate calculation is applied - if isCumulativeProfile(profileTypeID) && stepDurationSec > 0 { - frameMeta.Custom = map[string]interface{}{ - "rateCalculated": true, - } - } - frame.Meta = frameMeta + frame.Meta = &data.FrameMeta{PreferredVisualization: "flamegraph"} levelField := data.NewField("level", nil, []int64{}) valueField := data.NewField("value", nil, []int64{}) @@ -382,17 +374,9 @@ func treeToNestedSetDataFrame(tree *ProfileTree, unit string, stepDurationSec fl if tree != nil { walkTree(tree, func(tree *ProfileTree) { levelField.Append(int64(tree.Level)) - - // Apply rate calculation for cumulative profiles - value := tree.Value - self := tree.Self - if isCumulativeProfile(profileTypeID) && stepDurationSec > 0 { - value = int64(float64(value) / stepDurationSec) - self = int64(float64(self) / stepDurationSec) - } - - valueField.Append(value) - selfField.Append(self) + // Flamegraphs show cumulative values without rate calculation + valueField.Append(tree.Value) + selfField.Append(tree.Self) labelField.Append(tree.Name) }) } diff --git a/pkg/tsdb/grafana-pyroscope-datasource/query_test.go b/pkg/tsdb/grafana-pyroscope-datasource/query_test.go index 1f1404da99f..2f6d7185d7a 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/query_test.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/query_test.go @@ -229,15 +229,13 @@ func Test_treeToNestedDataFrame(t *testing.T) { require.Equal(t, 0, frame.Fields[0].Len()) }) - t.Run("rateCalculated metadata for cumulative profile", func(t *testing.T) { + t.Run("no rateCalculated metadata for flamegraph", func(t *testing.T) { tree := &ProfileTree{ Value: 100, Level: 0, Self: 1, Name: "root", } frame := treeToNestedSetDataFrame(tree, "short", 15.0, "process_cpu:cpu:nanoseconds:cpu:nanoseconds") require.NotNil(t, frame.Meta) - require.NotNil(t, frame.Meta.Custom) - custom := frame.Meta.Custom.(map[string]interface{}) - require.Equal(t, true, custom["rateCalculated"]) + require.Nil(t, frame.Meta.Custom) }) t.Run("no rateCalculated metadata for instant profile", func(t *testing.T) { @@ -249,26 +247,24 @@ func Test_treeToNestedDataFrame(t *testing.T) { require.Nil(t, frame.Meta.Custom) }) - t.Run("CPU time keeps original units for tree data", func(t *testing.T) { + t.Run("CPU time keeps original values and units for flamegraph", func(t *testing.T) { tree := &ProfileTree{ Value: 3000000000, Level: 0, Self: 1500000000, Name: "root", // 3s total, 1.5s self in nanoseconds } - // Test CPU profile (should keep nanoseconds for flamegraph, no unit conversion) + // Test CPU profile flamegraph - should keep original cumulative values and units frame := treeToNestedSetDataFrame(tree, "ns", 15.0, "process_cpu:cpu:nanoseconds:cpu:nanoseconds") - // Check unit remains as nanoseconds (no conversion for flamegraphs) + // Check unit remains as nanoseconds require.Equal(t, "ns", frame.Fields[1].Config.Unit) require.Equal(t, "ns", frame.Fields[2].Config.Unit) - // Check values were rate calculated but not unit converted: 3000000000/15 = 200000000, 1500000000/15 = 100000000 - require.Equal(t, int64(200000000), frame.Fields[1].At(0)) - require.Equal(t, int64(100000000), frame.Fields[2].At(0)) + // Check values are NOT rate calculated - flamegraphs show cumulative totals + require.Equal(t, int64(3000000000), frame.Fields[1].At(0)) + require.Equal(t, int64(1500000000), frame.Fields[2].At(0)) - // Check metadata shows rate was calculated + // Check metadata shows rate was NOT calculated for flamegraphs require.NotNil(t, frame.Meta) - require.NotNil(t, frame.Meta.Custom) - custom := frame.Meta.Custom.(map[string]interface{}) - require.Equal(t, true, custom["rateCalculated"]) + require.Nil(t, frame.Meta.Custom) }) }