Skip to content

Commit dad7f49

Browse files
marcsanmimarcsanmi
committed
fix: add read-path stub creation for unsymbolized profiles
1 parent 2af1eea commit dad7f49

File tree

3 files changed

+321
-8
lines changed

3 files changed

+321
-8
lines changed

‎pkg/frontend/readpath/queryfrontend/query_frontend.go‎

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"math/rand"
7+
"path/filepath"
78
"slices"
89
"sync"
910
"time"
@@ -134,6 +135,12 @@ func (q *QueryFrontend) Query(
134135
if err != nil {
135136
return nil, status.Error(codes.Internal, fmt.Sprintf("symbolizing profiles: %v", err))
136137
}
138+
} else if q.hasAnyUnsymbolizedBlocks(blocks) {
139+
// When symbolizer is disabled, create stubs for unsymbolized locations
140+
err = q.createStubsForUnsymbolizedProfiles(ctx, resp, req.Query)
141+
if err != nil {
142+
return nil, status.Error(codes.Internal, fmt.Sprintf("creating stubs: %v", err))
143+
}
137144
}
138145

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

211+
// hasAnyUnsymbolizedBlocks checks if any block has unsymbolized profiles
212+
func (q *QueryFrontend) hasAnyUnsymbolizedBlocks(blocks []*metastorev1.BlockMeta) bool {
213+
for _, block := range blocks {
214+
if q.hasUnsymbolizedProfiles(block) {
215+
return true
216+
}
217+
}
218+
return false
219+
}
220+
204221
// shouldSymbolize determines if we should symbolize profiles based on tenant settings
205222
func (q *QueryFrontend) shouldSymbolize(tenants []string, blocks []*metastorev1.BlockMeta) bool {
206223
if q.symbolizer == nil {
@@ -213,13 +230,7 @@ func (q *QueryFrontend) shouldSymbolize(tenants []string, blocks []*metastorev1.
213230
}
214231
}
215232

216-
for _, block := range blocks {
217-
if q.hasUnsymbolizedProfiles(block) {
218-
return true
219-
}
220-
}
221-
222-
return false
233+
return q.hasAnyUnsymbolizedBlocks(blocks)
223234
}
224235

225236
// processAndSymbolizeProfiles handles the symbolization of profiles from the response
@@ -267,3 +278,82 @@ func (q *QueryFrontend) processAndSymbolizeProfiles(
267278

268279
return nil
269280
}
281+
282+
// createStubsForUnsymbolizedProfiles creates placeholder functions for unsymbolized locations
283+
// when symbolizer is disabled.
284+
func (q *QueryFrontend) createStubsForUnsymbolizedProfiles(
285+
_ context.Context,
286+
resp *queryv1.InvokeResponse,
287+
originalQueries []*queryv1.Query,
288+
) error {
289+
if len(originalQueries) != len(resp.Reports) {
290+
return fmt.Errorf("query/report count mismatch: %d queries but %d reports",
291+
len(originalQueries), len(resp.Reports))
292+
}
293+
294+
for _, r := range resp.Reports {
295+
if r.Pprof == nil || r.Pprof.Pprof == nil {
296+
continue
297+
}
298+
299+
var prof googlev1.Profile
300+
if err := pprof.Unmarshal(r.Pprof.Pprof, &prof); err != nil {
301+
return fmt.Errorf("failed to unmarshal profile: %w", err)
302+
}
303+
304+
stubFuncs := make(map[string]uint64)
305+
modified := false
306+
307+
for _, loc := range prof.Location {
308+
if len(loc.Line) > 0 {
309+
continue
310+
}
311+
312+
if loc.MappingId == 0 || int(loc.MappingId) > len(prof.Mapping) {
313+
continue
314+
}
315+
mapping := prof.Mapping[loc.MappingId-1]
316+
317+
binaryName := "unknown"
318+
if mapping.Filename > 0 && int(mapping.Filename) < len(prof.StringTable) {
319+
filename := prof.StringTable[mapping.Filename]
320+
if len(filename) > 0 {
321+
binaryName = filepath.Base(filename)
322+
}
323+
}
324+
325+
stubName := fmt.Sprintf("%s 0x%x", binaryName, loc.Address)
326+
327+
funcID, exists := stubFuncs[stubName]
328+
if !exists {
329+
stubIdx := int64(len(prof.StringTable))
330+
prof.StringTable = append(prof.StringTable, stubName)
331+
332+
// Create function
333+
funcID = uint64(len(prof.Function) + 1)
334+
prof.Function = append(prof.Function, &googlev1.Function{
335+
Id: funcID,
336+
Name: stubIdx,
337+
})
338+
stubFuncs[stubName] = funcID
339+
}
340+
341+
loc.Line = []*googlev1.Line{{
342+
FunctionId: funcID,
343+
}}
344+
modified = true
345+
}
346+
347+
if !modified {
348+
continue
349+
}
350+
351+
profileBytes, err := pprof.Marshal(&prof, true)
352+
if err != nil {
353+
return fmt.Errorf("failed to marshal profile with stubs: %w", err)
354+
}
355+
r.Pprof.Pprof = profileBytes
356+
}
357+
358+
return nil
359+
}

