diff --git a/Dockerfile b/Dockerfile index 1d4bec46014..9914a48dce0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -124,6 +124,7 @@ FROM ${JS_SRC} AS js-src FROM ${BASE_IMAGE} LABEL maintainer="Grafana Labs " +LABEL org.opencontainers.image.source="https://github.com/grafana/grafana" ARG GF_UID="472" ARG GF_GID="0" diff --git a/pkg/infra/metrics/service.go b/pkg/infra/metrics/service.go index 99c1e048d4f..599c2c5ff8d 100644 --- a/pkg/infra/metrics/service.go +++ b/pkg/infra/metrics/service.go @@ -124,11 +124,16 @@ func (g *addPrefixWrapper) Gather() ([]*dto.MetricFamily, error) { var _ prometheus.Gatherer = (*multiRegistry)(nil) type multiRegistry struct { + denyList map[string]struct{} gatherers []prometheus.Gatherer } func NewMultiRegistry(gatherers ...prometheus.Gatherer) *multiRegistry { + denyList := map[string]struct{}{ + "grafana_apiserver_request_slo_duration_seconds_bucket": {}, + } return &multiRegistry{ + denyList: denyList, gatherers: gatherers, } } @@ -143,9 +148,12 @@ func (r *multiRegistry) Gather() (mfs []*dto.MetricFamily, err error) { for i := 0; i < len(mf); i++ { m := mf[i] - _, exists := names[*m.Name] + // skip metrics in the deny list + if _, denied := r.denyList[*m.Name]; denied { + continue + } // prevent duplicate metric names - if exists { + if _, exists := names[*m.Name]; exists { // we can skip go_ and process_ metrics without returning an error // because they are known to be duplicates in both // the k8s and prometheus gatherers. diff --git a/pkg/infra/metrics/service_test.go b/pkg/infra/metrics/service_test.go index 4ee13be81b2..5275ee0ba89 100644 --- a/pkg/infra/metrics/service_test.go +++ b/pkg/infra/metrics/service_test.go @@ -131,6 +131,30 @@ func TestMultiRegistry_Gather(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedMF, mf) }) + + t.Run("denied metrics are not included", func(t *testing.T) { + one.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("grafana_apiserver_request_slo_duration_seconds_bucket")}, + }, nil + } + + two.GatherFunc = func() ([]*dto.MetricFamily, error) { + return []*dto.MetricFamily{ + {Name: strptr("b")}, + {Name: strptr("a")}, + }, nil + } + + expectedMF := []*dto.MetricFamily{ + {Name: strptr("a")}, + {Name: strptr("b")}, + } + + mf, err := g.Gather() + require.NoError(t, err) + require.Equal(t, expectedMF, mf) + }) } type mockGatherer struct { diff --git a/pkg/services/apiserver/aggregator/aggregator.go b/pkg/services/apiserver/aggregator/aggregator.go index 76a446c8990..0945ac83139 100644 --- a/pkg/services/apiserver/aggregator/aggregator.go +++ b/pkg/services/apiserver/aggregator/aggregator.go @@ -33,6 +33,7 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/cache" + "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" "k8s.io/klog/v2" v1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" @@ -266,11 +267,11 @@ func CreateAggregatorServer(config *Config, delegateAPIServer genericapiserver.D } } - metrics := newAvailabilityMetrics() - + registry := legacyregistry.DefaultGatherer.(metrics.KubeRegistry) + availibilityMetrics := newAvailabilityMetrics() // create shared (remote and local) availability metrics // TODO: decouple from legacyregistry - registerIntoLegacyRegistryOnce.Do(func() { err = metrics.Register(legacyregistry.Register, legacyregistry.CustomRegister) }) + registerIntoLegacyRegistryOnce.Do(func() { err = availibilityMetrics.Register(registry.Register, registry.CustomRegister) }) if err != nil { return nil, err } @@ -282,7 +283,7 @@ func CreateAggregatorServer(config *Config, delegateAPIServer genericapiserver.D nil, proxyCurrentCertKeyContentFunc, completedConfig.ExtraConfig.ServiceResolver, - metrics, + availibilityMetrics, ) if err != nil { return nil, err diff --git a/pkg/services/apiserver/aggregator/availableController.go b/pkg/services/apiserver/aggregator/availableController.go index e1935469c97..fb285715f7d 100644 --- a/pkg/services/apiserver/aggregator/availableController.go +++ b/pkg/services/apiserver/aggregator/availableController.go @@ -127,7 +127,10 @@ func NewAvailableConditionController( func (c *AvailableConditionController) sync(key string) error { originalAPIService, err := c.apiServiceLister.Get(key) if apierrors.IsNotFound(err) { - c.metrics.ForgetAPIService(key) + if originalAPIService.Spec.Service != nil { + // Only reset state, if the service was a remote service + c.metrics.ForgetAPIService(key) + } return nil } if err != nil { @@ -201,6 +204,10 @@ func (c *AvailableConditionController) sync(key string) error { results := make(chan error, attempts) for i := 0; i < attempts; i++ { go func() { + // stagger these requests to reduce pressure on aggregated services + waitDuration := time.Second * time.Duration(int32(i)) + time.Sleep(waitDuration) + discoveryURL, err := c.serviceResolver.ResolveEndpoint(apiService.Spec.Service.Namespace, apiService.Spec.Service.Name, *apiService.Spec.Service.Port) if err != nil { results <- err @@ -292,7 +299,10 @@ func (c *AvailableConditionController) sync(key string) error { // apiservices. Doing that means we don't want to quickly issue no-op updates. func (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService, newAPIService *apiregistrationv1.APIService) (*apiregistrationv1.APIService, error) { // update this metric on every sync operation to reflect the actual state - c.metrics.SetUnavailableGauge(newAPIService) + if newAPIService.Spec.Service != nil { + // Only expose the metric for remote services, trusts the type on the new object + c.metrics.SetUnavailableGauge(newAPIService) + } if equality.Semantic.DeepEqual(originalAPIService.Status, newAPIService.Status) { return newAPIService, nil @@ -319,7 +329,10 @@ func (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService return nil, err } - c.metrics.SetUnavailableCounter(originalAPIService, newAPIService) + if newAPIService.Spec.Service != nil { + // Only expose the metric for remote services, trusts the type on the new object + c.metrics.SetUnavailableCounter(originalAPIService, newAPIService) + } return newAPIService, nil } diff --git a/pkg/services/apiserver/client/discovery.go b/pkg/services/apiserver/client/discovery.go new file mode 100644 index 00000000000..bdf2a46a29e --- /dev/null +++ b/pkg/services/apiserver/client/discovery.go @@ -0,0 +1,65 @@ +package client + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/discovery" + "k8s.io/client-go/rest" +) + +type DiscoveryClient interface { + discovery.DiscoveryInterface + GetResourceForKind(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) + GetKindForResource(gvr schema.GroupVersionResource) (schema.GroupVersionKind, error) +} + +type DiscoveryClientImpl struct { + restConfig *rest.Config + discovery.DiscoveryInterface +} + +func NewDiscoveryClient(restConfig *rest.Config) (*DiscoveryClientImpl, error) { + discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig) + if err != nil { + return nil, err + } + return &DiscoveryClientImpl{ + restConfig: restConfig, + DiscoveryInterface: discoveryClient, + }, nil +} + +func (d *DiscoveryClientImpl) GetResourceForKind(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) { + resourceList, err := d.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) + if err != nil { + return schema.GroupVersionResource{}, err + } + for _, resource := range resourceList.APIResources { + if resource.Kind == gvk.Kind { + return schema.GroupVersionResource{ + Group: gvk.Group, + Version: gvk.Version, + Resource: resource.Name, + }, nil + } + } + return schema.GroupVersionResource{}, fmt.Errorf("resource not found for %s", gvk.String()) +} + +func (d *DiscoveryClientImpl) GetKindForResource(gvr schema.GroupVersionResource) (schema.GroupVersionKind, error) { + resourceList, err := d.ServerResourcesForGroupVersion(gvr.GroupVersion().String()) + if err != nil { + return schema.GroupVersionKind{}, err + } + for _, resource := range resourceList.APIResources { + if resource.Name == gvr.Resource { + return schema.GroupVersionKind{ + Group: gvr.Group, + Version: gvr.Version, + Kind: resource.Kind, + }, nil + } + } + return schema.GroupVersionKind{}, fmt.Errorf("kind not found for %s", gvr.String()) +} diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index 6b6a768b992..aad82d9f7f8 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -366,7 +366,7 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d } // Validate folder - if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) { + if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesClientDashboardsFolders) && (dash.FolderID != 0 || dash.FolderUID != "") { // nolint:staticcheck folder, err := dr.folderService.Get(ctx, &folder.GetFolderQuery{ OrgID: dash.OrgID, UID: &dash.FolderUID, diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage.go b/pkg/services/folder/folderimpl/folder_unifiedstorage.go index 92bb8e7180e..18202327590 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage.go @@ -637,10 +637,12 @@ func (s *Service) deleteFromApiServer(ctx context.Context, cmd *folder.DeleteFol return err } - folders := []string{cmd.UID} + folders := []string{} for _, f := range descFolders { folders = append(folders, f.UID) } + // must delete children first, then the parent folder + folders = append(folders, cmd.UID) if cmd.ForceDeleteRules { if err := s.deleteChildrenInFolder(ctx, cmd.OrgID, folders, cmd.SignedInUser); err != nil { diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index 6ed300d80f5..244f11043f8 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -825,7 +825,7 @@ func TestDeleteFoldersFromApiServer(t *testing.T) { { Key: resource.SEARCH_FIELD_FOLDER, Operator: string(selection.In), - Values: []string{"uid", "uid2"}, + Values: []string{"uid2", "uid"}, }, }, }, diff --git a/pkg/storage/unified/client.go b/pkg/storage/unified/client.go index 75febc247e0..7b34b95ab92 100644 --- a/pkg/storage/unified/client.go +++ b/pkg/storage/unified/client.go @@ -6,7 +6,6 @@ import ( "path/filepath" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "gocloud.dev/blob/fileblob" "google.golang.org/grpc" @@ -14,8 +13,6 @@ import ( authnlib "github.com/grafana/authlib/authn" "github.com/grafana/authlib/types" - "github.com/grafana/dskit/grpcclient" - "github.com/grafana/dskit/middleware" infraDB "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" @@ -42,10 +39,6 @@ type Options struct { Docs resource.DocumentBuilderSupplier } -type ClientMetrics struct { - requestDuration *prometheus.HistogramVec -} - // This adds a UnifiedStorage client into the wire dependency tree func ProvideUnifiedStorageClient(opts *Options) (resource.ResourceClient, error) { // See: apiserver.ApplyGrafanaConfig(cfg, features, o) @@ -111,8 +104,11 @@ func newClient(opts options.StorageOptions, return nil, fmt.Errorf("expecting address for storage_type: %s", opts.StorageType) } - // Create a connection to the gRPC server. - conn, err := grpcConn(opts.Address, reg) + // Create a connection to the gRPC server + conn, err := grpc.NewClient(opts.Address, + grpc.WithStatsHandler(otelgrpc.NewClientHandler()), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) if err != nil { return nil, err } @@ -157,41 +153,3 @@ func newResourceClient(conn *grpc.ClientConn, cfg *setting.Cfg, features feature } return resource.NewRemoteResourceClient(tracer, conn, clientCfgMapping(grpcutils.ReadGrpcClientConfig(cfg)), cfg.Env == setting.Dev) } - -// grpcConn creates a new gRPC connection to the provided address. -func grpcConn(address string, reg prometheus.Registerer) (*grpc.ClientConn, error) { - // This works for now as the Provide function is only called once during startup. - // We might eventually want to tie this factory to a struct for more runtime control. - metrics := ClientMetrics{ - requestDuration: promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{ - Name: "resource_server_client_request_duration_seconds", - Help: "Time spent executing requests to the resource server.", - Buckets: prometheus.ExponentialBuckets(0.008, 4, 7), - }, []string{"operation", "status_code"}), - } - - // Report gRPC status code errors as labels. - var instrumentationOptions []middleware.InstrumentationOption - instrumentationOptions = append(instrumentationOptions, middleware.ReportGRPCStatusOption) - unary, stream := grpcclient.Instrument(metrics.requestDuration, instrumentationOptions...) - - // We can later pass in the gRPC config here, i.e. to set MaxRecvMsgSize etc. - cfg := grpcclient.Config{} - opts, err := cfg.DialOption(unary, stream) - if err != nil { - return nil, fmt.Errorf("could not instrument grpc client: %w", err) - } - - opts = append(opts, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) - opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) - - // Use round_robin to balance requests more evenly over the available Storage server. - opts = append(opts, grpc.WithDefaultServiceConfig(`{"loadBalancingPolicy":"round_robin"}`)) - - // Disable looking up service config from TXT DNS records. - // This reduces the number of requests made to the DNS servers. - opts = append(opts, grpc.WithDisableServiceConfig()) - - // Create a connection to the gRPC server - return grpc.NewClient(address, opts...) -} diff --git a/public/locales/de-DE/grafana.json b/public/locales/de-DE/grafana.json index 817b7b49c7b..ec82fb1aab8 100644 --- a/public/locales/de-DE/grafana.json +++ b/public/locales/de-DE/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/es-ES/grafana.json b/public/locales/es-ES/grafana.json index aef7d9c6d99..47b9c17242a 100644 --- a/public/locales/es-ES/grafana.json +++ b/public/locales/es-ES/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/fr-FR/grafana.json b/public/locales/fr-FR/grafana.json index 816cafefcc1..bdd9bbee422 100644 --- a/public/locales/fr-FR/grafana.json +++ b/public/locales/fr-FR/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/pt-BR/grafana.json b/public/locales/pt-BR/grafana.json index 9edeeba05c5..2226b4a5d49 100644 --- a/public/locales/pt-BR/grafana.json +++ b/public/locales/pt-BR/grafana.json @@ -2536,6 +2536,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": "" diff --git a/public/locales/zh-Hans/grafana.json b/public/locales/zh-Hans/grafana.json index ca894dee800..d849cb5e829 100644 --- a/public/locales/zh-Hans/grafana.json +++ b/public/locales/zh-Hans/grafana.json @@ -2526,6 +2526,8 @@ "history-wrapper": { "collapse": "", "expand": "", + "icon-selected": "", + "icon-unselected": "", "show-more": "", "today": "", "yesterday": ""