[fix] Don't 500 on bad profile types

.gitignore

@@ -24,6 +24,7 @@ data-metastore/
 .cursor
 .idea
 .vscode
+.zed
 
 # Frontend
 public/build

pkg/frontend/readpath/queryfrontend/query_diff.go

@@ -32,6 +32,16 @@ func (q *QueryFrontend) Diff(
 	c.Msg.Left.MaxNodes = &maxNodes
 	c.Msg.Right.MaxNodes = &maxNodes
 
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.Left.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.Right.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
 	var left, right []byte
 	g, ctx := errgroup.WithContext(ctx)
 	g.Go(func() error {

pkg/frontend/readpath/queryfrontend/query_select_merge_profile.go

@@ -9,6 +9,7 @@ import (
 	profilev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
 	querierv1 "github.com/grafana/pyroscope/api/gen/proto/go/querier/v1"
 	queryv1 "github.com/grafana/pyroscope/api/gen/proto/go/query/v1"
+	phlaremodel "github.com/grafana/pyroscope/pkg/model"
 	"github.com/grafana/pyroscope/pkg/pprof"
 	"github.com/grafana/pyroscope/pkg/validation"
 )
@@ -29,6 +30,11 @@ func (q *QueryFrontend) SelectMergeProfile(
 		return connect.NewResponse(&profilev1.Profile{}), nil
 	}
 
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
 	// NOTE: Max nodes limit is not set by default:
 	//   the method is used for pprof export and
 	//   truncation might not be desirable.

pkg/frontend/readpath/queryfrontend/query_select_merge_span_profile.go

@@ -30,6 +30,11 @@ func (q *QueryFrontend) SelectMergeSpanProfile(
 		return connect.NewResponse(&querierv1.SelectMergeSpanProfileResponse{}), nil
 	}
 
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
 	maxNodes, err := validation.ValidateMaxNodes(q.limits, tenantIDs, c.Msg.GetMaxNodes())
 	if err != nil {
 		return nil, connect.NewError(connect.CodeInvalidArgument, err)

pkg/frontend/readpath/queryfrontend/query_select_merge_stacktraces.go

@@ -54,6 +54,12 @@ func (q *QueryFrontend) selectMergeStacktracesTree(
 	if err != nil {
 		return nil, connect.NewError(connect.CodeInvalidArgument, err)
 	}
+
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
 	labelSelector, err := buildLabelSelectorWithProfileType(c.Msg.LabelSelector, c.Msg.ProfileTypeID)
 	if err != nil {
 		return nil, err

pkg/frontend/readpath/queryfrontend/query_select_time_series.go

@@ -29,6 +29,11 @@ func (q *QueryFrontend) SelectSeries(
 		return connect.NewResponse(&querierv1.SelectSeriesResponse{}), nil
 	}
 
+	_, err = phlaremodel.ParseProfileTypeSelector(c.Msg.ProfileTypeID)
+	if err != nil {
+		return nil, connect.NewError(connect.CodeInvalidArgument, err)
+	}
+
 	stepMs := time.Duration(c.Msg.Step * float64(time.Second)).Milliseconds()
 	start := c.Msg.Start - stepMs
 

pkg/model/profile.go

@@ -8,9 +8,7 @@ import (
 	"strings"
 
 	"github.com/cespare/xxhash/v2"
-	"github.com/gogo/status"
 	"github.com/prometheus/prometheus/model/labels"
-	"google.golang.org/grpc/codes"
 
 	profilev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
 	ingestv1 "github.com/grafana/pyroscope/api/gen/proto/go/ingester/v1"
@@ -31,7 +29,7 @@ func ParseProfileTypeSelector(id string) (*typesv1.ProfileType, error) {
 	parts := strings.Split(id, ":")
 
 	if len(parts) != 5 && len(parts) != 6 {
-		return nil, status.Errorf(codes.InvalidArgument, "profile-type selection must be of the form <name>:<sample-type>:<sample-unit>:<period-type>:<period-unit>(:delta), got(%d): %q", len(parts), id)
+		return nil, fmt.Errorf("profile-type selection must be of the form <name>:<sample-type>:<sample-unit>:<period-type>:<period-unit>(:delta), got(%d): %q", len(parts), id)
 	}
 	name, sampleType, sampleUnit, periodType, periodUnit := parts[0], parts[1], parts[2], parts[3], parts[4]
 	return &typesv1.ProfileType{

pkg/model/profile_test.go

@@ -4,8 +4,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	profilev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
+	typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1"
 )
 
 func Test_extractMappingFilename(t *testing.T) {
@@ -97,3 +99,263 @@ func Test_symbolsPartitionKeyForProfile(t *testing.T) {
 		})
 	}
 }
+
+func Test_ParseProfileTypeSelector(t *testing.T) {
+	tests := []struct {
+		Name    string
+		ID      string
+		Want    *typesv1.ProfileType
+		WantErr string
+	}{
+		{
+			Name: "block_contentions_count",
+			ID:   "block:contentions:count:contentions:count",
+			Want: &typesv1.ProfileType{
+				Name:       "block",
+				ID:         "block:contentions:count:contentions:count",
+				SampleType: "contentions",
+				SampleUnit: "count",
+				PeriodType: "contentions",
+				PeriodUnit: "count",
+			},
+		},
+		{
+			Name: "mutex_delay_nanoseconds",
+			ID:   "mutex:delay:nanoseconds:contentions:count",
+			Want: &typesv1.ProfileType{
+				Name:       "mutex",
+				ID:         "mutex:delay:nanoseconds:contentions:count",
+				SampleType: "delay",
+				SampleUnit: "nanoseconds",
+				PeriodType: "contentions",
+				PeriodUnit: "count",
+			},
+		},
+		{
+			Name: "memory_alloc_space_bytes",
+			ID:   "memory:alloc_space:bytes:space:bytes",
+			Want: &typesv1.ProfileType{
+				Name:       "memory",
+				ID:         "memory:alloc_space:bytes:space:bytes",
+				SampleType: "alloc_space",
+				SampleUnit: "bytes",
+				PeriodType: "space",
+				PeriodUnit: "bytes",
+			},
+		},
+		{
+			Name:    "too_few_parts",
+			ID:      "memory:alloc_space:bytes:space",
+			WantErr: `profile-type selection must be of the form <name>:<sample-type>:<sample-unit>:<period-type>:<period-unit>(:delta), got(4): "memory:alloc_space:bytes:space"`,
+		},
+		{
+			Name:    "too_many_parts",
+			ID:      "memory:alloc_space:bytes:space:bytes:extra:part",
+			WantErr: `profile-type selection must be of the form <name>:<sample-type>:<sample-unit>:<period-type>:<period-unit>(:delta), got(7): "memory:alloc_space:bytes:space:bytes:extra:part"`,
+		},
+		{
+			Name:    "empty_string",
+			ID:      "",
+			WantErr: `profile-type selection must be of the form <name>:<sample-type>:<sample-unit>:<period-type>:<period-unit>(:delta), got(1): ""`,
+		},
+		{
+			Name: "valid_with_delta",
+			ID:   "cpu:samples:count:cpu:nanoseconds:delta",
+			Want: &typesv1.ProfileType{
+				Name:       "cpu",
+				ID:         "cpu:samples:count:cpu:nanoseconds:delta",
+				SampleType: "samples",
+				SampleUnit: "count",
+				PeriodType: "cpu",
+				PeriodUnit: "nanoseconds",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.Name, func(t *testing.T) {
+			got, err := ParseProfileTypeSelector(tt.ID)
+			if tt.WantErr != "" {
+				require.Error(t, err)
+				require.EqualError(t, err, tt.WantErr)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.Want, got)
+			}
+		})
+	}
+}
+
+func Test_ParseProfileTypeSelector_ValidProfileTypes(t *testing.T) {
+	// Shamelessly copied from: https://github.com/grafana/profiles-drilldown/blob/4e261a8744034bddefdec757d5d2e1d8dc0ec2bb/src/shared/infrastructure/profile-metrics/profile-metrics.json#L93
+	var validProfileTypes = map[string]*typesv1.ProfileType{
+		"block:contentions:count:contentions:count": {
+			ID:         "block:contentions:count:contentions:count",
+			Name:       "block",
+			SampleType: "contentions",
+			SampleUnit: "count",
+			PeriodType: "contentions",
+			PeriodUnit: "count",
+		},
+		"block:delay:nanoseconds:contentions:count": {
+			ID:         "block:delay:nanoseconds:contentions:count",
+			Name:       "block",
+			SampleType: "delay",
+			SampleUnit: "nanoseconds",
+			PeriodType: "contentions",
+			PeriodUnit: "count",
+		},
+		"goroutine:goroutine:count:goroutine:count": {
+			ID:         "goroutine:goroutine:count:goroutine:count",
+			Name:       "goroutine",
+			SampleType: "goroutine",
+			SampleUnit: "count",
+			PeriodType: "goroutine",
+			PeriodUnit: "count",
+		},
+		"goroutines:goroutine:count:goroutine:count": {
+			ID:         "goroutines:goroutine:count:goroutine:count",
+			Name:       "goroutines",
+			SampleType: "goroutine",
+			SampleUnit: "count",
+			PeriodType: "goroutine",
+			PeriodUnit: "count",
+		},
+		"memory:alloc_in_new_tlab_bytes:bytes::": {
+			ID:         "memory:alloc_in_new_tlab_bytes:bytes::",
+			Name:       "memory",
+			SampleType: "alloc_in_new_tlab_bytes",
+			SampleUnit: "bytes",
+			PeriodType: "",
+			PeriodUnit: "",
+		},
+		"memory:alloc_in_new_tlab_objects:count::": {
+			ID:         "memory:alloc_in_new_tlab_objects:count::",
+			Name:       "memory",
+			SampleType: "alloc_in_new_tlab_objects",
+			SampleUnit: "count",
+			PeriodType: "",
+			PeriodUnit: "",
+		},
+		"memory:alloc_objects:count:space:bytes": {
+			ID:         "memory:alloc_objects:count:space:bytes",
+			Name:       "memory",
+			SampleType: "alloc_objects",
+			SampleUnit: "count",
+			PeriodType: "space",
+			PeriodUnit: "bytes",
+		},
+		"memory:alloc_space:bytes:space:bytes": {
+			ID:         "memory:alloc_space:bytes:space:bytes",
+			Name:       "memory",
+			SampleType: "alloc_space",
+			SampleUnit: "bytes",
+			PeriodType: "space",
+			PeriodUnit: "bytes",
+		},
+		"memory:inuse_objects:count:space:bytes": {
+			ID:         "memory:inuse_objects:count:space:bytes",
+			Name:       "memory",
+			SampleType: "inuse_objects",
+			SampleUnit: "count",
+			PeriodType: "space",
+			PeriodUnit: "bytes",
+		},
+		"memory:inuse_space:bytes:space:bytes": {
+			ID:         "memory:inuse_space:bytes:space:bytes",
+			Name:       "memory",
+			SampleType: "inuse_space",
+			SampleUnit: "bytes",
+			PeriodType: "space",
+			PeriodUnit: "bytes",
+		},
+		"mutex:contentions:count:contentions:count": {
+			ID:         "mutex:contentions:count:contentions:count",
+			Name:       "mutex",
+			SampleType: "contentions",
+			SampleUnit: "count",
+			PeriodType: "contentions",
+			PeriodUnit: "count",
+		},
+		"mutex:delay:nanoseconds:contentions:count": {
+			ID:         "mutex:delay:nanoseconds:contentions:count",
+			Name:       "mutex",
+			SampleType: "delay",
+			SampleUnit: "nanoseconds",
+			PeriodType: "contentions",
+			PeriodUnit: "count",
+		},
+		"process_cpu:alloc_samples:count:cpu:nanoseconds": {
+			ID:         "process_cpu:alloc_samples:count:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "alloc_samples",
+			SampleUnit: "count",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:alloc_size:bytes:cpu:nanoseconds": {
+			ID:         "process_cpu:alloc_size:bytes:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "alloc_size",
+			SampleUnit: "bytes",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:cpu:nanoseconds:cpu:nanoseconds": {
+			ID:         "process_cpu:cpu:nanoseconds:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "cpu",
+			SampleUnit: "nanoseconds",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:exception:count:cpu:nanoseconds": {
+			ID:         "process_cpu:exception:count:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "exception",
+			SampleUnit: "count",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:lock_count:count:cpu:nanoseconds": {
+			ID:         "process_cpu:lock_count:count:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "lock_count",
+			SampleUnit: "count",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:lock_time:nanoseconds:cpu:nanoseconds": {
+			ID:         "process_cpu:lock_time:nanoseconds:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "lock_time",
+			SampleUnit: "nanoseconds",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+		"process_cpu:samples:count::milliseconds": {
+			ID:         "process_cpu:samples:count::milliseconds",
+			Name:       "process_cpu",
+			SampleType: "samples",
+			SampleUnit: "count",
+			PeriodType: "",
+			PeriodUnit: "milliseconds",
+		},
+		"process_cpu:samples:count:cpu:nanoseconds": {
+			ID:         "process_cpu:samples:count:cpu:nanoseconds",
+			Name:       "process_cpu",
+			SampleType: "samples",
+			SampleUnit: "count",
+			PeriodType: "cpu",
+			PeriodUnit: "nanoseconds",
+		},
+	}
+
+	for id, want := range validProfileTypes {
+		t.Run(id, func(t *testing.T) {
+			got, err := ParseProfileTypeSelector(id)
+			require.NoError(t, err)
+			require.Equal(t, want, got)
+		})
+	}
+}