diff --git a/pkg/query/pprof.go b/pkg/query/pprof.go index c23816122de..7824badf1dc 100644 --- a/pkg/query/pprof.go +++ b/pkg/query/pprof.go @@ -252,12 +252,21 @@ func (w *PprofWriter) mapping( return 0 } + filename := int64(0) + if !r.MappingFileIndices.IsNull(j) { + filename = t.mappingFile(r.MappingFileIndices.Value(j)) + } + buildID := int64(0) + if !r.MappingBuildIDIndices.IsNull(j) { + buildID = t.mappingBuildID(r.MappingBuildIDIndices.Value(j)) + } + m := &pprofpb.Mapping{ MemoryStart: r.MappingStart.Value(j), MemoryLimit: r.MappingLimit.Value(j), FileOffset: r.MappingOffset.Value(j), - Filename: t.mappingFile(r.MappingFileIndices.Value(j)), - BuildId: t.mappingBuildID(r.MappingBuildIDIndices.Value(j)), + Filename: filename, + BuildId: buildID, HasFunctions: true, } @@ -291,6 +300,9 @@ func (w *PprofWriter) location( for k := int(lineStart); k < int(lineEnd); k++ { if r.Line.IsValid(k) { functionId := w.function(r, t, k) + if functionId == 0 { + continue + } loc.Line = append(loc.Line, &pprofpb.Line{ FunctionId: functionId, Line: r.LineNumber.Value(k), @@ -317,15 +329,34 @@ func (w *PprofWriter) function( t *pprofTranspositions, k int, ) uint64 { - if r.LineFunctionNameIndices.IsNull(k) { + nameNull := r.LineFunctionNameIndices.IsNull(k) + systemNameNull := r.LineFunctionSystemNameIndices.IsNull(k) + filenameNull := r.LineFunctionFilenameIndices.IsNull(k) + if nameNull && systemNameNull && filenameNull && r.LineFunctionStartLine.IsNull(k) { return 0 } + name := int64(0) + if !nameNull { + name = t.functionName(r.LineFunctionNameIndices.Value(k)) + } + systemName := int64(0) + if !systemNameNull { + systemName = t.functionSystemName(r.LineFunctionSystemNameIndices.Value(k)) + } + filename := int64(0) + if !filenameNull { + filename = t.functionFilename(r.LineFunctionFilenameIndices.Value(k)) + } + startLine := int64(0) + if !r.LineFunctionStartLine.IsNull(k) { + startLine = r.LineFunctionStartLine.Value(k) + } f := &pprofpb.Function{ - Name: t.functionName(r.LineFunctionNameIndices.Value(k)), - SystemName: t.functionSystemName(r.LineFunctionSystemNameIndices.Value(k)), - Filename: t.functionFilename(r.LineFunctionFilenameIndices.Value(k)), - StartLine: r.LineFunctionStartLine.Value(k), + Name: name, + SystemName: systemName, + Filename: filename, + StartLine: startLine, } key := makeFunctionKey(f) diff --git a/pkg/query/pprof_test.go b/pkg/query/pprof_test.go index fb353e11267..0dd1d46f6af 100644 --- a/pkg/query/pprof_test.go +++ b/pkg/query/pprof_test.go @@ -20,6 +20,9 @@ import ( "testing" "time" + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/memory" pprofprofile "github.com/google/pprof/profile" "github.com/stretchr/testify/require" @@ -169,3 +172,117 @@ func TestGeneratePprofNilMapping(t *testing.T) { require.NoError(t, f.Close()) require.NoError(t, resProf.CheckValid()) } + +// Null metadata must remain absent even when another sample populates the +// dictionary. Reading its index can either panic or copy another frame's data. +func TestGeneratePprofNullMetadata(t *testing.T) { + for _, field := range []string{"mapping_file", "mapping_build_id", "function_name", "function_system_name", "function_filename", "function"} { + for _, populated := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/populated=%t", field, populated), func(t *testing.T) { + mem := memory.NewCheckedAllocator(memory.DefaultAllocator) + defer mem.AssertSize(t, 0) + w := profile.NewWriter(mem, []string{"service"}) + defer w.Release() + appendSample := func(address uint64, missing string) { + w.LocationsList.Append(true) + w.Locations.Append(true) + w.Addresses.Append(address) + w.MappingStart.Append(0x1000) + w.MappingLimit.Append(0x2000) + w.MappingOffset.Append(0) + w.Lines.Append(true) + w.Line.Append(true) + w.LineNumber.Append(42) + w.ColumnNumber.Append(3) + if missing == "function" { + w.FunctionStartLine.AppendNull() + } else { + w.FunctionStartLine.Append(40) + } + for _, attr := range []struct { + name, value string + builder *array.BinaryDictionaryBuilder + }{ + {"mapping_file", "app", w.MappingFile}, + {"mapping_build_id", "build", w.MappingBuildID}, + {"function_name", "main.work", w.FunctionName}, + {"function_system_name", "main.work", w.FunctionSystemName}, + {"function_filename", "main.go", w.FunctionFilename}, + } { + if missing == attr.name || (missing == "function" && (attr.name == "function_name" || attr.name == "function_system_name" || attr.name == "function_filename")) { + attr.builder.AppendNull() + } else { + require.NoError(t, attr.builder.Append([]byte(attr.value))) + } + } + require.NoError(t, w.LabelBuilders[0].Append([]byte("checkout"))) + w.Value.Append(13) + w.Diff.Append(0) + w.TimeNanos.Append(1) + w.Period.Append(1) + } + if populated { + appendSample(0x1010, "") + } + appendSample(0x1020, field) + rec := w.RecordBuilder.NewRecordBatch() + defer rec.Release() + out, err := GenerateFlatPprof(context.Background(), false, profile.Profile{ + Meta: profile.Meta{SampleType: profile.ValueType{Type: "cpu", Unit: "nanoseconds"}}, + Samples: []arrow.RecordBatch{rec}, + }) + require.NoError(t, err) + raw, err := SerializePprof(out) + require.NoError(t, err) + parsed, err := pprofprofile.ParseData(raw) + require.NoError(t, err) + count := 1 + if populated { + count++ + } + require.Len(t, parsed.Sample, count) + for _, sample := range parsed.Sample { + require.Equal(t, []int64{13}, sample.Value) + require.Equal(t, []string{"checkout"}, sample.Label["service"]) + require.Len(t, sample.Location, 1) + loc := sample.Location[0] + require.Contains(t, []uint64{0x1010, 0x1020}, loc.Address) + require.NotNil(t, loc.Mapping) + wantFile, wantBuild := "app", "build" + if loc.Address == 0x1020 && field == "mapping_file" && !populated { + wantFile = "" + } + if loc.Address == 0x1020 && field == "mapping_build_id" { + wantBuild = "" + } + require.Equal(t, wantFile, loc.Mapping.File) + require.Equal(t, wantBuild, loc.Mapping.BuildID) + if loc.Address == 0x1020 && field == "function" { + require.Empty(t, loc.Line) + continue + } + require.Len(t, loc.Line, 1) + require.Equal(t, int64(42), loc.Line[0].Line) + require.Equal(t, int64(3), loc.Line[0].Column) + fn := loc.Line[0].Function + require.NotNil(t, fn) + wantName, wantSystem, wantSource := "main.work", "main.work", "main.go" + if loc.Address == 0x1020 { + switch field { + case "function_name": + wantName = "" + case "function_system_name": + wantSystem = "" + case "function_filename": + wantSource = "" + } + } + require.Equal(t, wantName, fn.Name) + require.Equal(t, wantSystem, fn.SystemName) + require.Equal(t, wantSource, fn.Filename) + require.Equal(t, int64(40), fn.StartLine) + } + }) + } + } +}