Skip to content

Commit

Permalink
fix(operator): fix calculation of deployment interval metrics (#822)
Browse files Browse the repository at this point in the history 
Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
  • Loading branch information
@bacherfl
bacherfl committed Feb 13, 2023
1 parent c90db1e commit a798eed
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 83 deletions.
3 changes: 3 additions & 0 deletions operator/apis/lifecycle/v1alpha2/keptnappversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,9 @@ func TestKeptnAppVersionList(t *testing.T) {
got := list.GetItems()
require.Len(t, got, 2)

require.Equal(t, "obj1", list.Items[0].GetName())
require.Equal(t, "obj2", list.Items[1].GetName())

// remove deprecated items from the list
list.RemoveDeprecated()

Expand Down
10 changes: 5 additions & 5 deletions operator/apis/lifecycle/v1alpha2/keptnappversion_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ type KeptnAppVersionList struct {

func (a KeptnAppVersionList) GetItems() []client.Object {
b := make([]client.Object, 0, len(a.Items))
for _, i := range a.Items {
b = append(b, &i)
for i := 0; i < len(a.Items); i++ {
b = append(b, &a.Items[i])
}
return b
}

func (a *KeptnAppVersionList) RemoveDeprecated() {
b := make([]KeptnAppVersion, 0, len(a.Items))
for _, i := range a.Items {
if i.Status.Status != common.StateDeprecated {
b = append(b, i)
for i := 0; i < len(a.Items); i++ {
if a.Items[i].Status.Status != common.StateDeprecated {
b = append(b, a.Items[i])
}
}
a.Items = b
Expand Down
2 changes: 2 additions & 0 deletions operator/apis/lifecycle/v1alpha2/keptnworkloadinstance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,4 +405,6 @@ func TestKeptnWorkloadInstanceList(t *testing.T) {

got := list.GetItems()
require.Len(t, got, 2)
require.Equal(t, "obj1", got[0].GetName())
require.Equal(t, "obj2", got[1].GetName())
}
4 changes: 2 additions & 2 deletions operator/apis/lifecycle/v1alpha2/keptnworkloadinstance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ func init() {

func (w KeptnWorkloadInstanceList) GetItems() []client.Object {
var b []client.Object
for _, i := range w.Items {
b = append(b, &i)
for i := 0; i < len(w.Items); i++ {
b = append(b, &w.Items[i])
}
return b
}
Expand Down
44 changes: 31 additions & 13 deletions operator/controllers/common/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package common
import (
"context"
"fmt"
"strings"

controllererrors "github.com/keptn/lifecycle-toolkit/operator/controllers/errors"
"github.com/keptn/lifecycle-toolkit/operator/controllers/lifecycle/interfaces"
"go.opentelemetry.io/otel/metric/instrument/asyncfloat64"
"go.opentelemetry.io/otel/metric/instrument/asyncint64"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -34,7 +34,7 @@ func ObserveDeploymentDuration(ctx context.Context, client client.Client, reconc
return nil
}

func ObserveDeploymentInterval(ctx context.Context, client client.Client, reconcileObjectList client.ObjectList, previousObject client.Object, gauge asyncfloat64.Gauge) error {
func ObserveDeploymentInterval(ctx context.Context, client client.Client, reconcileObjectList client.ObjectList, gauge asyncfloat64.Gauge) error {
err := client.List(ctx, reconcileObjectList)
if err != nil {
return fmt.Errorf(controllererrors.ErrCannotRetrieveInstancesMsg, err)
Expand All @@ -45,27 +45,45 @@ func ObserveDeploymentInterval(ctx context.Context, client client.Client, reconc
return err
}

for _, ro := range piWrapper.GetItems() {
items := piWrapper.GetItems()
for index := 0; index < len(items); index++ {
ro := items[index]
reconcileObject, _ := interfaces.NewMetricsObjectWrapperFromClientObject(ro)
if reconcileObject.GetPreviousVersion() != "" {
err := client.Get(ctx, types.NamespacedName{Name: fmt.Sprintf("%s-%s", reconcileObject.GetParentName(), reconcileObject.GetPreviousVersion()), Namespace: reconcileObject.GetNamespace()}, previousObject)
if err != nil {
return nil
}
piWrapper2, err := interfaces.NewMetricsObjectWrapperFromClientObject(previousObject)
if err != nil {
return err
if !reconcileObject.IsEndTimeSet() {
continue
}
if reconcileObject.IsEndTimeSet() {
previousInterval := reconcileObject.GetEndTime().Sub(piWrapper2.GetStartTime())
gauge.Observe(ctx, previousInterval.Seconds(), reconcileObject.GetDurationMetricsAttributes()...)
predecessor := getPredecessor(reconcileObject, items)
if predecessor == nil {
continue
}

previousInterval := reconcileObject.GetEndTime().Sub(predecessor.GetStartTime())
gauge.Observe(ctx, previousInterval.Seconds(), reconcileObject.GetDurationMetricsAttributes()...)
}
}

return nil
}

func getPredecessor(successor *interfaces.MetricsObjectWrapper, items []client.Object) interfaces.MetricsObject {
var predecessor interfaces.MetricsObject
for i := 0; i < len(items); i++ {
// to calculate the interval, we take the earliest revision of the previous version as a reference
if strings.HasPrefix(items[i].GetName(), fmt.Sprintf("%s-%s", successor.GetParentName(), successor.GetPreviousVersion())) {
predecessorCandidate, err := interfaces.NewMetricsObjectWrapperFromClientObject(items[i])
if err != nil {
// continue with the other items
continue
}
if predecessor == nil || predecessorCandidate.GetStartTime().Before(predecessor.GetStartTime()) {
predecessor = predecessorCandidate
}
}
}
return predecessor
}

func ObserveActiveInstances(ctx context.Context, client client.Client, reconcileObjectList client.ObjectList, gauge asyncint64.Gauge) error {
err := client.List(ctx, reconcileObjectList)
if err != nil {
Expand Down
159 changes: 98 additions & 61 deletions operator/controllers/common/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

lifecyclev1alpha2 "github.com/keptn/lifecycle-toolkit/operator/apis/lifecycle/v1alpha2"
controllererrors "github.com/keptn/lifecycle-toolkit/operator/controllers/errors"
"github.com/keptn/lifecycle-toolkit/operator/controllers/lifecycle/interfaces"
"github.com/stretchr/testify/require"
noop "go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/instrument/asyncfloat64"
Expand Down Expand Up @@ -166,7 +167,6 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
tests := []struct {
name string
clientObjects client.ObjectList
clientObject client.Object
list client.ObjectList
previous client.Object
err error
Expand All @@ -175,13 +175,11 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
name: "failed to create wrapper",
list: &lifecyclev1alpha2.KeptnAppList{},
clientObjects: &lifecyclev1alpha2.KeptnAppList{},
clientObject: &lifecyclev1alpha2.KeptnApp{},
err: controllererrors.ErrCannotWrapToListItem,
},
{
name: "no previous version",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
clientObject: &lifecyclev1alpha2.KeptnApp{},
name: "no previous version",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
clientObjects: &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
Expand All @@ -201,9 +199,8 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
err: nil,
},
{
name: "previous version - no previous object",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
clientObject: &lifecyclev1alpha2.KeptnApp{},
name: "previous version - no previous object",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
clientObjects: &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
Expand All @@ -223,15 +220,9 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
err: nil,
},
{
name: "previous version - object found but cannot unwrap",
name: "previous version - object found but no endtime",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
previous: &lifecyclev1alpha2.KeptnApp{},
clientObject: &lifecyclev1alpha2.KeptnApp{
ObjectMeta: metav1.ObjectMeta{
Name: "appName-previousVersion",
Namespace: "namespace",
},
},
previous: &lifecyclev1alpha2.KeptnAppVersion{},
clientObjects: &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
Expand All @@ -247,40 +238,17 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
PreviousVersion: "previousVersion",
},
},
},
},
err: controllererrors.ErrCannotWrapToMetricsObject,
},
{
name: "previous version - object found but no endtime",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
previous: &lifecyclev1alpha2.KeptnAppVersion{},
clientObject: &lifecyclev1alpha2.KeptnAppVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "appName-previousVersion",
Namespace: "namespace",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "previousVersion",
},
AppName: "appName",
PreviousVersion: "",
},
},
clientObjects: &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
ObjectMeta: metav1.ObjectMeta{
Name: "appName-version",
Name: "appName-previousVersion",
Namespace: "namespace",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "version",
Version: "previousVersion",
},
AppName: "appName",
PreviousVersion: "previousVersion",
PreviousVersion: "",
},
},
},
Expand All @@ -291,23 +259,6 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
name: "previous version - object found with endtime",
list: &lifecyclev1alpha2.KeptnAppVersionList{},
previous: &lifecyclev1alpha2.KeptnAppVersion{},
clientObject: &lifecyclev1alpha2.KeptnAppVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "appName-previousVersion",
Namespace: "namespace",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "previousVersion",
},
AppName: "appName",
PreviousVersion: "",
},
Status: lifecyclev1alpha2.KeptnAppVersionStatus{
EndTime: metav1.Time{Time: metav1.Now().Time.Add(5 * time.Second)},
StartTime: metav1.Time{Time: metav1.Now().Time},
},
},
clientObjects: &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
Expand All @@ -327,6 +278,23 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
StartTime: metav1.Time{Time: metav1.Now().Time},
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "appName-previousVersion",
Namespace: "namespace",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "previousVersion",
},
AppName: "appName",
PreviousVersion: "",
},
Status: lifecyclev1alpha2.KeptnAppVersionStatus{
EndTime: metav1.Time{Time: metav1.Now().Time.Add(5 * time.Second)},
StartTime: metav1.Time{Time: metav1.Now().Time},
},
},
},
},
err: nil,
Expand All @@ -340,10 +308,79 @@ func TestMetrics_ObserveDeploymentInterval(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := lifecyclev1alpha2.AddToScheme(scheme.Scheme)
require.Nil(t, err)
client := fake.NewClientBuilder().WithObjects(tt.clientObject).WithLists(tt.clientObjects).Build()
err = ObserveDeploymentInterval(context.TODO(), client, tt.list, tt.previous, gauge)
fakeClient := fake.NewClientBuilder().WithLists(tt.clientObjects).Build()
err = ObserveDeploymentInterval(context.TODO(), fakeClient, tt.list, gauge)
require.ErrorIs(t, err, tt.err)
})

}
}