‎pkg/frontend/readpath/queryfrontend/query_frontend_test.go‎

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
2121
"github.com/grafana/pyroscope/pkg/block/metadata"
2222
"github.com/grafana/pyroscope/pkg/featureflags"
23+
"github.com/grafana/pyroscope/pkg/pprof"
2324
"github.com/grafana/pyroscope/pkg/tenant"
2425
"github.com/grafana/pyroscope/pkg/test/mocks/mockfrontend"
2526
"github.com/grafana/pyroscope/pkg/test/mocks/mockmetastorev1"
@@ -477,3 +478,225 @@ func Test_QueryFrontend_Series_WithLabelNameFiltering(t *testing.T) {
477478
})
478479
}
479480
}
481+
482+
func TestCreateStubsForUnsymbolizedProfiles(t *testing.T) {
483+
tests := []struct {
484+
name string
485+
profile *profilev1.Profile
486+
queries []*queryv1.Query
487+
expectError bool
488+
validateStubs func(t *testing.T, profile *profilev1.Profile)
489+
}{
490+
{
491+
name: "creates stubs for unsymbolized locations",
492+
profile: &profilev1.Profile{
493+
StringTable: []string{"", "/usr/lib/libjvm.so"},
494+
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
495+
Mapping: []*profilev1.Mapping{
496+
{Id: 1, Filename: 1, HasFunctions: false},
497+
},
498+
Location: []*profilev1.Location{
499+
{Id: 1, MappingId: 1, Address: 0xcafebabe, Line: nil},
500+
{Id: 2, MappingId: 1, Address: 0xdeadbeef, Line: nil},
501+
},
502+
Function: []*profilev1.Function{},
503+
Sample: []*profilev1.Sample{
504+
{LocationId: []uint64{1, 2}, Value: []int64{100}},
505+
},
506+
},
507+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
508+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
509+
require.Len(t, profile.Location[0].Line, 1)
510+
require.Len(t, profile.Location[1].Line, 1)
511+
require.Len(t, profile.Function, 2)
512+
func1Name := profile.StringTable[profile.Function[0].Name]
513+
func2Name := profile.StringTable[profile.Function[1].Name]
514+
assert.Equal(t, "libjvm.so 0xcafebabe", func1Name)
515+
assert.Equal(t, "libjvm.so 0xdeadbeef", func2Name)
516+
assert.Equal(t, profile.Function[0].Id, profile.Location[0].Line[0].FunctionId)
517+
assert.Equal(t, profile.Function[1].Id, profile.Location[1].Line[0].FunctionId)
518+
},
519+
},
520+
{
521+
name: "deduplicates stubs by mapping and address combination",
522+
profile: &profilev1.Profile{
523+
StringTable: []string{"", "/lib/libc.so", "/lib/libm.so"},
524+
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
525+
Mapping: []*profilev1.Mapping{
526+
{Id: 1, Filename: 1, HasFunctions: false},
527+
{Id: 2, Filename: 2, HasFunctions: false},
528+
},
529+
Location: []*profilev1.Location{
530+
{Id: 1, MappingId: 1, Address: 0x1234, Line: nil},
531+
{Id: 2, MappingId: 1, Address: 0x5678, Line: nil},
532+
{Id: 3, MappingId: 2, Address: 0x1234, Line: nil},
533+
},
534+
Function: []*profilev1.Function{},
535+
Sample: []*profilev1.Sample{
536+
{LocationId: []uint64{1, 2, 3}, Value: []int64{50}},
537+
},
538+
},
539+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
540+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
541+
require.Len(t, profile.Function, 3)
542+
func1Name := profile.StringTable[profile.Function[0].Name]
543+
func2Name := profile.StringTable[profile.Function[1].Name]
544+
func3Name := profile.StringTable[profile.Function[2].Name]
545+
assert.Equal(t, "libc.so 0x1234", func1Name)
546+
assert.Equal(t, "libc.so 0x5678", func2Name)
547+
assert.Equal(t, "libm.so 0x1234", func3Name)
548+
},
549+
},
550+
{
551+
name: "skips already symbolized locations",
552+
profile: &profilev1.Profile{
553+
StringTable: []string{"", "/usr/bin/app", "symbolized_func"},
554+
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
555+
Mapping: []*profilev1.Mapping{
556+
{Id: 1, Filename: 1, HasFunctions: true},
557+
},
558+
Location: []*profilev1.Location{
559+
{Id: 1, MappingId: 1, Address: 0x1000, Line: []*profilev1.Line{{FunctionId: 1}}},
560+
{Id: 2, MappingId: 1, Address: 0x2000, Line: nil},
561+
},
562+
Function: []*profilev1.Function{
563+
{Id: 1, Name: 2},
564+
},
565+
Sample: []*profilev1.Sample{
566+
{LocationId: []uint64{1, 2}, Value: []int64{25}},
567+
},
568+
},
569+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
570+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
571+
require.Len(t, profile.Location[0].Line, 1)
572+
assert.Equal(t, uint64(1), profile.Location[0].Line[0].FunctionId)
573+
require.Len(t, profile.Location[1].Line, 1)
574+
require.Len(t, profile.Function, 2)
575+
stubFunc := profile.Function[1]
576+
stubName := profile.StringTable[stubFunc.Name]
577+
assert.Equal(t, "app 0x2000", stubName)
578+
},
579+
},
580+
{
581+
name: "handles multiple mappings",
582+
profile: &profilev1.Profile{
583+
StringTable: []string{"", "/lib/liba.so", "/lib/libb.so"},
584+
SampleType: []*profilev1.ValueType{{Type: 1, Unit: 1}},
585+
Mapping: []*profilev1.Mapping{
586+
{Id: 1, Filename: 1, HasFunctions: false},
587+
{Id: 2, Filename: 2, HasFunctions: false},
588+
},
589+
Location: []*profilev1.Location{
590+
{Id: 1, MappingId: 1, Address: 0x100, Line: nil},
591+
{Id: 2, MappingId: 2, Address: 0x200, Line: nil},
592+
},
593+
Function: []*profilev1.Function{},
594+
Sample: []*profilev1.Sample{
595+
{LocationId: []uint64{1, 2}, Value: []int64{10}},
596+
},
597+
},
598+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
599+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
600+
require.Len(t, profile.Function, 2)
601+
func1Name := profile.StringTable[profile.Function[0].Name]
602+
func2Name := profile.StringTable[profile.Function[1].Name]
603+
assert.Equal(t, "liba.so 0x100", func1Name)
604+
assert.Equal(t, "libb.so 0x200", func2Name)
605+
},
606+
},
607+
{
608+
name: "handles unknown binary name",
609+
profile: &profilev1.Profile{
610+
StringTable: []string{""},
611+
SampleType: []*profilev1.ValueType{{Type: 0, Unit: 0}},
612+
Mapping: []*profilev1.Mapping{
613+
{Id: 1, Filename: 0, HasFunctions: false},
614+
},
615+
Location: []*profilev1.Location{
616+
{Id: 1, MappingId: 1, Address: 0xabc, Line: nil},
617+
},
618+
Function: []*profilev1.Function{},
619+
Sample: []*profilev1.Sample{
620+
{LocationId: []uint64{1}, Value: []int64{5}},
621+
},
622+
},
623+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
624+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
625+
require.Len(t, profile.Function, 1)
626+
funcName := profile.StringTable[profile.Function[0].Name]
627+
assert.Equal(t, "unknown 0xabc", funcName)
628+
},
629+
},
630+
{
631+
name: "query/report count mismatch returns error",
632+
profile: &profilev1.Profile{
633+
StringTable: []string{""},
634+
Location: []*profilev1.Location{},
635+
Function: []*profilev1.Function{},
636+
Sample: []*profilev1.Sample{},
637+
},
638+
queries: []*queryv1.Query{
639+
{QueryType: queryv1.QueryType_QUERY_PPROF},
640+
{QueryType: queryv1.QueryType_QUERY_PPROF},
641+
},
642+
expectError: true,
643+
},
644+
{
645+
name: "no changes needed returns early",
646+
profile: &profilev1.Profile{
647+
StringTable: []string{"", "/usr/bin/app", "func1"},
648+
Mapping: []*profilev1.Mapping{
649+
{Id: 1, Filename: 1, HasFunctions: true},
650+
},
651+
Location: []*profilev1.Location{
652+
{Id: 1, MappingId: 1, Address: 0x1000, Line: []*profilev1.Line{{FunctionId: 1}}},
653+
},
654+
Function: []*profilev1.Function{
655+
{Id: 1, Name: 2},
656+
},
657+
Sample: []*profilev1.Sample{
658+
{LocationId: []uint64{1}, Value: []int64{10}},
659+
},
660+
},
661+
queries: []*queryv1.Query{{QueryType: queryv1.QueryType_QUERY_PPROF}},
662+
validateStubs: func(t *testing.T, profile *profilev1.Profile) {
663+
require.Len(t, profile.Function, 1, "should not add functions")
664+
assert.Equal(t, "func1", profile.StringTable[2], "should not modify string table")
665+
},
666+
},
667+
}
668+
669+
for _, tt := range tests {
670+
t.Run(tt.name, func(t *testing.T) {
671+
profileBytes, err := pprof.Marshal(tt.profile, true)
672+
require.NoError(t, err)
673+
674+
resp := &queryv1.InvokeResponse{
675+
Reports: []*queryv1.Report{
676+
{
677+
Pprof: &queryv1.PprofReport{Pprof: profileBytes},
678+
ReportType: queryv1.ReportType_REPORT_PPROF,
679+
},
680+
},
681+
}
682+
683+
qf := &QueryFrontend{}
684+
err = qf.createStubsForUnsymbolizedProfiles(context.Background(), resp, tt.queries)
685+
686+
if tt.expectError {
687+
require.Error(t, err)
688+
return
689+
}
690+
691+
require.NoError(t, err)
692+
693+
var resultProfile profilev1.Profile
694+
err = pprof.Unmarshal(resp.Reports[0].Pprof.Pprof, &resultProfile)
695+
require.NoError(t, err)
696+
697+
if tt.validateStubs != nil {
698+
tt.validateStubs(t, &resultProfile)
699+
}
700+
})
701+
}
702+
}

‎pkg/ingester/otlp/testdata/TestSampleAttributes.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,4 @@
3838
"time_nanos": "239",
3939
"duration_nanos": "10000000000",
4040
"period": "0"
41-
}
41+
}

0 commit comments

Comments
 (0)