Skip to content

Commit

Permalink
[DEVHAS-481] Add Application Creation metrics (#399)
Browse files Browse the repository at this point in the history
Signed-off-by: John Collier <[email protected]>
  • Loading branch information
johnmcollier authored Oct 12, 2023
1 parent 2a15ddd commit 4ec3ad6
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 1 deletion.
5 changes: 5 additions & 0 deletions controllers/application_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// If devfile hasn't been generated yet, generate it
// If the devfile hasn't been generated, the CR was just created.
if application.Status.Devfile == "" {
metrics.ApplicationCreationTotalReqs.Inc()
// See if a gitops/appModel repo(s) were passed in. If not, generate them.
gitOpsRepo := application.Spec.GitOpsRepository.URL
appModelRepo := application.Spec.AppModelRepository.URL
Expand All @@ -165,6 +166,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
repoUrl, err := ghClient.GenerateNewRepository(ctx, r.GitHubOrg, repoName, "GitOps Repository")
if err != nil {
metrics.HandleRateLimitMetrics(err, metricsLabel)
metrics.ApplicationCreationFailed.Inc()
log.Error(err, fmt.Sprintf("Unable to create repository %v", repoUrl))
r.SetCreateConditionAndUpdateCR(ctx, req, &application, err)
return reconcile.Result{}, err
Expand All @@ -180,6 +182,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// Convert the devfile string to a devfile object
devfileData, err := devfile.ConvertApplicationToDevfile(application, gitOpsRepo, appModelRepo)
if err != nil {
metrics.ApplicationCreationFailed.Inc()
log.Error(err, fmt.Sprintf("Unable to convert Application CR to devfile, exiting reconcile loop %v", req.NamespacedName))
r.SetCreateConditionAndUpdateCR(ctx, req, &application, err)
return reconcile.Result{}, err
Expand All @@ -195,6 +198,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)

yamlData, err := yaml.Marshal(devfileData)
if err != nil {
metrics.ApplicationCreationFailed.Inc()
log.Error(err, fmt.Sprintf("Unable to marshall Application devfile, exiting reconcile loop %v", req.NamespacedName))
r.SetCreateConditionAndUpdateCR(ctx, req, &application, err)
return reconcile.Result{}, err
Expand All @@ -204,6 +208,7 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)

// Create GitOps repository
// Update the status of the CR
metrics.ApplicationCreationSucceeded.Inc()
r.SetCreateConditionAndUpdateCR(ctx, req, &application, nil)
} else {
// If the model already exists, see if either the displayname or description need updating
Expand Down
17 changes: 17 additions & 0 deletions controllers/application_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ import (
"time"

"github.com/devfile/library/v2/pkg/devfile/parser"
"github.com/prometheus/client_golang/prometheus/testutil"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
appstudiov1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
cdqanalysis "github.com/redhat-appstudio/application-service/cdq-analysis/pkg"
"github.com/redhat-appstudio/application-service/pkg/metrics"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
//+kubebuilder:scaffold:imports
Expand All @@ -51,6 +53,9 @@ var _ = Describe("Application controller", func() {

Context("Create Application with no repositories set", func() {
It("Should create successfully with generated repositories", func() {
beforeCreateTotalReqs := testutil.ToFloat64(metrics.ApplicationCreationTotalReqs)
beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ApplicationCreationSucceeded)
beforeCreateFailedReqs := testutil.ToFloat64(metrics.ApplicationCreationFailed)
ctx := context.Background()

applicationName := HASAppName + "1"
Expand Down Expand Up @@ -91,6 +96,11 @@ var _ = Describe("Application controller", func() {
Expect(string(devfile.GetMetadata().Attributes["gitOpsRepository.url"].Raw)).Should(Not(Equal("")))
Expect(string(devfile.GetMetadata().Attributes["appModelRepository.url"].Raw)).Should(Not(Equal("")))

// Success metrics should get counted here
Expect(testutil.ToFloat64(metrics.ApplicationCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue())
Expect(testutil.ToFloat64(metrics.ApplicationCreationSucceeded) > beforeCreateSucceedReqs).To(BeTrue())
Expect(testutil.ToFloat64(metrics.ApplicationCreationFailed) == beforeCreateFailedReqs).To(BeTrue())

// Delete the specified resource
deleteHASAppCR(hasAppLookupKey)
})
Expand Down Expand Up @@ -310,6 +320,9 @@ var _ = Describe("Application controller", func() {

Context("Application CR with gitops repo creation failure", func() {
It("Should update successfully with updated description", func() {
beforeCreateTotalReqs := testutil.ToFloat64(metrics.ApplicationCreationTotalReqs)
beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ApplicationCreationSucceeded)
beforeCreateFailedReqs := testutil.ToFloat64(metrics.ApplicationCreationFailed)
ctx := context.Background()

applicationName := "test-error-response" + "5"
Expand Down Expand Up @@ -345,6 +358,10 @@ var _ = Describe("Application controller", func() {

Expect(fetchedHasApp.Status.Conditions[0].Status).Should(Equal(metav1.ConditionFalse))

Expect(testutil.ToFloat64(metrics.ApplicationCreationTotalReqs) > beforeCreateTotalReqs).To(BeTrue())
Expect(testutil.ToFloat64(metrics.ApplicationCreationSucceeded) == beforeCreateSucceedReqs).To(BeTrue())
Expect(testutil.ToFloat64(metrics.ApplicationCreationFailed) > beforeCreateFailedReqs).To(BeTrue())

// Delete the specified resource
deleteHASAppCR(hasAppLookupKey)
})
Expand Down
4 changes: 4 additions & 0 deletions controllers/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
appstudiov1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
cdqanalysis "github.com/redhat-appstudio/application-service/cdq-analysis/pkg"
devfile "github.com/redhat-appstudio/application-service/pkg/devfile"
"github.com/redhat-appstudio/application-service/pkg/metrics"
"github.com/redhat-appstudio/application-service/pkg/util"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -286,6 +287,7 @@ func (r *ApplicationReconciler) getAndAddComponentApplicationsToModel(log logr.L
})
if err != nil {
log.Error(err, fmt.Sprintf("Unable to list Components for %v", req.NamespacedName))
metrics.ApplicationCreationFailed.Inc()
return err
}

Expand All @@ -298,6 +300,8 @@ func (r *ApplicationReconciler) getAndAddComponentApplicationsToModel(log logr.L
// Add the components to the Devfile model
err = r.addComponentsToApplicationDevfileModel(devSpec, components)
if err != nil {
// User error - so increment the "success" metric - since we're tracking only system errors
metrics.ApplicationCreationSucceeded.Inc()
log.Error(err, fmt.Sprintf("Error adding components to devfile for Application %v", req.NamespacedName))
return err
}
Expand Down
15 changes: 15 additions & 0 deletions controllers/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import (
"github.com/devfile/api/v2/pkg/devfile"
v2 "github.com/devfile/library/v2/pkg/devfile/parser/data/v2"
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
"github.com/prometheus/client_golang/prometheus/testutil"
appstudiov1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
devfilePkg "github.com/redhat-appstudio/application-service/pkg/devfile"
"github.com/redhat-appstudio/application-service/pkg/metrics"
"github.com/stretchr/testify/assert"
"go.uber.org/zap/zapcore"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -516,6 +518,9 @@ func TestGetAndAddComponentApplicationsToModel(t *testing.T) {
fakeClient = NewFakeClient(t, tt.componentTwo)
}

beforeCreateSucceedReqs := testutil.ToFloat64(metrics.ApplicationCreationSucceeded)
beforeCreateFailedReqs := testutil.ToFloat64(metrics.ApplicationCreationFailed)

devSpec := devfileAPIV1.DevWorkspaceTemplateSpec{
DevWorkspaceTemplateSpecContent: devfileAPIV1.DevWorkspaceTemplateSpecContent{
Attributes: tt.attributes,
Expand All @@ -534,6 +539,16 @@ func TestGetAndAddComponentApplicationsToModel(t *testing.T) {
t.Errorf("TestGetAndAddComponentApplicationsToModel() unexpected error: %v", err)
}

if err != nil && tt.name == "Container image already exists" {
// This is a user error scenario, so expect the "creation success" metric to be incremented (as only system errors are counted for failure)
if testutil.ToFloat64(metrics.ApplicationCreationSucceeded) <= beforeCreateSucceedReqs {
t.Errorf("TestGetAndAddComponentApplicationsToModel() expected metric 'ApplicationCreationSucceeded' to be incremented")
}
if testutil.ToFloat64(metrics.ApplicationCreationFailed) != beforeCreateFailedReqs {
t.Errorf("TestGetAndAddComponentApplicationsToModel() expected metric 'ApplicationCreationFailed' to be unchanged")
}
}

components := []appstudiov1alpha1.Component{}
if tt.componentOne != nil {
components = append(components, *tt.componentOne)
Expand Down
22 changes: 21 additions & 1 deletion pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,26 @@ var (
},
)

ApplicationCreationTotalReqs = prometheus.NewCounter(
prometheus.CounterOpts{
Name: "has_application_creation_total",
Help: "Number of application creation requests processed",
},
)
ApplicationCreationFailed = prometheus.NewCounter(
prometheus.CounterOpts{
Name: "has_application_failed_creation_total",
Help: "Number of failed application creation requests",
},
)

ApplicationCreationSucceeded = prometheus.NewCounter(
prometheus.CounterOpts{
Name: "has_application_successful_creation_total",
Help: "Number of successful application creation requests",
},
)

ComponentDeletionTotalReqs = prometheus.NewCounter(
prometheus.CounterOpts{
Name: "has_component_deletion_total",
Expand Down Expand Up @@ -142,7 +162,7 @@ func init() {
// Register custom metrics with the global prometheus registry
metrics.Registry.MustRegister(GitOpsRepoCreationTotalReqs, GitOpsRepoCreationFailed, GitOpsRepoCreationSucceeded, ControllerGitRequest, SecondaryRateLimitCounter,
PrimaryRateLimitCounter, TokenPoolGauge, ApplicationDeletionTotalReqs, ApplicationDeletionSucceeded, ApplicationDeletionFailed,
ComponentDeletionTotalReqs, ComponentDeletionSucceeded, ComponentDeletionFailed,
ApplicationCreationSucceeded, ApplicationCreationFailed, ApplicationCreationTotalReqs, ComponentDeletionTotalReqs, ComponentDeletionSucceeded, ComponentDeletionFailed,
ImportGitRepoTotalReqs, ImportGitRepoFailed, ImportGitRepoSucceeded)
}

Expand Down

0 comments on commit 4ec3ad6

Please sign in to comment.