diff --git a/cmd/pyroscope/help-all.txt.tmpl b/cmd/pyroscope/help-all.txt.tmpl index fba3fba141..094ed64922 100644 --- a/cmd/pyroscope/help-all.txt.tmpl +++ b/cmd/pyroscope/help-all.txt.tmpl @@ -15,6 +15,8 @@ Usage of ./pyroscope: How frequently to scan the bucket, or to refresh the bucket index (if enabled), in order to look for changes (new blocks shipped by ingesters and blocks deleted by retention or compaction). (default 15m0s) -blocks-storage.bucket-store.tenant-sync-concurrency int Maximum number of concurrent tenants synching blocks. (default 10) + -client-capability.allow-utf8-labelnames + [experimental] Enable reading and writing utf-8 label names. To use this feature, API calls must include `allow-utf8-labelnames=true` in the `Accept` header. -compactor.block-ranges value List of compaction time ranges. (default 1h0m0s,2h0m0s,8h0m0s) -compactor.block-sync-concurrency int diff --git a/docs/sources/configure-server/reference-configuration-parameters/index.md b/docs/sources/configure-server/reference-configuration-parameters/index.md index c46dfb9e5c..7de947a4b8 100644 --- a/docs/sources/configure-server/reference-configuration-parameters/index.md +++ b/docs/sources/configure-server/reference-configuration-parameters/index.md @@ -160,6 +160,12 @@ tenant_settings: # CLI flag: -tenant-settings.recording-rules.enabled [enabled: | default = false] +client_capability: + # Enable reading and writing utf-8 label names. To use this feature, API calls + # must include `allow-utf8-labelnames=true` in the `Accept` header. + # CLI flag: -client-capability.allow-utf8-labelnames + [allow_utf_8_label_names: | default = false] + storage: # Backend storage to use. Supported backends are: s3, gcs, azure, swift, # filesystem, cos. diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go index 830f3cef5f..be70a6b970 100644 --- a/pkg/distributor/distributor.go +++ b/pkg/distributor/distributor.go @@ -17,6 +17,8 @@ import ( "connectrpc.com/connect" "go.uber.org/atomic" + "github.com/grafana/pyroscope/pkg/featureflags" + "github.com/dustin/go-humanize" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -651,7 +653,7 @@ func visitSampleSeriesForIngester(profile *profilev1.Profile, labels []*typesv1. } func (d *Distributor) sendRequestsToIngester(ctx context.Context, req *distributormodel.ProfileSeries) (resp *connect.Response[pushv1.PushResponse], err error) { - sampleSeries, err := d.visitSampleSeries(req, visitSampleSeriesForIngester) + sampleSeries, err := d.visitSampleSeries(ctx, req, visitSampleSeriesForIngester) if err != nil { return nil, err } @@ -740,7 +742,7 @@ func (d *Distributor) sendRequestsToSegmentWriter(ctx context.Context, req *dist // NOTE(kolesnikovae): if we return early, e.g., due to a validation error, // or if there are no series, the write path router has already seen the // request, and could have already accounted for the size, latency, etc. - serviceSeries, err := d.visitSampleSeries(req, visitSampleSeriesForSegmentWriter) + serviceSeries, err := d.visitSampleSeries(ctx, req, visitSampleSeriesForSegmentWriter) if err != nil { return nil, err } @@ -1126,15 +1128,23 @@ func injectMappingVersions(s *distributormodel.ProfileSeries) error { type visitFunc func(*profilev1.Profile, []*typesv1.LabelPair, []*relabel.Config, *sampleSeriesVisitor) error -func (d *Distributor) visitSampleSeries(s *distributormodel.ProfileSeries, visit visitFunc) ([]*distributormodel.ProfileSeries, error) { +func (d *Distributor) visitSampleSeries(ctx context.Context, s *distributormodel.ProfileSeries, visit visitFunc) ([]*distributormodel.ProfileSeries, error) { relabelingRules := d.limits.IngestionRelabelingRules(s.TenantID) usageConfig := d.limits.DistributorUsageGroups(s.TenantID) var result []*distributormodel.ProfileSeries usageGroups := d.usageGroupEvaluator.GetMatch(s.TenantID, usageConfig, s.Labels) + + // Extract client capabilities from context + var clientCapabilities *featureflags.ClientCapabilities + if capabilities, ok := featureflags.GetClientCapabilities(ctx); ok { + clientCapabilities = &capabilities + } + visitor := &sampleSeriesVisitor{ - tenantID: s.TenantID, - limits: d.limits, - profile: s.Profile, + tenantID: s.TenantID, + limits: d.limits, + profile: s.Profile, + clientCapabilities: clientCapabilities, } if err := visit(s.Profile.Profile, s.Labels, relabelingRules, visitor); err != nil { validation.DiscardedProfiles.WithLabelValues(string(validation.ReasonOf(err)), s.TenantID).Add(float64(s.TotalProfiles)) @@ -1164,18 +1174,19 @@ func (d *Distributor) visitSampleSeries(s *distributormodel.ProfileSeries, visit } type sampleSeriesVisitor struct { - tenantID string - limits Limits - profile *pprof.Profile - exp *pprof.SampleExporter - series []*distributormodel.ProfileSeries + tenantID string + limits Limits + profile *pprof.Profile + exp *pprof.SampleExporter + clientCapabilities *featureflags.ClientCapabilities + series []*distributormodel.ProfileSeries discardedBytes int discardedProfiles int } func (v *sampleSeriesVisitor) ValidateLabels(labels phlaremodel.Labels) error { - return validation.ValidateLabels(v.limits, v.tenantID, labels) + return validation.ValidateLabels(v.clientCapabilities, v.limits, v.tenantID, labels) } func (v *sampleSeriesVisitor) VisitProfile(labels phlaremodel.Labels) { diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index 52dc958646..563e437cd9 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -1228,7 +1228,7 @@ func Test_SampleLabels_Ingester(t *testing.T) { }}, overrides, nil, log.NewLogfmtLogger(os.Stdout), nil) require.NoError(t, err) var series []*distributormodel.ProfileSeries - series, err = d.visitSampleSeries(tc.pushReq, visitSampleSeriesForIngester) + series, err = d.visitSampleSeries(context.Background(), tc.pushReq, visitSampleSeriesForIngester) assert.Equal(t, tc.expectBytesDropped, float64(tc.pushReq.DiscardedBytesRelabeling)) assert.Equal(t, tc.expectProfilesDropped, float64(tc.pushReq.DiscardedProfilesRelabeling)) @@ -1786,7 +1786,7 @@ func Test_SampleLabels_SegmentWriter(t *testing.T) { require.NoError(t, err) var series []*distributormodel.ProfileSeries - series, err = d.visitSampleSeries(tc.pushReq, visitSampleSeriesForSegmentWriter) + series, err = d.visitSampleSeries(context.Background(), tc.pushReq, visitSampleSeriesForSegmentWriter) assert.Equal(t, tc.expectBytesDropped, float64(tc.pushReq.DiscardedBytesRelabeling)) assert.Equal(t, tc.expectProfilesDropped, float64(tc.pushReq.DiscardedProfilesRelabeling)) diff --git a/pkg/featureflags/client_capability.go b/pkg/featureflags/client_capability.go index f8f5c1c7f0..ec78176877 100644 --- a/pkg/featureflags/client_capability.go +++ b/pkg/featureflags/client_capability.go @@ -2,14 +2,15 @@ package featureflags import ( "context" + "flag" "mime" "net/http" "strings" "connectrpc.com/connect" + "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/grafana/dskit/middleware" - "github.com/grafana/pyroscope/pkg/util" "google.golang.org/grpc" "google.golang.org/grpc/metadata" ) @@ -17,8 +18,26 @@ import ( const ( // Capability names - update parseClientCapabilities below when new capabilities added allowUtf8LabelNamesCapabilityName string = "allow-utf8-labelnames" + + // Config + clientCapabilityPrefix = "client-capability." + allowUtf8LabelNames = clientCapabilityPrefix + allowUtf8LabelNamesCapabilityName ) +type ClientCapabilityConfig struct { + AllowUtf8LabelNames bool `yaml:"allow_utf_8_label_names" category:"experimental"` +} + +func (cfg *ClientCapabilityConfig) RegisterFlags(fs *flag.FlagSet) { + fs.BoolVar( + &cfg.AllowUtf8LabelNames, + allowUtf8LabelNames, + false, + "Enable reading and writing utf-8 label names. To use this feature, API calls must "+ + "include `allow-utf8-labelnames=true` in the `Accept` header.", + ) +} + // Define a custom context key type to avoid collisions type contextKey struct{} @@ -35,7 +54,7 @@ func GetClientCapabilities(ctx context.Context) (ClientCapabilities, bool) { return value, ok } -func ClientCapabilitiesGRPCMiddleware() grpc.UnaryServerInterceptor { +func ClientCapabilitiesGRPCMiddleware(cfg *ClientCapabilityConfig, logger log.Logger) grpc.UnaryServerInterceptor { return func( ctx context.Context, req interface{}, @@ -56,7 +75,7 @@ func ClientCapabilitiesGRPCMiddleware() grpc.UnaryServerInterceptor { } // Reuse existing HTTP header parsing - clientCapabilities, err := parseClientCapabilities(httpHeader) + clientCapabilities, err := parseClientCapabilities(httpHeader, cfg, logger) if err != nil { return nil, connect.NewError(connect.CodeInvalidArgument, err) } @@ -68,10 +87,10 @@ func ClientCapabilitiesGRPCMiddleware() grpc.UnaryServerInterceptor { // ClientCapabilitiesHttpMiddleware creates middleware that extracts and parses the // `Accept` header for capabilities the client supports -func ClientCapabilitiesHttpMiddleware() middleware.Interface { +func ClientCapabilitiesHttpMiddleware(cfg *ClientCapabilityConfig, logger log.Logger) middleware.Interface { return middleware.Func(func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - clientCapabilities, err := parseClientCapabilities(r.Header) + clientCapabilities, err := parseClientCapabilities(r.Header, cfg, logger) if err != nil { http.Error(w, "Invalid header format: "+err.Error(), http.StatusBadRequest) return @@ -83,7 +102,7 @@ func ClientCapabilitiesHttpMiddleware() middleware.Interface { }) } -func parseClientCapabilities(header http.Header) (ClientCapabilities, error) { +func parseClientCapabilities(header http.Header, cfg *ClientCapabilityConfig, logger log.Logger) (ClientCapabilities, error) { acceptHeaderValues := header.Values("Accept") var capabilities ClientCapabilities @@ -100,10 +119,16 @@ func parseClientCapabilities(header http.Header) (ClientCapabilities, error) { switch k { case allowUtf8LabelNamesCapabilityName: if v == "true" { - capabilities.AllowUtf8LabelNames = true + if !cfg.AllowUtf8LabelNames { + level.Warn(logger).Log( + "msg", "client requested capability that is not enabled on server", + "capability", allowUtf8LabelNamesCapabilityName) + } else { + capabilities.AllowUtf8LabelNames = true + } } default: - level.Debug(util.Logger).Log( + level.Debug(logger).Log( "msg", "unknown capability parsed from Accept header", "acceptHeaderKey", k, "acceptHeaderValue", v) diff --git a/pkg/featureflags/client_capability_test.go b/pkg/featureflags/client_capability_test.go index 85d284deca..f4019e526d 100644 --- a/pkg/featureflags/client_capability_test.go +++ b/pkg/featureflags/client_capability_test.go @@ -4,6 +4,7 @@ import ( "net/http" "testing" + "github.com/go-kit/log" "github.com/stretchr/testify/require" ) @@ -11,6 +12,7 @@ func Test_parseClientCapabilities(t *testing.T) { tests := []struct { Name string Header http.Header + Config *ClientCapabilityConfig Want ClientCapabilities WantError bool ErrorMessage string @@ -18,6 +20,7 @@ func Test_parseClientCapabilities(t *testing.T) { { Name: "empty header returns default capabilities", Header: http.Header{}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { @@ -25,80 +28,99 @@ func Test_parseClientCapabilities(t *testing.T) { Header: http.Header{ "Content-Type": []string{"application/json"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "empty Accept header value returns default capabilities", Header: http.Header{ "Accept": []string{""}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "simple Accept header without capabilities", Header: http.Header{ "Accept": []string{"application/json"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, + }, + { + Name: "Accept header with utf8 label names capability true - config enabled", + Header: http.Header{ + "Accept": []string{"*/*;allow-utf8-labelnames=true"}, + }, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { - Name: "Accept header with utf8 label names capability true", + Name: "Accept header with utf8 label names capability true - config disabled", Header: http.Header{ - "Accept": []string{"*/*; allow-utf8-labelnames=true"}, + "Accept": []string{"*/*;allow-utf8-labelnames=true"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: false}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "Accept header with utf8 label names capability false", Header: http.Header{ - "Accept": []string{"*/*; allow-utf8-labelnames=false"}, + "Accept": []string{"*/*;allow-utf8-labelnames=false"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "Accept header with utf8 label names capability invalid value", Header: http.Header{ - "Accept": []string{"*/*; allow-utf8-labelnames=invalid"}, + "Accept": []string{"*/*;allow-utf8-labelnames=invalid"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "Accept header with unknown capability", Header: http.Header{ - "Accept": []string{"*/*; unknown-capability=true"}, + "Accept": []string{"*/*;unknown-capability=true"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, { Name: "Accept header with multiple capabilities", Header: http.Header{ - "Accept": []string{"*/*; allow-utf8-labelnames=true; unknown-capability=false"}, + "Accept": []string{"*/*;allow-utf8-labelnames=true;unknown-capability=false"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "multiple Accept header values", Header: http.Header{ - "Accept": []string{"application/json", "*/*; allow-utf8-labelnames=true"}, + "Accept": []string{"application/json", "*/*;allow-utf8-labelnames=true"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "multiple Accept header values with different capabilities", Header: http.Header{ "Accept": []string{ - "application/json; allow-utf8-labelnames=false", - "*/*; allow-utf8-labelnames=true", + "application/json;allow-utf8-labelnames=false", + "*/*;allow-utf8-labelnames=true", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "Accept header with quality values", Header: http.Header{ - "Accept": []string{"text/html; q=0.9; allow-utf8-labelnames=true"}, + "Accept": []string{"text/html;q=0.9;allow-utf8-labelnames=true"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "complex Accept header", @@ -107,24 +129,27 @@ func Test_parseClientCapabilities(t *testing.T) { "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8;allow-utf8-labelnames=true", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "multiple Accept header entries", Header: http.Header{ "Accept": []string{ "application/json", - "text/plain; allow-utf8-labelnames=true", - "*/*; q=0.1", + "text/plain;allow-utf8-labelnames=true", + "*/*;q=0.1", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "invalid mime type in Accept header", Header: http.Header{ "Accept": []string{"invalid/mime/type/format"}, }, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, Want: ClientCapabilities{}, WantError: true, ErrorMessage: "mime: unexpected content after media subtype", @@ -132,8 +157,9 @@ func Test_parseClientCapabilities(t *testing.T) { { Name: "Accept header with invalid syntax", Header: http.Header{ - "Accept": []string{"text/html; invalid-parameter-syntax"}, + "Accept": []string{"text/html;invalid-parameter-syntax"}, }, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, Want: ClientCapabilities{}, WantError: true, ErrorMessage: "mime: invalid media parameter", @@ -146,6 +172,7 @@ func Test_parseClientCapabilities(t *testing.T) { "invalid/mime/type/format", }, }, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, Want: ClientCapabilities{}, WantError: true, ErrorMessage: "mime: unexpected content after media subtype", @@ -154,16 +181,18 @@ func Test_parseClientCapabilities(t *testing.T) { // Parameter names are case-insensitive in mime.ParseMediaType Name: "case sensitivity test for capability name", Header: http.Header{ - "Accept": []string{"*/*; Allow-Utf8-Labelnames=true"}, + "Accept": []string{"*/*;Allow-Utf8-Labelnames=true"}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "whitespace handling in Accept header", Header: http.Header{ "Accept": []string{" application/json ; allow-utf8-labelnames=true "}, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, } @@ -171,7 +200,13 @@ func Test_parseClientCapabilities(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { t.Parallel() - got, err := parseClientCapabilities(tt.Header) + cfg := tt.Config + if cfg == nil { + cfg = &ClientCapabilityConfig{AllowUtf8LabelNames: true} + } + + logger := log.NewNopLogger() + got, err := parseClientCapabilities(tt.Header, cfg, logger) if tt.WantError { require.Error(t, err) @@ -193,37 +228,41 @@ func Test_parseClientCapabilities_MultipleCapabilities(t *testing.T) { tests := []struct { Name string Header http.Header + Config *ClientCapabilityConfig Want ClientCapabilities }{ { Name: "capability appears multiple times - last true wins", Header: http.Header{ "Accept": []string{ - "application/json; allow-utf8-labelnames=false", - "text/plain; allow-utf8-labelnames=true", + "application/json;allow-utf8-labelnames=false", + "text/plain;allow-utf8-labelnames=true", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "capability appears multiple times - last false loses to earlier true", Header: http.Header{ "Accept": []string{ - "application/json; allow-utf8-labelnames=true", - "text/plain; allow-utf8-labelnames=false", + "application/json;allow-utf8-labelnames=true", + "text/plain;allow-utf8-labelnames=false", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: true}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: true}, }, { Name: "capability appears multiple times - all false", Header: http.Header{ "Accept": []string{ - "application/json; allow-utf8-labelnames=false", - "text/plain; allow-utf8-labelnames=false", + "application/json;allow-utf8-labelnames=false", + "text/plain;allow-utf8-labelnames=false", }, }, - Want: ClientCapabilities{AllowUtf8LabelNames: false}, + Config: &ClientCapabilityConfig{AllowUtf8LabelNames: true}, + Want: ClientCapabilities{AllowUtf8LabelNames: false}, }, } @@ -231,7 +270,13 @@ func Test_parseClientCapabilities_MultipleCapabilities(t *testing.T) { t.Run(tt.Name, func(t *testing.T) { t.Parallel() - got, err := parseClientCapabilities(tt.Header) + cfg := tt.Config + if cfg == nil { + cfg = &ClientCapabilityConfig{AllowUtf8LabelNames: true} + } + + logger := log.NewNopLogger() + got, err := parseClientCapabilities(tt.Header, cfg, logger) require.NoError(t, err) require.Equal(t, tt.Want, got) }) diff --git a/pkg/pyroscope/feature_flags.go b/pkg/pyroscope/feature_flags.go index 7e8872d1b4..81269c3dc7 100644 --- a/pkg/pyroscope/feature_flags.go +++ b/pkg/pyroscope/feature_flags.go @@ -12,7 +12,7 @@ func (c *Config) getFeatureFlags() map[string]bool { featureflags.V2StorageLayer: c.V2, featureflags.PyroscopeRuler: rulerEnabled, featureflags.PyroscopeRulerFunctions: rulerEnabled, - featureflags.UTF8LabelNames: false, // not supported yet + featureflags.UTF8LabelNames: c.ClientCapability.AllowUtf8LabelNames, } } diff --git a/pkg/pyroscope/modules.go b/pkg/pyroscope/modules.go index 0a2db5e030..3c3fb5ca50 100644 --- a/pkg/pyroscope/modules.go +++ b/pkg/pyroscope/modules.go @@ -467,7 +467,7 @@ func (f *Pyroscope) initServer() (services.Service, error) { f.Cfg.Server.ExcludeRequestInLog = true // gRPC-specific. f.Cfg.Server.GRPCMiddleware = append(f.Cfg.Server.GRPCMiddleware, util.RecoveryInterceptorGRPC, - featureflags.ClientCapabilitiesGRPCMiddleware(), + featureflags.ClientCapabilitiesGRPCMiddleware(&f.Cfg.ClientCapability, f.logger), ) if f.Cfg.V2 { @@ -526,7 +526,7 @@ func (f *Pyroscope) initServer() (services.Service, error) { httpMetric, objstoreTracerMiddleware, httputil.K6Middleware(), - featureflags.ClientCapabilitiesHttpMiddleware(), + featureflags.ClientCapabilitiesHttpMiddleware(&f.Cfg.ClientCapability, f.logger), } if f.Cfg.SelfProfiling.UseK6Middleware { defaultHTTPMiddleware = append(defaultHTTPMiddleware, httputil.K6Middleware()) diff --git a/pkg/pyroscope/pyroscope.go b/pkg/pyroscope/pyroscope.go index 03fb7c90b4..383edc5cec 100644 --- a/pkg/pyroscope/pyroscope.go +++ b/pkg/pyroscope/pyroscope.go @@ -41,6 +41,8 @@ import ( "github.com/samber/lo" "google.golang.org/grpc/health" + "github.com/grafana/pyroscope/pkg/featureflags" + "github.com/grafana/pyroscope/pkg/api" apiversion "github.com/grafana/pyroscope/pkg/api/version" "github.com/grafana/pyroscope/pkg/cfg" @@ -80,24 +82,25 @@ import ( ) type Config struct { - Target flagext.StringSliceCSV `yaml:"target,omitempty"` - API api.Config `yaml:"api"` - Server server.Config `yaml:"server,omitempty"` - Distributor distributor.Config `yaml:"distributor,omitempty"` - Querier querier.Config `yaml:"querier,omitempty"` - Frontend frontend.Config `yaml:"frontend,omitempty"` - Worker worker.Config `yaml:"frontend_worker"` - LimitsConfig validation.Limits `yaml:"limits"` - QueryScheduler scheduler.Config `yaml:"query_scheduler"` - Ingester ingester.Config `yaml:"ingester,omitempty"` - StoreGateway storegateway.Config `yaml:"store_gateway,omitempty"` - MemberlistKV memberlist.KVConfig `yaml:"memberlist"` - PhlareDB phlaredb.Config `yaml:"pyroscopedb,omitempty"` - Tracing tracing.Config `yaml:"tracing"` - OverridesExporter exporter.Config `yaml:"overrides_exporter" doc:"hidden"` - RuntimeConfig runtimeconfig.Config `yaml:"runtime_config"` - Compactor compactor.Config `yaml:"compactor"` - TenantSettings settings.Config `yaml:"tenant_settings"` + Target flagext.StringSliceCSV `yaml:"target,omitempty"` + API api.Config `yaml:"api"` + Server server.Config `yaml:"server,omitempty"` + Distributor distributor.Config `yaml:"distributor,omitempty"` + Querier querier.Config `yaml:"querier,omitempty"` + Frontend frontend.Config `yaml:"frontend,omitempty"` + Worker worker.Config `yaml:"frontend_worker"` + LimitsConfig validation.Limits `yaml:"limits"` + QueryScheduler scheduler.Config `yaml:"query_scheduler"` + Ingester ingester.Config `yaml:"ingester,omitempty"` + StoreGateway storegateway.Config `yaml:"store_gateway,omitempty"` + MemberlistKV memberlist.KVConfig `yaml:"memberlist"` + PhlareDB phlaredb.Config `yaml:"pyroscopedb,omitempty"` + Tracing tracing.Config `yaml:"tracing"` + OverridesExporter exporter.Config `yaml:"overrides_exporter" doc:"hidden"` + RuntimeConfig runtimeconfig.Config `yaml:"runtime_config"` + Compactor compactor.Config `yaml:"compactor"` + TenantSettings settings.Config `yaml:"tenant_settings"` + ClientCapability featureflags.ClientCapabilityConfig `yaml:"client_capability"` Storage StorageConfig `yaml:"storage"` SelfProfiling SelfProfilingConfig `yaml:"self_profiling,omitempty"` @@ -201,6 +204,7 @@ func (c *Config) RegisterFlagsWithContext(f *flag.FlagSet) { c.API.RegisterFlags(f) c.EmbeddedGrafana.RegisterFlags(f) c.TenantSettings.RegisterFlags(f) + c.ClientCapability.RegisterFlags(f) } // registerServerFlagsWithChangedDefaultValues registers *Config.Server flags, but overrides some defaults set by the dskit package. diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go index da20976022..2daa3483f9 100644 --- a/pkg/validation/validate.go +++ b/pkg/validation/validate.go @@ -9,6 +9,7 @@ import ( "time" "unicode/utf8" + "github.com/grafana/pyroscope/pkg/featureflags" "github.com/grafana/pyroscope/pkg/pprof" "github.com/go-kit/log/level" @@ -115,7 +116,7 @@ type LabelValidationLimits interface { } // ValidateLabels validates the labels of a profile. -func ValidateLabels(limits LabelValidationLimits, tenantID string, ls []*typesv1.LabelPair) error { +func ValidateLabels(clientCapabilities *featureflags.ClientCapabilities, limits LabelValidationLimits, tenantID string, ls []*typesv1.LabelPair) error { if len(ls) == 0 { return NewErrorf(MissingLabels, MissingLabelsErrorMsg) } @@ -146,7 +147,11 @@ func ValidateLabels(limits LabelValidationLimits, tenantID string, ls []*typesv1 if len(l.Value) > limits.MaxLabelValueLength(tenantID) { return NewErrorf(LabelValueTooLong, LabelValueTooLongErrorMsg, phlaremodel.LabelPairsString(ls), l.Value) } - if origName, newName, ok := SanitizeLegacyLabelName(l.Name); ok && origName != newName { + if clientCapabilities != nil && clientCapabilities.AllowUtf8LabelNames { + if !model.UTF8Validation.IsValidLabelName(l.Name) { + return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label name '"+l.Name+"'") + } + } else if origName, newName, ok := SanitizeLegacyLabelName(l.Name); ok && origName != newName { var err error ls, idx, err = handleSanitizedLabel(ls, idx, origName, newName) if err != nil { diff --git a/pkg/validation/validate_test.go b/pkg/validation/validate_test.go index 5baa5ab2fe..5f941af741 100644 --- a/pkg/validation/validate_test.go +++ b/pkg/validation/validate_test.go @@ -9,6 +9,7 @@ import ( googlev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1" typesv1 "github.com/grafana/pyroscope/api/gen/proto/go/types/v1" + "github.com/grafana/pyroscope/pkg/featureflags" phlaremodel "github.com/grafana/pyroscope/pkg/model" "github.com/grafana/pyroscope/pkg/pprof" ) @@ -136,7 +137,7 @@ func TestValidateLabels(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - err := ValidateLabels(MockLimits{ + err := ValidateLabels(nil, MockLimits{ MaxLabelNamesPerSeriesValue: 4, MaxLabelNameLengthValue: 12, MaxLabelValueLengthValue: 10, @@ -152,6 +153,183 @@ func TestValidateLabels(t *testing.T) { } } +func TestValidateLabels_WithClientCapabilities(t *testing.T) { + for _, tt := range []struct { + name string + clientCapabilities *featureflags.ClientCapabilities + lbs []*typesv1.LabelPair + expectedErr string + expectedReason Reason + expectedLabels []*typesv1.LabelPair // Expected labels after validation/sanitization + }{ + { + name: "UTF-8 labels allowed when capabilities enabled", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: true, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "日本語", Value: "test"}, + {Name: "emoji_🚀", Value: "rocket"}, + }, + // Labels get sorted alphabetically + expectedLabels: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: "emoji_🚀", Value: "rocket"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "日本語", Value: "test"}, + }, + }, + { + name: "UTF-8 labels rejected when capabilities disabled", + clientCapabilities: nil, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "日本語", Value: "test"}, + }, + expectedErr: `invalid labels '{__name__="cpu", service_name="svc", 日本語="test"}' with error: invalid label name '日本語'`, + expectedReason: InvalidLabels, + }, + { + name: "UTF-8 labels rejected when AllowUtf8LabelNames false", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: false, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "café", Value: "test"}, + }, + expectedErr: `invalid labels '{__name__="cpu", café="test", service_name="svc"}' with error: invalid label name 'café'`, + expectedReason: InvalidLabels, + }, + { + name: "valid underscore and hyphen labels with UTF-8 enabled", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: true, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "label_with_underscore", Value: "test"}, + {Name: "label-with-hyphen", Value: "test2"}, + }, + expectedLabels: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: "label-with-hyphen", Value: "test2"}, + {Name: "label_with_underscore", Value: "test"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + }, + }, + { + name: "legacy invalid label names rejected when capabilities disabled", + clientCapabilities: nil, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "123invalid", Value: "test"}, + }, + expectedErr: `invalid labels '{123invalid="test", __name__="cpu", service_name="svc"}' with error: invalid label name '123invalid'`, + expectedReason: InvalidLabels, + }, + { + name: "dots allowed as-is with UTF-8 enabled", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: true, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "app.kubernetes.io/name", Value: "test"}, + }, + // With UTF-8 enabled, dots are allowed (Prometheus UTF-8 validation) + expectedLabels: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: "app.kubernetes.io/name", Value: "test"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + }, + }, + { + name: "dots rejected without UTF-8", + clientCapabilities: nil, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "app.kubernetes.io/name", Value: "test"}, + }, + expectedErr: `invalid labels '{__name__="cpu", app.kubernetes.io/name="test", service_name="svc"}' with error: invalid label name 'app.kubernetes.io/name'`, + expectedReason: InvalidLabels, + }, + { + name: "hyphens allowed with UTF-8 enabled", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: true, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "some-label-name", Value: "test"}, + }, + expectedLabels: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "some-label-name", Value: "test"}, + }, + }, + { + name: "mixed ASCII and UTF-8 labels with capabilities enabled", + clientCapabilities: &featureflags.ClientCapabilities{ + AllowUtf8LabelNames: true, + }, + lbs: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "region", Value: "us-east-1"}, + {Name: "地域", Value: "東京"}, + {Name: "environment", Value: "prod"}, + }, + // Labels get sorted alphabetically + expectedLabels: []*typesv1.LabelPair{ + {Name: model.MetricNameLabel, Value: "cpu"}, + {Name: "environment", Value: "prod"}, + {Name: "region", Value: "us-east-1"}, + {Name: phlaremodel.LabelNameServiceName, Value: "svc"}, + {Name: "地域", Value: "東京"}, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + lbsCopy := make([]*typesv1.LabelPair, len(tt.lbs)) + for i, lb := range tt.lbs { + lbsCopy[i] = &typesv1.LabelPair{Name: lb.Name, Value: lb.Value} + } + + err := ValidateLabels(tt.clientCapabilities, MockLimits{ + MaxLabelNamesPerSeriesValue: 10, + MaxLabelNameLengthValue: 1024, + MaxLabelValueLengthValue: 2048, + }, "test-tenant", lbsCopy) + + if tt.expectedErr != "" { + require.Error(t, err) + require.Equal(t, tt.expectedErr, err.Error()) + require.Equal(t, tt.expectedReason, ReasonOf(err)) + } else { + require.NoError(t, err) + if tt.expectedLabels != nil { + require.Equal(t, len(tt.expectedLabels), len(lbsCopy), "label count mismatch") + for i, expected := range tt.expectedLabels { + require.Equal(t, expected.Name, lbsCopy[i].Name, "label name mismatch at index %d", i) + require.Equal(t, expected.Value, lbsCopy[i].Value, "label value mismatch at index %d", i) + } + } + } + }) + } +} + func Test_ValidateRangeRequest(t *testing.T) { now := model.Now() for _, tt := range []struct {