chore: move symbolizatoin into query frontend & add intermediate representation for lookups in dwarf

Author: marcsanmi

cmd/symbolization/main.go

@@ -5,93 +5,124 @@ import (
 	"fmt"
 	"log"
 
-	pprof "github.com/google/pprof/profile"
-
+	googlev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
+	queryv1 "github.com/grafana/pyroscope/api/gen/proto/go/query/v1"
 	"github.com/grafana/pyroscope/pkg/experiment/symbolizer"
 )
 
 const (
 	debuginfodBaseURL = "https://debuginfod.elfutils.org"
-	buildID           = "2fa2055ef20fabc972d5751147e093275514b142"
+	// buildID           = "c047672cae7964324658491e7dee26748ae5d2f8"
+	buildID = "2fa2055ef20fabc972d5751147e093275514b142"
 )
 
 func main() {
-	client := symbolizer.NewDebuginfodClient(debuginfodBaseURL, nil)
+	client := symbolizer.NewDebuginfodClient(debuginfodBaseURL, symbolizer.NewMetrics(nil))
 
 	// Alternatively, use a local debug info file:
 	//client := &localDebuginfodClient{debugFilePath: "/path/to/your/debug/file"}
 
-	s := symbolizer.NewSymbolizer(client, nil, nil)
+	s := symbolizer.NewProfileSymbolizer(client, nil, symbolizer.NewMetrics(nil))
 	ctx := context.Background()
 
 	_, err := client.FetchDebuginfo(buildID)
 	if err != nil {
 		log.Fatalf("Failed to fetch debug info: %v", err)
 	}
-	//defer os.Remove(debugFilePath)
 
-	// Create a request to symbolize specific addresses
-	req := symbolizer.Request{
-		BuildID: buildID,
-		Mappings: []symbolizer.RequestMapping{
+	// {
+	// 	Address: 0x1500,
+	// 	Mapping: &pprof.Mapping{},
+	// },
+	// {
+	// 	Address: 0x3c5a,
+	// 	Mapping: &pprof.Mapping{},
+	// },
+	// {
+	// 	Address: 0x2745,
+	// 	Mapping: &pprof.Mapping{},
+	// },
+
+	// Create a profile with the address we want to symbolize
+	// c047672cae7964324658491e7dee26748ae5d2f8
+	// profile := &googlev1.Profile{
+	// 	Mapping: []*googlev1.Mapping{{
+	// 		BuildId:     1,
+	// 		MemoryStart: 0x0,
+	// 		MemoryLimit: 0x1000000,
+	// 		FileOffset:  0x0,
+	// 	}},
+	// 	Location: []*googlev1.Location{{
+	// 		MappingId: 1,
+	// 		Address:   0x11a230,
+	// 	}},
+	// 	StringTable: []string{"", buildID},
+	// }
+
+	profile := &googlev1.Profile{
+		Mapping: []*googlev1.Mapping{{
+			BuildId:     1,
+			MemoryStart: 0x0,
+			MemoryLimit: 0x1000000,
+			FileOffset:  0x0,
+		}},
+		Location: []*googlev1.Location{
+			{
+				MappingId: 1,
+				Address:   0x1500,
+			},
+			{
+				MappingId: 1,
+				Address:   0x3c5a,
+			},
 			{
-				Locations: []*symbolizer.Location{
-					{
-						Address: 0x1500,
-						Mapping: &pprof.Mapping{},
-					},
-					{
-						Address: 0x3c5a,
-						Mapping: &pprof.Mapping{},
-					},
-					{
-						Address: 0x2745,
-						Mapping: &pprof.Mapping{},
-					},
-				},
+				MappingId: 1,
+				Address:   0x2745,
 			},
 		},
+		StringTable: []string{"", buildID},
+	}
+
+	// Marshal profile into tree report
+	data, err := profile.MarshalVT()
+	if err != nil {
+		log.Fatalf("Failed to marshal profile: %v", err)
 	}
+	report := &queryv1.TreeReport{Tree: data}
 
-	if err := s.Symbolize(ctx, req); err != nil {
+	// Run symbolization
+	if err := s.SymbolizeTree(ctx, report); err != nil {
 		log.Fatalf("Failed to symbolize: %v", err)
 	}
 
+	// Unmarshal result for printing
+	result := &googlev1.Profile{}
+	if err := result.UnmarshalVT(report.Tree); err != nil {
+		log.Fatalf("Failed to unmarshal result: %v", err)
+	}
+
+	printResults(result)
+}
+
+func printResults(p *googlev1.Profile) {
 	fmt.Println("Symbolization Results:")
-	fmt.Printf("Build ID: %s\n", buildID)
-	fmt.Println("----------------------------------------")
-
-	for i, mapping := range req.Mappings {
-		fmt.Printf("Mapping #%d:\n", i+1)
-		for _, loc := range mapping.Locations {
-			fmt.Printf("\nAddress: 0x%x\n", loc.Address)
-			if len(loc.Lines) == 0 {
-				fmt.Println("  No symbolization information found")
-				continue
-			}
+	for _, loc := range p.Location {
+		fmt.Printf("\nAddress: 0x%x\n", loc.Address)
+		if len(loc.Line) == 0 {
+			fmt.Println("  No symbolization information found")
+			continue
+		}
 
-			for j, line := range loc.Lines {
-				fmt.Printf("  Line %d:\n", j+1)
-				if line.Function != nil {
-					fmt.Printf("    Function:   %s\n", line.Function.Name)
-					fmt.Printf("    File:       %s\n", line.Function.Filename)
-					fmt.Printf("    Line:       %d\n", line.Line)
-					fmt.Printf("    StartLine:  %d\n", line.Function.StartLine)
-				} else {
-					fmt.Println("    No function information available")
-				}
+		for i, line := range loc.Line {
+			fmt.Printf("  Line %d:\n", i+1)
+			if fn := p.Function[line.FunctionId]; fn != nil {
+				fmt.Printf("    Function:   %s\n", p.StringTable[fn.Name])
+				fmt.Printf("    File:       %s\n", p.StringTable[fn.Filename])
+				fmt.Printf("    Line:       %d\n", line.Line)
+				fmt.Printf("    StartLine:  %d\n", fn.StartLine)
 			}
-			fmt.Println("----------------------------------------")
 		}
 	}
-
-	// Alternatively: Symbolize all addresses in the binary
-	// Note: Comment out the above specific symbolization when using this
-	// as it's a different approach meant for exploring all available symbols
-	//if err := symbolizer.SymbolizeAll(ctx, buildID); err != nil {
-	//	log.Fatalf("Failed to symbolize all addresses: %v", err)
-	//}
-
 	fmt.Println("\nSymbolization completed successfully.")
 }
 

pkg/experiment/query_backend/backend.go

@@ -4,31 +4,27 @@ import (
 	"context"
 	"flag"
 	"fmt"
+
 	"github.com/go-kit/log"
 	"github.com/grafana/dskit/grpcclient"
 	"github.com/grafana/dskit/services"
-	"github.com/grafana/pyroscope/pkg/objstore/client"
 	"github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/sync/errgroup"
 
 	metastorev1 "github.com/grafana/pyroscope/api/gen/proto/go/metastore/v1"
 	queryv1 "github.com/grafana/pyroscope/api/gen/proto/go/query/v1"
-	"github.com/grafana/pyroscope/pkg/experiment/symbolizer"
 	"github.com/grafana/pyroscope/pkg/util"
 )
 
 type Config struct {
 	Address          string            `yaml:"address"`
 	GRPCClientConfig grpcclient.Config `yaml:"grpc_client_config" doc:"description=Configures the gRPC client used to communicate between the query-frontends and the query-schedulers."`
-	Symbolizer       symbolizer.Config `yaml:"symbolizer"`
-	DebugStorage     client.Config     `yaml:"debug_storage"`
 }
 
 func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
 	f.StringVar(&cfg.Address, "query-backend.address", "localhost:9095", "")
 	cfg.GRPCClientConfig.RegisterFlagsWithPrefix("query-backend.grpc-client-config", f)
-	cfg.Symbolizer.RegisterFlagsWithPrefix("query-backend.symbolizer", f)
 }
 
 func (cfg *Config) Validate() error {
@@ -52,8 +48,6 @@ type QueryBackend struct {
 
 	backendClient QueryHandler
 	blockReader   QueryHandler
-
-	symbolizer *symbolizer.Symbolizer
 }
 
 func New(
@@ -63,29 +57,13 @@ func New(
 	backendClient QueryHandler,
 	blockReader QueryHandler,
 ) (*QueryBackend, error) {
-	var sym *symbolizer.Symbolizer
-	if config.Symbolizer.DebuginfodURL != "" {
-		var err error
-		sym, err = symbolizer.NewFromConfig(context.Background(), config.Symbolizer, reg)
-		if err != nil {
-			return nil, fmt.Errorf("create symbolizer: %w", err)
-		}
-	}
-
 	q := QueryBackend{
 		config:        config,
 		logger:        logger,
 		reg:           reg,
 		backendClient: backendClient,
 		blockReader:   blockReader,
-		symbolizer:    sym,
-	}
-
-	// Pass symbolizer to BlockReader if it's the right type
-	if br, ok := blockReader.(*BlockReader); ok {
-		br.symbolizer = sym
 	}
-
 	q.service = services.NewIdleService(q.starting, q.stopping)
 	return &q, nil
 }

pkg/experiment/query_backend/query_tree.go

@@ -56,9 +56,7 @@ func queryTree(q *queryContext, query *queryv1.Query) (*queryv1.Report, error) {
 	defer runutil.CloseWithErrCapture(&err, profiles, "failed to close profile stream")
 
 	resolver := symdb.NewResolver(q.ctx, q.ds.Symbols(),
-		symdb.WithResolverMaxNodes(query.Tree.GetMaxNodes()),
-		symdb.WithSymbolizer(q.symbolizer))
-
+		symdb.WithResolverMaxNodes(query.Tree.GetMaxNodes()))
 	defer resolver.Release()
 
 	if len(spanSelector) > 0 {

pkg/experiment/symbolizer/addrmapper.go

@@ -59,7 +59,8 @@ func MapRuntimeAddress(runtimeAddr uint64, ei *BinaryLayout, m Mapping) (uint64,
 		return runtimeAddr, fmt.Errorf("calculate base offset: %w", err)
 	}
 
-	return runtimeAddr - baseOffset, nil
+	result := runtimeAddr - baseOffset
+	return result, nil
 }
 
 // CalculateBase determines the base address adjustment needed for address translation.

pkg/experiment/symbolizer/cache.go

@@ -105,11 +105,9 @@ func NewNullCache() DebugInfoCache {
 }
 
 func (n *NullCache) Get(ctx context.Context, buildID string) (io.ReadCloser, error) {
-	// Always return cache miss
 	return nil, fmt.Errorf("cache miss")
 }
 
 func (n *NullCache) Put(ctx context.Context, buildID string, reader io.Reader) error {
-	// Do nothing
 	return nil
 }

pkg/experiment/symbolizer/debuginfod_client.go

@@ -1,6 +1,7 @@
 package symbolizer
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"net/http"
@@ -15,21 +16,44 @@ type DebuginfodClient interface {
 }
 
 type debuginfodClient struct {
-	baseURL string
-	metrics *Metrics
+	baseURL    string
+	metrics    *Metrics
+	httpClient *http.Client
 }
 
 func NewDebuginfodClient(baseURL string, metrics *Metrics) DebuginfodClient {
+	// Create a default transport with reasonable timeouts
+	transport := &http.Transport{
+		MaxIdleConnsPerHost: 10,
+		IdleConnTimeout:     90 * time.Second,
+		TLSHandshakeTimeout: 10 * time.Second,
+	}
+
+	client := &http.Client{
+		Transport: transport,
+		Timeout:   30 * time.Second, // Overall timeout for requests
+	}
+
 	return &debuginfodClient{
-		baseURL: baseURL,
-		metrics: metrics,
+		baseURL:    baseURL,
+		metrics:    metrics,
+		httpClient: client,
 	}
 }
 
 // FetchDebuginfo fetches the debuginfo file for a specific build ID.
 func (c *debuginfodClient) FetchDebuginfo(buildID string) (string, error) {
-	c.metrics.debuginfodRequestsTotal.Inc()
 	start := time.Now()
+	var err error
+	c.metrics.debuginfodRequestsTotal.Inc()
+	defer func() {
+		status := "success"
+		if err != nil {
+			status = "error"
+			c.metrics.debuginfodRequestErrorsTotal.WithLabelValues("http").Inc()
+		}
+		c.metrics.debuginfodRequestDuration.WithLabelValues(status).Observe(time.Since(start).Seconds())
+	}()
 
 	sanitizedBuildID, err := sanitizeBuildID(buildID)
 	if err != nil {
@@ -39,7 +63,16 @@ func (c *debuginfodClient) FetchDebuginfo(buildID string) (string, error) {
 
 	url := fmt.Sprintf("%s/buildid/%s/debuginfo", c.baseURL, sanitizedBuildID)
 
-	resp, err := http.Get(url)
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil)
+	if err != nil {
+		c.metrics.debuginfodRequestErrorsTotal.WithLabelValues("request_creation").Inc()
+		return "", fmt.Errorf("failed to create request: %w", err)
+	}
+
+	req.Header.Set("Accept-Encoding", "gzip, deflate")
+
+	resp, err := c.httpClient.Do(req)
+	//resp, err := http.Get(url)
 	if err != nil {
 		c.metrics.debuginfodRequestErrorsTotal.WithLabelValues("http").Inc()
 		c.metrics.debuginfodRequestDuration.WithLabelValues("error").Observe(time.Since(start).Seconds())
@@ -65,20 +98,16 @@ func (c *debuginfodClient) FetchDebuginfo(buildID string) (string, error) {
 	outFile, err := os.Create(filePath)
 	if err != nil {
 		c.metrics.debuginfodRequestErrorsTotal.WithLabelValues("file_create").Inc()
-		c.metrics.debuginfodRequestDuration.WithLabelValues("error").Observe(time.Since(start).Seconds())
 		return "", fmt.Errorf("failed to create temp file: %w", err)
 	}
 	defer outFile.Close()
 
 	_, err = io.Copy(outFile, resp.Body)
 	if err != nil {
 		c.metrics.debuginfodRequestErrorsTotal.WithLabelValues("write").Inc()
-		c.metrics.debuginfodRequestDuration.WithLabelValues("error").Observe(time.Since(start).Seconds())
 		return "", fmt.Errorf("failed to write debuginfod to file: %w", err)
 	}
 
-	c.metrics.debuginfodRequestDuration.WithLabelValues("success").Observe(time.Since(start).Seconds())
-
 	return filePath, nil
 }