fix(ingest): line numbers for pyspy (#3337)

* fix(ingest): rewrite pprof to include line numbers in function names for pyspy

cmd/pyroscope/frontend.Dockerfile

@@ -0,0 +1,13 @@
+FROM node:18 as builder
+RUN apt-get update && apt-get install -y libpango1.0-dev libcairo2-dev
+WORKDIR /pyroscope
+COPY yarn.lock package.json tsconfig.json ./
+RUN yarn --frozen-lockfile
+COPY scripts/webpack ./scripts/webpack/
+COPY public/app ./public/app
+COPY public/templates ./public/templates
+RUN yarn build
+
+# Usage: docker build  -f cmd/pyroscope/frontend.Dockerfile --output=public/build .
+FROM scratch
+COPY --from=builder /pyroscope/public/build /

pkg/og/convert/pprof/profile.go

@@ -245,26 +245,55 @@ func isScriptingSpy(md ingestion.Metadata) bool {
 	return md.SpyName == "pyspy" || md.SpyName == "rbspy" || md.SpyName == "scripting"
 }
 
+// FixFunctionNamesForScriptingLanguages modifies the function names in the provided profile
+// to include line numbers. This is a workaround for frontend limitations in rendering line numbers.
+// The function is specifically designed for profiles generated by scripting languages.
+// Note: This function modifies the provided profile in place.
 func FixFunctionNamesForScriptingLanguages(p *pprof.Profile, md ingestion.Metadata) {
 	if !needFunctionNameRewrite(md) {
 		return
 	}
 	smap := map[string]int{}
-	for _, fn := range p.Function {
-		// obtaining correct line number will require rewriting functions and slices
-		// lets not do it and wait until we render line numbers on frontend
-		const lineNumber = -1
-		name := fmt.Sprintf("%s:%d - %s",
-			p.StringTable[fn.Filename],
-			lineNumber,
-			p.StringTable[fn.Name])
+	addString := func(name string) int {
 		sid := smap[name]
 		if sid == 0 {
 			sid = len(p.StringTable)
 			p.StringTable = append(p.StringTable, name)
 			smap[name] = sid
 		}
-		fn.Name = int64(sid)
+		return sid
+	}
+	funcId2Index := map[uint64]int64{}
+	newFunctions := map[string]*profilev1.Function{}
+	maxId := uint64(0)
+	for index, fn := range p.Function {
+		funcId2Index[fn.Id] = int64(index)
+		if fn.Id > maxId {
+			maxId = fn.Id
+		}
+	}
+	for _, location := range p.Location {
+		for _, line := range location.Line {
+			fn := p.Function[funcId2Index[line.FunctionId]]
+			name := fmt.Sprintf("%s:%d - %s",
+				p.StringTable[fn.Filename],
+				line.Line,
+				p.StringTable[fn.Name])
+			newFunc, ok := newFunctions[name]
+			if !ok {
+				maxId++
+				newFunc = &profilev1.Function{
+					Id:         maxId,
+					Name:       int64(addString(name)),
+					Filename:   fn.Filename,
+					SystemName: fn.SystemName,
+					StartLine:  fn.StartLine,
+				}
+				newFunctions[name] = newFunc
+				p.Function = append(p.Function, newFunc)
+			}
+			line.FunctionId = newFunc.Id
+		}
 	}
 }
 

pkg/og/convert/pprof/profile_test.go

@@ -1,6 +1,10 @@
 package pprof
 
 import (
+	profilev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
+	"github.com/grafana/pyroscope/pkg/og/convert/pprof/bench"
+	"github.com/grafana/pyroscope/pkg/pprof"
+	"github.com/stretchr/testify/assert"
 	"testing"
 
 	"github.com/grafana/pyroscope/pkg/og/ingestion"
@@ -22,3 +26,56 @@ func TestEmptyPPROF(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 0, len(profile.Series))
 }
+
+func TestFixFunctionNamesForScriptingLanguages(t *testing.T) {
+	profile := pprof.RawFromProto(&profilev1.Profile{
+		StringTable: []string{"", "main", "func1", "func2", "qwe.py"},
+		Function: []*profilev1.Function{
+			{Id: 1, Name: 1, Filename: 4, SystemName: 1, StartLine: 239},
+			{Id: 2, Name: 2, Filename: 4, SystemName: 2, StartLine: 42},
+			{Id: 3, Name: 3, Filename: 4, SystemName: 3, StartLine: 7},
+		},
+		Location: []*profilev1.Location{
+			{Id: 1, Line: []*profilev1.Line{{FunctionId: 1, Line: 242}}},
+			{Id: 2, Line: []*profilev1.Line{{FunctionId: 2, Line: 50}}},
+			{Id: 3, Line: []*profilev1.Line{{FunctionId: 3, Line: 8}}},
+		},
+		Sample: []*profilev1.Sample{
+			{LocationId: []uint64{2, 1}, Value: []int64{10, 1000}},
+			{LocationId: []uint64{3, 1}, Value: []int64{13, 1300}},
+		},
+	})
+	functionNameFromLocation := func(locID uint64) string {
+		for _, loc := range profile.Location {
+			if loc.Id == locID {
+				for _, fun := range profile.Function {
+					if fun.Id == loc.Line[0].FunctionId {
+						return profile.StringTable[fun.Name]
+					}
+				}
+			}
+		}
+		return ""
+	}
+
+	md := ingestion.Metadata{
+		SpyName: "pyspy",
+	}
+
+	FixFunctionNamesForScriptingLanguages(profile, md)
+
+	assert.Len(t, profile.Function, 6) // we do not remove unreferenced functions for now, let the distributor do it
+	assert.Len(t, profile.Location, 3)
+	assert.Len(t, profile.Sample, 2)
+
+	collapsed := bench.StackCollapseProto(profile.Profile, 0, 1)
+	expected := []string{
+		"qwe.py:242 - main;qwe.py:50 - func1 10",
+		"qwe.py:242 - main;qwe.py:8 - func2 13",
+	}
+	assert.Equal(t, expected, collapsed)
+
+	assert.Equal(t, "qwe.py:242 - main", functionNameFromLocation(profile.Location[0].Id))
+	assert.Equal(t, "qwe.py:50 - func1", functionNameFromLocation(profile.Location[1].Id))
+	assert.Equal(t, "qwe.py:8 - func2", functionNameFromLocation(profile.Location[2].Id))
+}

pkg/test/integration/ingest_pprof_test.go

@@ -267,6 +267,57 @@ func TestIngest(t *testing.T) {
 	}
 }
 
+func TestIngestPPROFFixPythonLinenumbers(t *testing.T) {
+	p := PyroscopeTest{}
+	p.Start(t)
+	defer p.Stop(t)
+	profile := pprof.RawFromProto(&profilev1.Profile{
+		SampleType: []*profilev1.ValueType{{
+			Type: 5,
+			Unit: 6,
+		}},
+		PeriodType: &profilev1.ValueType{
+			Type: 5, Unit: 6,
+		},
+		StringTable: []string{"", "main", "func1", "func2", "qwe.py", "cpu", "nanoseconds"},
+		Period:      1000000000,
+		Function: []*profilev1.Function{
+			{Id: 1, Name: 1, Filename: 4, SystemName: 1, StartLine: 239},
+			{Id: 2, Name: 2, Filename: 4, SystemName: 2, StartLine: 42},
+			{Id: 3, Name: 3, Filename: 4, SystemName: 3, StartLine: 7},
+		},
+		Location: []*profilev1.Location{
+			{Id: 1, Line: []*profilev1.Line{{FunctionId: 1, Line: 242}}},
+			{Id: 2, Line: []*profilev1.Line{{FunctionId: 2, Line: 50}}},
+			{Id: 3, Line: []*profilev1.Line{{FunctionId: 3, Line: 8}}},
+		},
+		Sample: []*profilev1.Sample{
+			{LocationId: []uint64{2, 1}, Value: []int64{10}},
+			{LocationId: []uint64{3, 1}, Value: []int64{13}},
+		},
+	})
+
+	tempProfileFile, err := os.CreateTemp("", "profile")
+	require.NoError(t, err)
+	_, err = profile.WriteTo(tempProfileFile)
+	assert.NoError(t, err)
+	tempProfileFile.Close()
+	defer os.Remove(tempProfileFile.Name())
+
+	rb := p.NewRequestBuilder(t).
+		Spy("pyspy")
+	req := rb.IngestPPROFRequest(tempProfileFile.Name(), "", "")
+	p.Ingest(t, req, 200)
+
+	renderedProfile := rb.SelectMergeProfile("process_cpu:cpu:nanoseconds:cpu:nanoseconds", nil)
+	actual := bench.StackCollapseProto(renderedProfile.Msg, 0, 1)
+	expected := []string{
+		"qwe.py:242 - main;qwe.py:50 - func1 10",
+		"qwe.py:242 - main;qwe.py:8 - func2 13",
+	}
+	assert.Equal(t, expected, actual)
+}
+
 func TestPush(t *testing.T) {
 	p := new(PyroscopeTest)
 	p.Start(t)