diff --git a/pkg/reconciler/knativeserving/controller.go b/pkg/reconciler/knativeserving/controller.go index ba5ba556..52c35d3a 100644 --- a/pkg/reconciler/knativeserving/controller.go +++ b/pkg/reconciler/knativeserving/controller.go @@ -22,7 +22,6 @@ import ( "github.com/go-logr/zapr" mf "github.com/jcrossley3/manifestival" "go.uber.org/zap" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/clientcmd" deploymentinformer "knative.dev/pkg/client/injection/kube/informers/apps/v1/deployment" @@ -56,7 +55,7 @@ func NewController( c := &Reconciler{ Base: rbase.NewBase(ctx, controllerAgentName, cmw), knativeServingLister: knativeServingInformer.Lister(), - servings: sets.String{}, + servings: map[string]int64{}, } koDataDir := os.Getenv("KO_DATA_PATH") diff --git a/pkg/reconciler/knativeserving/knativeserving_controller.go b/pkg/reconciler/knativeserving/knativeserving_controller.go index 3092fa37..5184cc69 100644 --- a/pkg/reconciler/knativeserving/knativeserving_controller.go +++ b/pkg/reconciler/knativeserving/knativeserving_controller.go @@ -17,6 +17,7 @@ limitations under the License. package knativeserving import ( + "fmt" "context" mf "github.com/jcrossley3/manifestival" @@ -28,7 +29,7 @@ import ( apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" "knative.dev/pkg/controller" @@ -40,7 +41,10 @@ import ( ) const ( - finalizerName = "delete-knative-serving-manifest" + finalizerName = "delete-knative-serving-manifest" + creationChange = "creation" + editChange = "edit" + deletionChange = "deletion" ) var ( @@ -54,7 +58,7 @@ type Reconciler struct { // Listers index properties about resources knativeServingLister listers.KnativeServingLister config mf.Manifest - servings sets.String + servings map[string]int64 } // Check that our Reconciler implements controller.Reconciler @@ -74,17 +78,33 @@ func (r *Reconciler) Reconcile(ctx context.Context, key string) error { original, err := r.knativeServingLister.KnativeServings(namespace).Get(name) if apierrs.IsNotFound(err) { return nil - } else if err != nil { r.Logger.Error(err, "Error getting KnativeServing") return err } if original.GetDeletionTimestamp() != nil { - r.servings.Delete(key) + if _, ok := r.servings[key]; ok { + delete(r.servings, key) + r.StatsReporter.ReportKnativeservingChange(key, deletionChange) + } return r.delete(original) } - // Keep track of the number of KnativeServings in the cluster - r.servings.Insert(key) + // Keep track of the number and generation of KnativeServings in the cluster. + newGen := original.Generation + if oldGen, ok := r.servings[key]; ok { + if newGen > oldGen { + r.StatsReporter.ReportKnativeservingChange(key, editChange) + } else if newGen < oldGen { + return fmt.Errorf("reconciling obsolete generation of KnativeServing %s: newGen = %d and oldGen = %d", key, newGen, oldGen) + } + } else { + // No metrics are emitted when newGen > 1: the first reconciling of + // a new operator on an existing KnativeServing resource. + if newGen == 1 { + r.StatsReporter.ReportKnativeservingChange(key, creationChange) + } + } + r.servings[key] = newGen // Don't modify the informers copy. knativeServing := original.DeepCopy() @@ -231,7 +251,7 @@ func (r *Reconciler) delete(instance *servingv1alpha1.KnativeServing) error { if len(instance.GetFinalizers()) == 0 || instance.GetFinalizers()[0] != finalizerName { return nil } - if r.servings.Len() == 0 { + if len(r.servings) == 0 { if err := r.config.DeleteAll(&metav1.DeleteOptions{}); err != nil { return err } diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 13006956..8630b2ec 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -62,6 +62,9 @@ type Base struct { // Kubernetes API. Recorder record.EventRecorder + // StatsReporter reports reconciler's metrics. + StatsReporter StatsReporter + // Sugared logger is easier to use but is not as performant as the // raw logger. In performance critical paths, call logger.Desugar() // and use the returned raw logger instead. In addition to the @@ -100,6 +103,12 @@ func NewBase(ctx context.Context, controllerAgentName string, cmw configmap.Watc }() } + // Create metrics reporter + statsReporter, err := NewStatsReporter(controllerAgentName) + if err != nil { + logger.Fatal(err) + } + base := &Base{ KubeClientSet: kubeClient, SharedClientSet: sharedclient.Get(ctx), @@ -107,6 +116,7 @@ func NewBase(ctx context.Context, controllerAgentName string, cmw configmap.Watc DynamicClientSet: dynamicclient.Get(ctx), ConfigMapWatcher: cmw, Recorder: recorder, + StatsReporter: statsReporter, Logger: logger, } diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index e91a95b0..976f7e0b 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -49,4 +49,7 @@ func TestNewBase(t *testing.T) { if r.Recorder == nil { t.Fatal("Expected NewBase to add a Recorder") } + if r.StatsReporter == nil { + t.Fatal("Expected NewBase to add a StatsReporter") + } } diff --git a/pkg/reconciler/stats_reporter.go b/pkg/reconciler/stats_reporter.go new file mode 100644 index 00000000..4cc91736 --- /dev/null +++ b/pkg/reconciler/stats_reporter.go @@ -0,0 +1,100 @@ +/* +Copyright 2019 The Knative 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 reconciler + +import ( + "context" + + "go.opencensus.io/stats" + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" + "knative.dev/pkg/metrics" +) + +const ( + knativeservingChangeCountName = "knativeserving_change_count" +) + +var ( + knativeservingChangeCountStat = stats.Int64( + knativeservingChangeCountName, + "Number of changes to the KnativeServing Custom Resource where a change can be creation, edit, or deletion", + stats.UnitDimensionless) + // Create the tag keys that will be used to add tags to our measurements. + // Tag keys must conform to the restrictions described in + // go.opencensus.io/tag/validate.go. Currently those restrictions are: + // - length between 1 and 255 inclusive + // - characters are printable US-ASCII + reconcilerNameTagKey = tag.MustNewKey("reconciler_name") + knativeservingResourceNameTagKey = tag.MustNewKey("knativeserving_resource_name") + changeTagKey = tag.MustNewKey("change") +) + +func init() { + // Create views to see our measurements. This can return an error if + // a previously-registered view has the same name with a different value. + // View name defaults to the measure name if unspecified. + views := []*view.View{{ + Description: knativeservingChangeCountStat.Description(), + Measure: knativeservingChangeCountStat, + Aggregation: view.Count(), + TagKeys: []tag.Key{reconcilerNameTagKey, knativeservingResourceNameTagKey, changeTagKey}, + }} + + if err := view.Register(views...); err != nil { + panic(err) + } +} + +// StatsReporter defines the interface for sending metrics. +type StatsReporter interface { + // ReportKnativeServingChange reports the count of KnativeServing changes. + ReportKnativeservingChange(knativeservingResourceName, change string) error +} + +// reporter holds cached metric objects to report metrics. +type reporter struct { + reconcilerName string + ctx context.Context +} + +// NewStatsReporter creates a reporter that collects and reports metrics. +func NewStatsReporter(reconcilerName string) (StatsReporter, error) { + // Reconciler tag is static. Create a context containing that and cache it. + ctx, err := tag.New( + context.Background(), + tag.Insert(reconcilerNameTagKey, reconcilerName)) + if err != nil { + return nil, err + } + return &reporter{reconcilerName: reconcilerName, ctx: ctx}, nil +} + +// ReportKnativeServingChange reports the number of changes to the KnativeServing +// Custom Resource where a change can be creation, edit, or deletion. +func (r *reporter) ReportKnativeservingChange(knativeservingResourceName, change string) error { + ctx, err := tag.New( + r.ctx, + tag.Insert(knativeservingResourceNameTagKey, knativeservingResourceName), + tag.Insert(changeTagKey, change)) + if err != nil { + return err + } + + metrics.Record(ctx, knativeservingChangeCountStat.M(1)) + return nil +} diff --git a/pkg/reconciler/stats_reporter_test.go b/pkg/reconciler/stats_reporter_test.go new file mode 100644 index 00000000..c295dc8f --- /dev/null +++ b/pkg/reconciler/stats_reporter_test.go @@ -0,0 +1,65 @@ +/* +Copyright 2019 The Knative 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 reconciler + +import ( + "testing" + + "go.opencensus.io/stats/view" + "go.opencensus.io/tag" + "knative.dev/pkg/metrics/metricstest" +) + +const ( + testReconcilerName = "test_reconciler" + testKnativeservingResourceName = "ns/name" +) + +func TestNewStatsReporter(t *testing.T) { + r, err := NewStatsReporter(testReconcilerName) + if err != nil { + t.Errorf("Failed to create reporter: %v", err) + } + + m := tag.FromContext(r.(*reporter).ctx) + v, ok := m.Value(reconcilerNameTagKey) + if !ok { + t.Fatalf("Expected tag %q", reconcilerNameTagKey) + } + if v != testReconcilerName { + t.Fatalf("Expected %q for tag %q, got %q", testReconcilerName, reconcilerNameTagKey, v) + } +} + +func TestReportKnativeServingChange(t *testing.T) { + r, _ := NewStatsReporter(testReconcilerName) + wantTags := map[string]string{ + reconcilerNameTagKey.Name(): testReconcilerName, + knativeservingResourceNameTagKey.Name(): testKnativeservingResourceName, + changeTagKey.Name(): "creation", + } + countWas := int64(0) + if d, err := view.RetrieveData(knativeservingChangeCountName); err == nil && len(d) == 1 { + countWas = d[0].Data.(*view.CountData).Value + } + + if err := r.ReportKnativeservingChange(testKnativeservingResourceName, "creation"); err != nil { + t.Error(err) + } + + metricstest.CheckCountData(t, knativeservingChangeCountName, wantTags, countWas+1) +}