fix: otlp pprof fixes (#3741)

    Do not append sample labels to series labels (imagine two samples with process=firefox and process=chrome, these attributes should not be appended to Push request series labels. Instead add them as sample labels.
    Fix function name for unsymbolized functions, previously it was incorrectly using only one function per mapping. Now we create multiple (a lot of them actually) functions with format libfoo.so 0x...
    Remove debugging pprof dump to fs

Author: korniltsev

.mockery.yaml

@@ -24,6 +24,9 @@ packages:
   github.com/grafana/pyroscope/pkg/frontend:
     interfaces:
       Limits:
+  github.com/grafana/pyroscope/pkg/ingester/otlp:
+    interfaces:
+      PushService:
   github.com/grafana/pyroscope/pkg/experiment/metastore/discovery:
     interfaces:
       Discovery:

pkg/ingester/otlp/convert.go

@@ -30,6 +30,15 @@ func ConvertOtelToGoogle(src *otelProfile.Profile) *googleProfile.Profile {
 		Comment:           src.Comment,
 		Mapping:           convertMappingsBack(src.Mapping),
 	}
+	stringmap := make(map[string]int)
+	addstr := func(s string) int64 {
+		if _, ok := stringmap[s]; !ok {
+			stringmap[s] = len(dst.StringTable)
+			dst.StringTable = append(dst.StringTable, s)
+		}
+		return int64(stringmap[s])
+	}
+	addstr("")
 
 	if dst.TimeNanos == 0 {
 		dst.TimeNanos = time.Now().UnixNano()
@@ -48,49 +57,44 @@ func ConvertOtelToGoogle(src *otelProfile.Profile) *googleProfile.Profile {
 		gf.Id = uint64(i + 1)
 		dst.Function = append(dst.Function, gf)
 	}
+	funcmap := map[string]uint64{}
+	addfunc := func(s string) uint64 {
+		if _, ok := funcmap[s]; !ok {
+			funcmap[s] = uint64(len(dst.Function)) + 1
+			dst.Function = append(dst.Function, &googleProfile.Function{
+				Id:   funcmap[s],
+				Name: addstr(s),
+			})
+		}
+		return funcmap[s]
+	}
 
-	functionOffset := uint64(len(dst.Function)) + 1
 	dst.Location = []*googleProfile.Location{}
-	locationMappingIndexAddressMap := make(map[uint64]uint64)
 	// Convert locations and mappings
 	for i, loc := range src.Location {
 		gl := convertLocationBack(loc)
 		gl.Id = uint64(i + 1)
 		if len(gl.Line) == 0 {
+			m := src.Mapping[loc.MappingIndex]
 			gl.Line = append(gl.Line, &googleProfile.Line{
-				FunctionId: loc.MappingIndex + functionOffset,
+				FunctionId: addfunc(fmt.Sprintf("%s 0x%x", src.StringTable[m.Filename], loc.Address)),
 			})
 		}
 		dst.Location = append(dst.Location, gl)
-		locationMappingIndexAddressMap[loc.MappingIndex] = loc.Address
-	}
-
-	for _, m := range src.Mapping {
-		address := locationMappingIndexAddressMap[m.Id]
-		addressStr := fmt.Sprintf("%s 0x%x", dst.StringTable[m.Filename], address)
-		dst.StringTable = append(dst.StringTable, addressStr)
-		// i == 0 function_id = functionOffset
-		id := uint64(len(dst.Function)) + 1
-		dst.Function = append(dst.Function, &googleProfile.Function{
-			Id:   id,
-			Name: int64(len(dst.StringTable) - 1),
-		})
 	}
 
 	// Convert samples
 	for _, sample := range src.Sample {
-		gs := convertSampleBack(sample, src.LocationIndices)
+		gs := convertSampleBack(src, sample, src.LocationIndices, addstr)
 		dst.Sample = append(dst.Sample, gs)
 	}
 
 	if len(dst.SampleType) == 0 {
-		dst.StringTable = append(dst.StringTable, "samples")
-		dst.StringTable = append(dst.StringTable, "ms")
 		dst.SampleType = []*googleProfile.ValueType{{
-			Type: int64(len(dst.StringTable) - 2),
-			Unit: int64(len(dst.StringTable) - 1),
+			Type: addstr("samples"),
+			Unit: addstr("ms"),
 		}}
-		dst.DefaultSampleType = int64(len(dst.StringTable) - 2)
+		dst.DefaultSampleType = addstr("samples")
 	}
 
 	return dst
@@ -147,14 +151,15 @@ func convertFunctionBack(of *otelProfile.Function) *googleProfile.Function {
 	}
 }
 
-func convertSampleBack(os *otelProfile.Sample, locationIndexes []int64) *googleProfile.Sample {
+func convertSampleBack(p *otelProfile.Profile, os *otelProfile.Sample, locationIndexes []int64, addstr func(s string) int64) *googleProfile.Sample {
 	gs := &googleProfile.Sample{
 		Value: os.Value,
 	}
 
 	if len(gs.Value) == 0 {
 		gs.Value = []int64{int64(len(os.TimestampsUnixNano))}
 	}
+	convertSampleAttributesToLabelsBack(p, os, gs, addstr)
 
 	for i := os.LocationsStartIndex; i < os.LocationsStartIndex+os.LocationsLength; i++ {
 		gs.LocationId = append(gs.LocationId, uint64(locationIndexes[i]+1))
@@ -163,6 +168,19 @@ func convertSampleBack(os *otelProfile.Sample, locationIndexes []int64) *googleP
 	return gs
 }
 
+func convertSampleAttributesToLabelsBack(p *otelProfile.Profile, os *otelProfile.Sample, gs *googleProfile.Sample, addstr func(s string) int64) {
+	gs.Label = make([]*googleProfile.Label, 0, len(os.Attributes))
+	for _, attribute := range os.Attributes {
+		att := p.AttributeTable[attribute]
+		if att.Value.GetStringValue() != "" {
+			gs.Label = append(gs.Label, &googleProfile.Label{
+				Key: addstr(att.Key),
+				Str: addstr(att.Value.GetStringValue()),
+			})
+		}
+	}
+}
+
 // convertMappingsBack converts a slice of OpenTelemetry Mapping entries to Google Mapping entries.
 func convertMappingsBack(otelMappings []*otelProfile.Mapping) []*googleProfile.Mapping {
 	googleMappings := make([]*googleProfile.Mapping, len(otelMappings))

pkg/ingester/otlp/ingest_handler.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"os"
 	"strings"
 
 	"connectrpc.com/connect"
@@ -18,7 +17,6 @@ import (
 	typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
 	pprofileotlp "github.com/grafana/pyroscope/api/otlp/collector/profiles/v1experimental"
 	v1 "github.com/grafana/pyroscope/api/otlp/common/v1"
-	"github.com/grafana/pyroscope/api/otlp/profiles/v1experimental"
 	"github.com/grafana/pyroscope/pkg/tenant"
 )
 
@@ -111,16 +109,11 @@ func (h *ingestHandler) Export(ctx context.Context, er *pprofileotlp.ExportProfi
 				// Add profile attributes
 				labels = appendAttributesUnique(labels, p.GetAttributes(), processedKeys)
 
-				// Add profile-specific attributes from samples/attributetable
-				labels = appendProfileLabels(labels, p.Profile, processedKeys)
-
 				pprofBytes, err := OprofToPprof(p.Profile)
 				if err != nil {
 					return &pprofileotlp.ExportProfilesServiceResponse{}, fmt.Errorf("failed to convert from OTLP to legacy pprof: %w", err)
 				}
 
-				_ = os.WriteFile(".tmp/elastic.pprof", pprofBytes, 0644)
-
 				req := &pushv1.PushRequest{
 					Series: []*pushv1.RawProfileSeries{
 						{
@@ -199,41 +192,3 @@ func appendAttributesUnique(labels []*typesv1.LabelPair, attrs []v1.KeyValue, pr
 	}
 	return labels
 }
-
-func appendProfileLabels(labels []*typesv1.LabelPair, profile *v1experimental.Profile, processedKeys map[string]bool) []*typesv1.LabelPair {
-	if profile == nil {
-		return labels
-	}
-
-	// Create mapping of attribute indices to their values
-	attrMap := make(map[uint64]v1.AnyValue)
-	for i, attr := range profile.GetAttributeTable() {
-		val := attr.GetValue()
-		if val.GetValue() != nil {
-			attrMap[uint64(i)] = val
-		}
-	}
-
-	// Process only attributes referenced in samples
-	for _, sample := range profile.Sample {
-		for _, attrIdx := range sample.GetAttributes() {
-			attr := profile.AttributeTable[attrIdx]
-			// Skip if we've already processed this key at any level
-			if processedKeys[attr.Key] {
-				continue
-			}
-
-			if value, exists := attrMap[attrIdx]; exists {
-				if sv := value.GetStringValue(); sv != "" {
-					labels = append(labels, &typesv1.LabelPair{
-						Name:  attr.Key,
-						Value: sv,
-					})
-					processedKeys[attr.Key] = true
-				}
-			}
-		}
-	}
-
-	return labels
-}