func TestGetPredecessor(t *testing.T) {
now := time.Now()
appVersions := &lifecyclev1alpha2.KeptnAppVersionList{
Items: []lifecyclev1alpha2.KeptnAppVersion{
{
ObjectMeta: metav1.ObjectMeta{
Name: "my-app-1.0.0-1",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "1.0.0",
Revision: 0,
},
AppName: "my-app",
},
Status: lifecyclev1alpha2.KeptnAppVersionStatus{
StartTime: metav1.NewTime(now),
EndTime: metav1.NewTime(now.Add(10 * time.Second)),
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "my-app-1.0.0-2",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "1.0.0",
Revision: 0,
},
AppName: "my-app",
},
Status: lifecyclev1alpha2.KeptnAppVersionStatus{
StartTime: metav1.NewTime(now.Add(1 * time.Second)),
EndTime: metav1.NewTime(now.Add(10 * time.Second)),
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "my-app-1.1.0-1",
},
Spec: lifecyclev1alpha2.KeptnAppVersionSpec{
KeptnAppSpec: lifecyclev1alpha2.KeptnAppSpec{
Version: "1.0.0",
Revision: 0,
},
AppName: "my-app",
},
Status: lifecyclev1alpha2.KeptnAppVersionStatus{
StartTime: metav1.NewTime(now),
EndTime: metav1.NewTime(now.Add(10 * time.Second)),
},
},
},
}

