Simplify the default controller.Impl constructor. (#427)

A while back we added a "StatsReporter" argument to `controller.NewImpl`,
but in serving every callsite of this is passing:
```
controller.NewImpl(r, logger, "my-string", MustNewStatsReporter("my-string", logger))
```

Where `MustNewStatsReporter` is just a form of knative/pkg's `controller.NewStatsReporter`
that logs fatally when an error is returned.  It is notable that Serving's logic has been
duplicated to both Build and Eventing.

There are a handful of changes here:
1. Move MustNewStatsReporter into knative/pkg
2. Expose the current interface as NewImplWithStats
3. Drop the StatsReporter from NewImpl and default to `MustNewStatsReporter()`

This is a breaking change for downstream repositories, but should make their callsites universally simpler.

controller/controller.go

@@ -134,7 +134,11 @@ type Impl struct {
 
 // NewImpl instantiates an instance of our controller that will feed work to the
 // provided Reconciler as it is enqueued.
-func NewImpl(r Reconciler, logger *zap.SugaredLogger, workQueueName string, reporter StatsReporter) *Impl {
+func NewImpl(r Reconciler, logger *zap.SugaredLogger, workQueueName string) *Impl {
+	return NewImplWithStats(r, logger, workQueueName, MustNewStatsReporter(workQueueName, logger))
+}
+
+func NewImplWithStats(r Reconciler, logger *zap.SugaredLogger, workQueueName string, reporter StatsReporter) *Impl {
 	return &Impl{
 		Reconciler: r,
 		WorkQueue: workqueue.NewNamedRateLimitingQueue(

controller/controller_test.go

@@ -481,7 +481,7 @@ func TestEnqueues(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			impl := NewImpl(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
+			impl := NewImplWithStats(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
 			test.work(impl)
 
 			// The rate limit on our queue delays when things are added to the queue.
@@ -497,7 +497,7 @@ func TestEnqueues(t *testing.T) {
 }
 
 func TestEnqeueAfter(t *testing.T) {
-	impl := NewImpl(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
+	impl := NewImplWithStats(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
 	impl.EnqueueAfter(&Resource{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "for",
@@ -532,7 +532,7 @@ func TestEnqeueAfter(t *testing.T) {
 }
 
 func TestEnqeueKeyAfter(t *testing.T) {
-	impl := NewImpl(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
+	impl := NewImplWithStats(&NopReconciler{}, TestLogger(t), "Testing", &FakeStatsReporter{})
 	impl.EnqueueKeyAfter("waiting/for", time.Second)
 	impl.EnqueueKeyAfter("the/waterfall", time.Second>>1)
 	impl.EnqueueKeyAfter("to/fall", time.Second<<1)
@@ -565,7 +565,7 @@ func (cr *CountingReconciler) Reconcile(context.Context, string) error {
 
 func TestStartAndShutdown(t *testing.T) {
 	r := &CountingReconciler{}
-	impl := NewImpl(r, TestLogger(t), "Testing", &FakeStatsReporter{})
+	impl := NewImplWithStats(r, TestLogger(t), "Testing", &FakeStatsReporter{})
 
 	stopCh := make(chan struct{})
 	doneCh := make(chan struct{})
@@ -598,7 +598,7 @@ func TestStartAndShutdown(t *testing.T) {
 func TestStartAndShutdownWithWork(t *testing.T) {
 	r := &CountingReconciler{}
 	reporter := &FakeStatsReporter{}
-	impl := NewImpl(r, TestLogger(t), "Testing", reporter)
+	impl := NewImplWithStats(r, TestLogger(t), "Testing", reporter)
 
 	stopCh := make(chan struct{})
 	doneCh := make(chan struct{})
@@ -644,7 +644,7 @@ func (er *ErrorReconciler) Reconcile(context.Context, string) error {
 func TestStartAndShutdownWithErroringWork(t *testing.T) {
 	r := &ErrorReconciler{}
 	reporter := &FakeStatsReporter{}
-	impl := NewImpl(r, TestLogger(t), "Testing", reporter)
+	impl := NewImplWithStats(r, TestLogger(t), "Testing", reporter)
 
 	stopCh := make(chan struct{})
 	doneCh := make(chan struct{})
@@ -692,7 +692,7 @@ func (er *PermanentErrorReconciler) Reconcile(context.Context, string) error {
 func TestStartAndShutdownWithPermanentErroringWork(t *testing.T) {
 	r := &PermanentErrorReconciler{}
 	reporter := &FakeStatsReporter{}
-	impl := NewImpl(r, TestLogger(t), "Testing", reporter)
+	impl := NewImplWithStats(r, TestLogger(t), "Testing", reporter)
 
 	stopCh := make(chan struct{})
 	doneCh := make(chan struct{})
@@ -763,7 +763,7 @@ func (*dummyStore) List() []interface{} {
 
 func TestImplGlobalResync(t *testing.T) {
 	r := &CountingReconciler{}
-	impl := NewImpl(r, TestLogger(t), "Testing", &FakeStatsReporter{})
+	impl := NewImplWithStats(r, TestLogger(t), "Testing", &FakeStatsReporter{})
 
 	stopCh := make(chan struct{})
 	doneCh := make(chan struct{})

controller/stats_reporter.go

@@ -25,6 +25,7 @@ import (
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
 	"go.opencensus.io/tag"
+	"go.uber.org/zap"
 )
 
 var (
@@ -103,6 +104,16 @@ func NewStatsReporter(reconciler string) (StatsReporter, error) {
 	return &reporter{reconciler: reconciler, globalCtx: ctx}, nil
 }
 
+// MustNewStatsReporter creates a new instance of StatsReporter.
+// Logs fatally if creation fails.
+func MustNewStatsReporter(reconciler string, logger *zap.SugaredLogger) StatsReporter {
+	stats, err := NewStatsReporter(reconciler)
+	if err != nil {
+		logger.Fatalw("Failed to initialize the stats reporter", zap.Error(err))
+	}
+	return stats
+}
+
 // ReportQueueDepth reports the queue depth metric
 func (r *reporter) ReportQueueDepth(v int64) error {
 	if r.globalCtx == nil {