Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metrics to controllers tracking reconciliations #154

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- [PR #138](https://github.com/konpyutaika/nifikop/pull/138) - **[Operator/NifiCluster]** Add ability to configure the NiFi Load Balance port.
- [PR #144](https://github.com/konpyutaika/nifikop/pull/144) - **[Operator]** Add automatic detection of k8s prior 1.21.
- [PR #152](https://github.com/konpyutaika/nifikop/pull/152) - **[Operator]** Added Prometheus metrics for controller reconciliations.

### Changed

Expand Down
29 changes: 20 additions & 9 deletions controllers/controller_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"time"

"github.com/go-logr/logr"
"github.com/konpyutaika/nifikop/pkg/metrics"
"github.com/konpyutaika/nifikop/pkg/util/clientconfig"
"go.uber.org/zap"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -27,25 +28,35 @@ var ClusterRefLabel = "nifiCluster"

var getGvk = apiutil.GVKForObject

// IntrumentedReconciler is an interface all of the controllers extend to benefit from metric tracking
type InstrumentedReconciler interface {
Metrics() *metrics.MetricRegistry
Log() *zap.Logger
}

// requeueWithError is a convenience wrapper around logging an error message
// separate from the stacktrace and then passing the error through to the controller
// manager
func RequeueWithError(logger zap.Logger, msg string, err error) (reconcile.Result, error) {
logger.Info(msg)
func RequeueWithError(r InstrumentedReconciler, msg string, err error) (reconcile.Result, error) {
r.Metrics().ReconcileErrorsCounter().Inc()
r.Log().Info(msg)
return reconcile.Result{}, err
}

func Requeue() (reconcile.Result, error) {
func Requeue(r InstrumentedReconciler) (reconcile.Result, error) {
r.Metrics().ReconcileCounter().Inc()
return reconcile.Result{Requeue: true}, nil
}

func RequeueAfter(time time.Duration) (reconcile.Result, error) {
func RequeueAfter(r InstrumentedReconciler, time time.Duration) (reconcile.Result, error) {
r.Metrics().ReconcileCounter().Inc()
return reconcile.Result{Requeue: true, RequeueAfter: time}, nil
}

// reconciled returns an empty result with nil error to signal a successful reconcile
// to the controller manager
func Reconciled() (reconcile.Result, error) {
func Reconciled(r InstrumentedReconciler) (reconcile.Result, error) {
r.Metrics().ReconcileCounter().Inc()
return reconcile.Result{}, nil
}

Expand All @@ -55,8 +66,8 @@ func ClusterLabelString(cluster *v1alpha1.NifiCluster) string {
}

// checkNodeConnectionError is a convenience wrapper for returning from common
// node connection errors
func CheckNodeConnectionError(logger zap.Logger, err error) (ctrl.Result, error) {
// node connection errors. This is only used in testing.
func checkNodeConnectionError(r InstrumentedReconciler, err error) (ctrl.Result, error) {
switch errors.Cause(err).(type) {
case errorfactory.NodesUnreachable:
return ctrl.Result{
Expand All @@ -69,13 +80,13 @@ func CheckNodeConnectionError(logger zap.Logger, err error) (ctrl.Result, error)
RequeueAfter: time.Duration(15) * time.Second,
}, nil
case errorfactory.ResourceNotReady:
logger.Info("Needed resource for node connection not found, may not be ready")
r.Log().Info("Needed resource for node connection not found, may not be ready")
return ctrl.Result{
Requeue: true,
RequeueAfter: time.Duration(5) * time.Second,
}, nil
default:
return RequeueWithError(logger, err.Error(), err)
return RequeueWithError(r, err.Error(), err)
}
}

Expand Down
31 changes: 21 additions & 10 deletions controllers/controller_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,32 @@ import (

"github.com/konpyutaika/nifikop/api/v1alpha1"
"github.com/konpyutaika/nifikop/pkg/errorfactory"
"github.com/konpyutaika/nifikop/pkg/metrics"
"github.com/prometheus/client_golang/prometheus"
)

func TestRequeueWithError(t *testing.T) {
logger, _ := zap.NewDevelopment()
type mockReconciler struct {
InstrumentedReconciler
}

func (m *mockReconciler) Metrics() *metrics.MetricRegistry {
reg := prometheus.NewRegistry()
return metrics.NewMetrics("foo", reg)
}

_, err := RequeueWithError(*logger, "test", errors.New("test error"))
func (m *mockReconciler) Log() *zap.Logger {
return zap.NewNop()
}

func TestRequeueWithError(t *testing.T) {
_, err := RequeueWithError(&mockReconciler{}, "test", errors.New("test error"))
if err == nil {
t.Error("Expected error to fall through, got nil")
}
}

func TestReconciled(t *testing.T) {
res, err := Reconciled()
res, err := Reconciled(&mockReconciler{})
if err != nil {
t.Error("Expected error to be nil, got:", err)
}
Expand Down Expand Up @@ -90,9 +103,7 @@ func TestCheckNodeConnectionError(t *testing.T) {

// Test nodes unreachable
err = errorfactory.New(errorfactory.NodesUnreachable{}, errors.New("test error"), "test message")
logger, _ := zap.NewDevelopment()

if res, err := CheckNodeConnectionError(*logger, err); err != nil {
if res, err := checkNodeConnectionError(&mockReconciler{}, err); err != nil {
t.Error("Expected no error in result, got:", err)
} else {
if !res.Requeue {
Expand All @@ -105,7 +116,7 @@ func TestCheckNodeConnectionError(t *testing.T) {

// Test nodes not ready
err = errorfactory.New(errorfactory.NodesNotReady{}, errors.New("test error"), "test message")
if res, err := CheckNodeConnectionError(*logger, err); err != nil {
if res, err := checkNodeConnectionError(&mockReconciler{}, err); err != nil {
t.Error("Expected no error in result, got:", err)
} else {
if !res.Requeue {
Expand All @@ -118,7 +129,7 @@ func TestCheckNodeConnectionError(t *testing.T) {

// test external resource not ready
err = errorfactory.New(errorfactory.ResourceNotReady{}, errors.New("test error"), "test message")
if res, err := CheckNodeConnectionError(*logger, err); err != nil {
if res, err := checkNodeConnectionError(&mockReconciler{}, err); err != nil {
t.Error("Expected no error in result, got:", err)
} else {
if !res.Requeue {
Expand All @@ -131,7 +142,7 @@ func TestCheckNodeConnectionError(t *testing.T) {

// test default response
err = errorfactory.New(errorfactory.InternalError{}, errors.New("test error"), "test message")
if _, err := CheckNodeConnectionError(*logger, err); err == nil {
if _, err := checkNodeConnectionError(&mockReconciler{}, err); err == nil {
t.Error("Expected error to fall through, got nil")
}
}
Expand Down
Loading