Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 102 additions & 7 deletions pkg/frontend/readpath/queryfrontend/query_frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"math/rand"
"path/filepath"
"slices"
"sync"
"time"
Expand Down Expand Up @@ -134,6 +135,12 @@ func (q *QueryFrontend) Query(
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("symbolizing profiles: %v", err))
}
} else if q.hasAnyUnsymbolizedBlocks(blocks) {
// When symbolizer is disabled, create stubs for unsymbolized locations
err = q.createStubsForUnsymbolizedProfiles(ctx, resp, req.Query)
if err != nil {
return nil, status.Error(codes.Internal, fmt.Sprintf("creating stubs: %v", err))
}
}

// TODO(kolesnikovae): Extend diagnostics
Expand Down Expand Up @@ -201,6 +208,16 @@ func (q *QueryFrontend) hasUnsymbolizedProfiles(block *metastorev1.BlockMeta) bo
return len(slices.Collect(metadata.FindDatasets(block, matcher))) > 0
}

// hasAnyUnsymbolizedBlocks checks if any block has unsymbolized profiles
func (q *QueryFrontend) hasAnyUnsymbolizedBlocks(blocks []*metastorev1.BlockMeta) bool {
for _, block := range blocks {
if q.hasUnsymbolizedProfiles(block) {
return true
}
}
return false
}

// shouldSymbolize determines if we should symbolize profiles based on tenant settings
func (q *QueryFrontend) shouldSymbolize(tenants []string, blocks []*metastorev1.BlockMeta) bool {
if q.symbolizer == nil {
Expand All @@ -213,13 +230,7 @@ func (q *QueryFrontend) shouldSymbolize(tenants []string, blocks []*metastorev1.
}
}

for _, block := range blocks {
if q.hasUnsymbolizedProfiles(block) {
return true
}
}

return false
return q.hasAnyUnsymbolizedBlocks(blocks)
}

// processAndSymbolizeProfiles handles the symbolization of profiles from the response
Expand Down Expand Up @@ -267,3 +278,87 @@ func (q *QueryFrontend) processAndSymbolizeProfiles(

return nil
}

// createStubsForUnsymbolizedProfiles creates placeholder functions for unsymbolized locations
// when symbolizer is disabled.
func (q *QueryFrontend) createStubsForUnsymbolizedProfiles(
_ context.Context,
resp *queryv1.InvokeResponse,
originalQueries []*queryv1.Query,
) error {
if len(originalQueries) != len(resp.Reports) {
return fmt.Errorf("query/report count mismatch: %d queries but %d reports",
len(originalQueries), len(resp.Reports))
}

for _, r := range resp.Reports {
if r.Pprof == nil || r.Pprof.Pprof == nil {
continue
}

var prof googlev1.Profile
if err := pprof.Unmarshal(r.Pprof.Pprof, &prof); err != nil {
return fmt.Errorf("failed to unmarshal profile: %w", err)
}

stubFuncs := make(map[string]uint64)
modified := false

for _, loc := range prof.Location {
if len(loc.Line) > 0 {
continue
}

if loc.MappingId == 0 || int(loc.MappingId) > len(prof.Mapping) {
continue
}
mapping := prof.Mapping[loc.MappingId-1]

binaryName := "unknown"
if mapping.Filename > 0 && int(mapping.Filename) < len(prof.StringTable) {
filename := prof.StringTable[mapping.Filename]
if len(filename) > 0 {
binaryName = filepath.Base(filename)
}
}

var stubName string
if loc.Address == 0 {
stubName = binaryName
} else {
stubName = fmt.Sprintf("%s 0x%x", binaryName, loc.Address)
}

funcID, exists := stubFuncs[stubName]
if !exists {
stubIdx := int64(len(prof.StringTable))
prof.StringTable = append(prof.StringTable, stubName)

// Create function
funcID = uint64(len(prof.Function) + 1)
prof.Function = append(prof.Function, &googlev1.Function{
Id: funcID,
Name: stubIdx,
})
stubFuncs[stubName] = funcID
}

loc.Line = []*googlev1.Line{{
FunctionId: funcID,
}}
modified = true
}

if !modified {
continue
}

profileBytes, err := pprof.Marshal(&prof, true)
if err != nil {
return fmt.Errorf("failed to marshal profile with stubs: %w", err)
}
r.Pprof.Pprof = profileBytes
}

return nil
}
246 changes: 246 additions & 0 deletions pkg/frontend/readpath/queryfrontend/query_frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
"github.com/grafana/pyroscope/pkg/block/metadata"
"github.com/grafana/pyroscope/pkg/featureflags"
"github.com/grafana/pyroscope/pkg/pprof"
"github.com/grafana/pyroscope/pkg/tenant"
"github.com/grafana/pyroscope/pkg/test/mocks/mockfrontend"
"github.com/grafana/pyroscope/pkg/test/mocks/mockmetastorev1"
Expand Down Expand Up @@ -477,3 +478,248 @@ func Test_QueryFrontend_Series_WithLabelNameFiltering(t *testing.T) {
})
}
}

