Merge pull request #52867 from liggitt/metric-name

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Remove GC ratelimiter metrics

Fixes #52801

The metrics were causing panics because the names weren't formatted correctly, there were concerns around the number of metrics series that resulted from registration per group/version, and the saturation metric being reported was of [questionable use](https://go-review.googlesource.com/c/time/+/29958#message-4caffc11669cadd90e2da4c05122cfec50ea6a22)
This commit is contained in:
Kubernetes Submit Queue 2017-09-21 18:47:05 -07:00 committed by GitHub
commit cec8acf5e5
5 changed files with 21 additions and 85 deletions

View File

@ -15,13 +15,11 @@ go_library(
"graph_builder.go",
"operations.go",
"patch.go",
"rate_limiter_helper.go",
"uid_cache.go",
],
deps = [
"//pkg/controller:go_default_library",
"//pkg/controller/garbagecollector/metaonly:go_default_library",
"//pkg/util/metrics:go_default_library",
"//pkg/util/reflector/prometheus:go_default_library",
"//pkg/util/workqueue/prometheus:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",

View File

@ -69,9 +69,6 @@ type GarbageCollector struct {
// garbage collector attempts to orphan the dependents of the items in the attemptToOrphan queue, then deletes the items.
attemptToOrphan workqueue.RateLimitingInterface
dependencyGraphBuilder *GraphBuilder
// used to register exactly once the rate limiter of the dynamic client
// used by the garbage collector controller.
registeredRateLimiter *RegisteredRateLimiter
// GC caches the owners that do not exist according to the API server.
absentOwnerCache *UIDCache
sharedInformers informers.SharedInformerFactory
@ -92,19 +89,17 @@ func NewGarbageCollector(
attemptToOrphan := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_attempt_to_orphan")
absentOwnerCache := NewUIDCache(500)
gc := &GarbageCollector{
clientPool: clientPool,
restMapper: mapper,
attemptToDelete: attemptToDelete,
attemptToOrphan: attemptToOrphan,
registeredRateLimiter: NewRegisteredRateLimiter(deletableResources),
absentOwnerCache: absentOwnerCache,
clientPool: clientPool,
restMapper: mapper,
attemptToDelete: attemptToDelete,
attemptToOrphan: attemptToOrphan,
absentOwnerCache: absentOwnerCache,
}
gb := &GraphBuilder{
metaOnlyClientPool: metaOnlyClientPool,
informersStarted: informersStarted,
registeredRateLimiterForControllers: NewRegisteredRateLimiter(deletableResources),
restMapper: mapper,
graphChanges: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_graph_changes"),
metaOnlyClientPool: metaOnlyClientPool,
informersStarted: informersStarted,
restMapper: mapper,
graphChanges: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "garbage_collector_graph_changes"),
uidToNode: &concurrentUIDToNode{
uidToNode: make(map[types.UID]*node),
},

View File

@ -92,9 +92,6 @@ type GraphBuilder struct {
// metaOnlyClientPool uses a special codec, which removes fields except for
// apiVersion, kind, and metadata during decoding.
metaOnlyClientPool dynamic.ClientPool
// used to register exactly once the rate limiters of the clients used by
// the `monitors`.
registeredRateLimiterForControllers *RegisteredRateLimiter
// monitors are the producer of the graphChanges queue, graphBuilder alters
// the in-memory graph according to the changes.
graphChanges workqueue.RateLimitingInterface
@ -204,8 +201,6 @@ func (gb *GraphBuilder) controllerFor(resource schema.GroupVersionResource, kind
if err != nil {
return nil, err
}
// TODO: since the gv is never unregistered, isn't this a memory leak?
gb.registeredRateLimiterForControllers.registerIfNotPresent(resource.GroupVersion(), client, "garbage_collector_monitoring")
_, monitor := cache.NewInformer(
listWatcher(client, resource),
nil,

View File

@ -50,7 +50,9 @@ func (gc *GarbageCollector) apiResource(apiVersion, kind string, namespaced bool
func (gc *GarbageCollector) deleteObject(item objectReference, policy *metav1.DeletionPropagation) error {
fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind)
client, err := gc.clientPool.ClientForGroupVersionKind(fqKind)
gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation")
if err != nil {
return err
}
resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
if err != nil {
return err
@ -64,7 +66,9 @@ func (gc *GarbageCollector) deleteObject(item objectReference, policy *metav1.De
func (gc *GarbageCollector) getObject(item objectReference) (*unstructured.Unstructured, error) {
fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind)
client, err := gc.clientPool.ClientForGroupVersionKind(fqKind)
gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation")
if err != nil {
return nil, err
}
resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
if err != nil {
return nil, err
@ -75,7 +79,9 @@ func (gc *GarbageCollector) getObject(item objectReference) (*unstructured.Unstr
func (gc *GarbageCollector) updateObject(item objectReference, obj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind)
client, err := gc.clientPool.ClientForGroupVersionKind(fqKind)
gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation")
if err != nil {
return nil, err
}
resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
if err != nil {
return nil, err
@ -86,7 +92,9 @@ func (gc *GarbageCollector) updateObject(item objectReference, obj *unstructured
func (gc *GarbageCollector) patchObject(item objectReference, patch []byte) (*unstructured.Unstructured, error) {
fqKind := schema.FromAPIVersionAndKind(item.APIVersion, item.Kind)
client, err := gc.clientPool.ClientForGroupVersionKind(fqKind)
gc.registeredRateLimiter.registerIfNotPresent(fqKind.GroupVersion(), client, "garbage_collector_operation")
if err != nil {
return nil, err
}
resource, err := gc.apiResource(item.APIVersion, item.Kind, len(item.Namespace) != 0)
if err != nil {
return nil, err

View File

@ -1,60 +0,0 @@
/*
Copyright 2016 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package garbagecollector
import (
"fmt"
"strings"
"sync"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/dynamic"
"k8s.io/kubernetes/pkg/util/metrics"
)
// RegisteredRateLimiter records the registered RateLimters to avoid
// duplication.
type RegisteredRateLimiter struct {
rateLimiters map[schema.GroupVersion]*sync.Once
}
// NewRegisteredRateLimiter returns a new RegisteredRateLimiter.
// TODO: NewRegisteredRateLimiter is not dynamic. We need to find a better way
// when GC dynamically change the resources it monitors.
func NewRegisteredRateLimiter(resources map[schema.GroupVersionResource]struct{}) *RegisteredRateLimiter {
rateLimiters := make(map[schema.GroupVersion]*sync.Once)
for resource := range resources {
gv := resource.GroupVersion()
if _, found := rateLimiters[gv]; !found {
rateLimiters[gv] = &sync.Once{}
}
}
return &RegisteredRateLimiter{rateLimiters: rateLimiters}
}
func (r *RegisteredRateLimiter) registerIfNotPresent(gv schema.GroupVersion, client dynamic.Interface, prefix string) {
once, found := r.rateLimiters[gv]
if !found {
return
}
once.Do(func() {
if rateLimiter := client.GetRateLimiter(); rateLimiter != nil {
group := strings.Replace(gv.Group, ".", ":", -1)
metrics.RegisterMetricAndTrackRateLimiterUsage(fmt.Sprintf("%s_%s_%s", prefix, group, gv.Version), rateLimiter)
}
})
}