appVersionsWrapper, err := interfaces.NewListItemWrapperFromClientObjectList(appVersions)
require.Nil(t, err)

latestAppVersion, err := interfaces.NewMetricsObjectWrapperFromClientObject(appVersionsWrapper.GetItems()[2])

require.Nil(t, err)
predecessor := getPredecessor(latestAppVersion, appVersionsWrapper.GetItems())

expectedPredecessor, err := interfaces.NewMetricsObjectWrapperFromClientObject(appVersionsWrapper.GetItems()[0])
require.Nil(t, err)

require.Equal(t, expectedPredecessor, predecessor)
}
4 changes: 2 additions & 2 deletions operator/controllers/common/otel_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ func observeDuration(ctx context.Context, mgr client.Client, appDeploymentDurati
}

func observeDeploymentInterval(ctx context.Context, mgr client.Client, appDeploymentIntervalGauge asyncfloat64.Gauge, workloadDeploymentIntervalGauge asyncfloat64.Gauge) {
err := ObserveDeploymentInterval(ctx, mgr, &lifecyclev1alpha2.KeptnAppVersionList{}, &lifecyclev1alpha2.KeptnWorkloadInstance{}, appDeploymentIntervalGauge)
err := ObserveDeploymentInterval(ctx, mgr, &lifecyclev1alpha2.KeptnAppVersionList{}, appDeploymentIntervalGauge)
if err != nil {
logger.Error(err, "unable to gather app deployment intervals")
}

err = ObserveDeploymentInterval(ctx, mgr, &lifecyclev1alpha2.KeptnWorkloadInstanceList{}, &lifecyclev1alpha2.KeptnWorkloadInstance{}, workloadDeploymentIntervalGauge)
err = ObserveDeploymentInterval(ctx, mgr, &lifecyclev1alpha2.KeptnWorkloadInstanceList{}, workloadDeploymentIntervalGauge)
if err != nil {
logger.Error(err, "unable to gather workload deployment intervals")
}
Expand Down

0 comments on commit a798eed

Please sign in to comment.