func TestCreateStubsForUnsymbolizedProfiles(t *testing.T) {
tests := []struct {
name string
profile *profilev1.Profile
queries []*queryv1.Query
expectError bool
validateStubs func(t *testing.T, profile *profilev1.Profile)
}{
{
name: "creates stubs for unsymbolized locations",
profile: &profilev1.Profile{
StringTable: []string{"", "/usr/lib/libjvm.so"},
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: false},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0xcafebabe, Line: nil},
{Id: 2, MappingId: 1, Address: 0xdeadbeef, Line: nil},
},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1, 2}, Value: []int64{100}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Location[0].Line, 1)
require.Len(t, profile.Location[1].Line, 1)
require.Len(t, profile.Function, 2)
func1Name := profile.StringTable[profile.Function[0].Name]
func2Name := profile.StringTable[profile.Function[1].Name]
assert.Equal(t, "libjvm.so 0xcafebabe", func1Name)
assert.Equal(t, "libjvm.so 0xdeadbeef", func2Name)
assert.Equal(t, profile.Function[0].Id, profile.Location[0].Line[0].FunctionId)
assert.Equal(t, profile.Function[1].Id, profile.Location[1].Line[0].FunctionId)
},
},
{
name: "deduplicates stubs by mapping and address combination",
profile: &profilev1.Profile{
StringTable: []string{"", "/lib/libc.so", "/lib/libm.so"},
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: false},
{Id: 2, Filename: 2, HasFunctions: false},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0x1234, Line: nil},
{Id: 2, MappingId: 1, Address: 0x5678, Line: nil},
{Id: 3, MappingId: 2, Address: 0x1234, Line: nil},
},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1, 2, 3}, Value: []int64{50}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Function, 3)
func1Name := profile.StringTable[profile.Function[0].Name]
func2Name := profile.StringTable[profile.Function[1].Name]
func3Name := profile.StringTable[profile.Function[2].Name]
assert.Equal(t, "libc.so 0x1234", func1Name)
assert.Equal(t, "libc.so 0x5678", func2Name)
assert.Equal(t, "libm.so 0x1234", func3Name)
},
},
{
name: "skips already symbolized locations",
profile: &profilev1.Profile{
StringTable: []string{"", "/usr/bin/app", "symbolized_func"},
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: true},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0x1000, Line: []*profilev1.Line{{FunctionId: 1}}},
{Id: 2, MappingId: 1, Address: 0x2000, Line: nil},
},
Function: []*profilev1.Function{
{Id: 1, Name: 2},
},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1, 2}, Value: []int64{25}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Location[0].Line, 1)
assert.Equal(t, uint64(1), profile.Location[0].Line[0].FunctionId)
require.Len(t, profile.Location[1].Line, 1)
require.Len(t, profile.Function, 2)
stubFunc := profile.Function[1]
stubName := profile.StringTable[stubFunc.Name]
assert.Equal(t, "app 0x2000", stubName)
},
},
{
name: "handles multiple mappings",
profile: &profilev1.Profile{
StringTable: []string{"", "/lib/liba.so", "/lib/libb.so"},
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: false},
{Id: 2, Filename: 2, HasFunctions: false},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0x100, Line: nil},
{Id: 2, MappingId: 2, Address: 0x200, Line: nil},
},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1, 2}, Value: []int64{10}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Function, 2)
func1Name := profile.StringTable[profile.Function[0].Name]
func2Name := profile.StringTable[profile.Function[1].Name]
assert.Equal(t, "liba.so 0x100", func1Name)
assert.Equal(t, "libb.so 0x200", func2Name)
},
},
{
name: "handles unknown binary name",
profile: &profilev1.Profile{
StringTable: []string{""},
SampleType: []*profilev1.ValueType{{Type: 0, Unit: 0}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 0, HasFunctions: false},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0xabc, Line: nil},
},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1}, Value: []int64{5}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Function, 1)
funcName := profile.StringTable[profile.Function[0].Name]
assert.Equal(t, "unknown 0xabc", funcName)
},
},
{
name: "handles zero address without 0x0 suffix",
profile: &profilev1.Profile{
StringTable: []string{"", "/usr/lib/libjvm.so"},
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: false},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0x0, Line: nil},
},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1}, Value: []int64{100}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Function, 1)
funcName := profile.StringTable[profile.Function[0].Name]
assert.Equal(t, "libjvm.so", funcName)
},
},
{
name: "query/report count mismatch returns error",
profile: &profilev1.Profile{
StringTable: []string{""},
Location: []*profilev1.Location{},
Function: []*profilev1.Function{},
Sample: []*profilev1.Sample{},
},
queries: []*queryv1.Query{
{QueryType: queryv1.QueryType_QUERY_PPROF},
{QueryType: queryv1.QueryType_QUERY_PPROF},
},
expectError: true,
},
{
name: "no changes needed returns early",
profile: &profilev1.Profile{
StringTable: []string{"", "/usr/bin/app", "func1"},
Mapping: []*profilev1.Mapping{
{Id: 1, Filename: 1, HasFunctions: true},
},
Location: []*profilev1.Location{
{Id: 1, MappingId: 1, Address: 0x1000, Line: []*profilev1.Line{{FunctionId: 1}}},
},
Function: []*profilev1.Function{
{Id: 1, Name: 2},
},
Sample: []*profilev1.Sample{
{LocationId: []uint64{1}, Value: []int64{10}},
},
},
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
require.Len(t, profile.Function, 1, "should not add functions")
assert.Equal(t, "func1", profile.StringTable[2], "should not modify string table")
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
profileBytes, err := pprof.Marshal(tt.profile, true)
require.NoError(t, err)

resp := &queryv1.InvokeResponse{
Reports: []*queryv1.Report{
{
Pprof: &queryv1.PprofReport{Pprof: profileBytes},
ReportType: queryv1.ReportType_REPORT_PPROF,
},
},
}

qf := &QueryFrontend{}
err = qf.createStubsForUnsymbolizedProfiles(context.Background(), resp, tt.queries)

if tt.expectError {
require.Error(t, err)
return
}

require.NoError(t, err)

var resultProfile profilev1.Profile
err = pprof.Unmarshal(resp.Reports[0].Pprof.Pprof, &resultProfile)
require.NoError(t, err)

if tt.validateStubs != nil {
tt.validateStubs(t, &resultProfile)
}
})
}
}
Loading