Skip to content

Commit

Permalink
Simplify GetSettingsForMonitoredEntity and fix timestamp comparision …
Browse files Browse the repository at this point in the history 
…bug incase of multiple MEs (#4070)
  • Loading branch information
@luhi-DT
luhi-DT authored Nov 19, 2024
1 parent d2b9476 commit ee30092
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 127 deletions.
4 changes: 2 additions & 2 deletions pkg/clients/dynatrace/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ type Client interface {
// or an api error otherwise
GetMonitoredEntitiesForKubeSystemUUID(ctx context.Context, kubeSystemUUID string) ([]MonitoredEntity, error)

// GetSettingsForMonitoredEntities returns the settings response with the number of settings objects,
// GetSettingsForMonitoredEntity returns the settings response with the number of settings objects,
// or an api error otherwise
GetSettingsForMonitoredEntities(ctx context.Context, monitoredEntities []MonitoredEntity, schemaId string) (GetSettingsResponse, error)
GetSettingsForMonitoredEntity(ctx context.Context, monitoredEntity *MonitoredEntity, schemaId string) (GetSettingsResponse, error)

GetRulesSettings(ctx context.Context, kubeSystemUUID string, entityID string) (GetRulesSettingsResponse, error)

Expand Down
15 changes: 3 additions & 12 deletions pkg/clients/dynatrace/settings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ func (dtc *dynatraceClient) GetMonitoredEntitiesForKubeSystemUUID(ctx context.Co
return resDataJson.Entities, nil
}

func (dtc *dynatraceClient) GetSettingsForMonitoredEntities(ctx context.Context, monitoredEntities []MonitoredEntity, schemaId string) (GetSettingsResponse, error) {
if len(monitoredEntities) < 1 {
func (dtc *dynatraceClient) GetSettingsForMonitoredEntity(ctx context.Context, monitoredEntity *MonitoredEntity, schemaId string) (GetSettingsResponse, error) {
if monitoredEntity == nil {
return GetSettingsResponse{TotalCount: 0}, nil
}

Expand All @@ -96,7 +96,7 @@ func (dtc *dynatraceClient) GetSettingsForMonitoredEntities(ctx context.Context,

q := req.URL.Query()
q.Add(schemaIDsQueryParam, schemaId)
q.Add(scopesQueryParam, createScopes(monitoredEntities))
q.Add(scopesQueryParam, monitoredEntity.EntityId)
req.URL.RawQuery = q.Encode()

res, err := dtc.httpClient.Do(req)
Expand Down Expand Up @@ -163,12 +163,3 @@ func handleErrorArrayResponseFromAPI(response []byte, statusCode int) error {
return errors.New(sb.String())
}
}

func createScopes(monitoredEntities []MonitoredEntity) string {
scopes := make([]string, 0, len(monitoredEntities))
for _, entity := range monitoredEntities {
scopes = append(scopes, entity.EntityId)
}

return strings.Join(scopes, ",")
}
12 changes: 5 additions & 7 deletions pkg/clients/dynatrace/settings_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestDynatraceClient_GetMonitoredEntitiesForKubeSystemUUID(t *testing.T) {
})
}

func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
func TestDynatraceClient_GetSettingsForMonitoredEntity(t *testing.T) {
ctx := context.Background()

t.Run(`settings for the given monitored entities exist`, func(t *testing.T) {
Expand All @@ -124,7 +124,7 @@ func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
require.NotNil(t, dtc)

// act
actual, err := dtc.GetSettingsForMonitoredEntities(ctx, expected, KubernetesSettingsSchemaId)
actual, err := dtc.GetSettingsForMonitoredEntity(ctx, &expected[0], KubernetesSettingsSchemaId)

// assert
require.NoError(t, err)
Expand All @@ -147,7 +147,7 @@ func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
require.NotNil(t, dtc)

// act
actual, err := dtc.GetSettingsForMonitoredEntities(ctx, expected, KubernetesSettingsSchemaId)
actual, err := dtc.GetSettingsForMonitoredEntity(ctx, &expected[0], KubernetesSettingsSchemaId)

// assert
require.NoError(t, err)
Expand All @@ -156,8 +156,6 @@ func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
})

t.Run(`no settings for an empty list of monitored entities exist`, func(t *testing.T) {
// arrange
entities := []MonitoredEntity{}
// it is immaterial what we put here since no http call is executed when the list of
// monitored entities is empty, therefore also no settings will be returned
totalCount := 999
Expand All @@ -171,7 +169,7 @@ func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
require.NotNil(t, dtc)

// act
actual, err := dtc.GetSettingsForMonitoredEntities(ctx, entities, KubernetesSettingsSchemaId)
actual, err := dtc.GetSettingsForMonitoredEntity(ctx, nil, KubernetesSettingsSchemaId)

// assert
require.NoError(t, err)
Expand All @@ -194,7 +192,7 @@ func TestDynatraceClient_GetSettingsForMonitoredEntities(t *testing.T) {
require.NotNil(t, dtc)

// act
actual, err := dtc.GetSettingsForMonitoredEntities(ctx, entities, KubernetesSettingsSchemaId)
actual, err := dtc.GetSettingsForMonitoredEntity(ctx, &entities[0], KubernetesSettingsSchemaId)

// assert
require.Error(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/dynakube/activegate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,9 @@ func TestReconcileActiveGate(t *testing.T) {
testUID,
mock.AnythingOfType("string"))

mockClient.AssertCalled(t, "GetSettingsForMonitoredEntities",
mockClient.AssertCalled(t, "GetSettingsForMonitoredEntity",
mock.AnythingOfType("context.backgroundCtx"),
[]dtclient.MonitoredEntity{{EntityId: dk.Status.KubernetesClusterMEID, DisplayName: "", LastSeenTms: 0}},
&dtclient.MonitoredEntity{EntityId: dk.Status.KubernetesClusterMEID, DisplayName: "", LastSeenTms: 0},
mock.AnythingOfType("string"))

require.NoError(t, err)
Expand Down
32 changes: 7 additions & 25 deletions pkg/controllers/dynakube/apimonitoring/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,16 @@ func (r *Reconciler) createObjectIdIfNotExists(ctx context.Context) (string, err
return "", err
}

var monitoredEntity []dtclient.MonitoredEntity
var monitoredEntity *dtclient.MonitoredEntity

if r.dk.Status.KubernetesClusterMEID != "" {
monitoredEntity = []dtclient.MonitoredEntity{
{
EntityId: r.dk.Status.KubernetesClusterMEID,
},
monitoredEntity = &dtclient.MonitoredEntity{
EntityId: r.dk.Status.KubernetesClusterMEID,
}
}

// check if Setting for ME exists
settings, err := r.dtc.GetSettingsForMonitoredEntities(ctx, monitoredEntity, dtclient.KubernetesSettingsSchemaId)
settings, err := r.dtc.GetSettingsForMonitoredEntity(ctx, monitoredEntity, dtclient.KubernetesSettingsSchemaId)
if err != nil {
return "", errors.WithMessage(err, "error trying to check if setting exists")
}
Expand Down Expand Up @@ -94,15 +92,15 @@ func (r *Reconciler) createObjectIdIfNotExists(ctx context.Context) (string, err
return objectID, nil
}

func (r *Reconciler) handleKubernetesAppEnabled(ctx context.Context, monitoredEntities []dtclient.MonitoredEntity) (string, error) {
func (r *Reconciler) handleKubernetesAppEnabled(ctx context.Context, monitoredEntity *dtclient.MonitoredEntity) (string, error) {
if r.dk.FeatureEnableK8sAppEnabled() {
appSettings, err := r.dtc.GetSettingsForMonitoredEntities(ctx, monitoredEntities, dtclient.AppTransitionSchemaId)
appSettings, err := r.dtc.GetSettingsForMonitoredEntity(ctx, monitoredEntity, dtclient.AppTransitionSchemaId)
if err != nil {
return "", errors.WithMessage(err, "error trying to check if app setting exists")
}

if appSettings.TotalCount == 0 {
meID := determineNewestMonitoredEntity(monitoredEntities)
meID := monitoredEntity.EntityId
if meID != "" {
transitionSchemaObjectID, err := r.dtc.CreateOrUpdateKubernetesAppSetting(ctx, meID)
if err != nil {
Expand All @@ -120,19 +118,3 @@ func (r *Reconciler) handleKubernetesAppEnabled(ctx context.Context, monitoredEn

return "", nil
}

// determineNewestMonitoredEntity returns the UUID of the newest entities; or empty string if the slice of entities is empty
func determineNewestMonitoredEntity(entities []dtclient.MonitoredEntity) string {
if len(entities) == 0 {
return ""
}

var newestMe dtclient.MonitoredEntity
for _, entity := range entities {
if entity.LastSeenTms > newestMe.LastSeenTms {
newestMe = entity
}
}

return newestMe.EntityId
}
82 changes: 28 additions & 54 deletions pkg/controllers/dynakube/apimonitoring/reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,20 @@ func TestNewDefaultReconiler(t *testing.T) {
}

func createDefaultReconciler(t *testing.T) *Reconciler {
return createReconciler(t, newDynaKube(), []dtclient.MonitoredEntity{}, dtclient.GetSettingsResponse{TotalCount: 0}, "", "")
return createReconciler(t, newDynaKube(), []dtclient.MonitoredEntity{}, nil, dtclient.GetSettingsResponse{TotalCount: 0}, "", "")
}

func createReconciler(t *testing.T, dk *dynakube.DynaKube, monitoredEntities []dtclient.MonitoredEntity, getSettingsResponse dtclient.GetSettingsResponse, objectID string, meID interface{}) *Reconciler { //nolint:revive // argument-limit doesn't apply to constructors
func createReconciler(t *testing.T, dk *dynakube.DynaKube, monitoredEntities []dtclient.MonitoredEntity, monitoredEntity *dtclient.MonitoredEntity, getSettingsResponse dtclient.GetSettingsResponse, objectID string, meID interface{}) *Reconciler { //nolint:revive // argument-limit doesn't apply to constructors
mockClient := dtclientmock.NewClient(t)
mockClient.On("GetMonitoredEntitiesForKubeSystemUUID", mock.AnythingOfType("context.backgroundCtx"), mock.AnythingOfType("string")).
Return(monitoredEntities, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), []dtclient.MonitoredEntity{{EntityId: "test-MEID"}},
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), &dtclient.MonitoredEntity{EntityId: "test-MEID"},
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), []dtclient.MonitoredEntity{{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F"}},
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), &dtclient.MonitoredEntity{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F"},
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), monitoredEntities,
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), monitoredEntity,
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("CreateOrUpdateKubernetesSetting", mock.AnythingOfType("context.backgroundCtx"), testName, testUID, mock.AnythingOfType("string")).
Expand All @@ -58,17 +58,17 @@ func createReconciler(t *testing.T, dk *dynakube.DynaKube, monitoredEntities []d
return r
}

func createReadOnlyReconciler(t *testing.T, dk *dynakube.DynaKube, monitoredEntities []dtclient.MonitoredEntity, getSettingsResponse dtclient.GetSettingsResponse) *Reconciler {
func createReadOnlyReconciler(t *testing.T, dk *dynakube.DynaKube, monitoredEntities []dtclient.MonitoredEntity, monitoredEntity *dtclient.MonitoredEntity, getSettingsResponse dtclient.GetSettingsResponse) *Reconciler {
mockClient := dtclientmock.NewClient(t)
mockClient.On("GetMonitoredEntitiesForKubeSystemUUID", mock.AnythingOfType("context.backgroundCtx"), mock.AnythingOfType("string")).
Return(monitoredEntities, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), []dtclient.MonitoredEntity{{EntityId: "test-MEID"}},
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), &dtclient.MonitoredEntity{EntityId: "test-MEID"},
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), []dtclient.MonitoredEntity{{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F"}},
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), &dtclient.MonitoredEntity{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F"},
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("GetSettingsForMonitoredEntities", mock.AnythingOfType("context.backgroundCtx"), monitoredEntities,
mockClient.On("GetSettingsForMonitoredEntity", mock.AnythingOfType("context.backgroundCtx"), monitoredEntity,
mock.AnythingOfType("string")).
Return(getSettingsResponse, nil)
mockClient.On("CreateOrUpdateKubernetesSetting", mock.AnythingOfType("context.backgroundCtx"), testName, testUID, "KUBERNETES_CLUSTER-119C75CCDA94799F").
Expand All @@ -93,9 +93,9 @@ func createReconcilerWithError(t *testing.T, dk *dynakube.DynaKube, monitoredEnt
mockClient := dtclientmock.NewClient(t)
mockClient.On("GetMonitoredEntitiesForKubeSystemUUID", mock.AnythingOfType("context.backgroundCtx"), mock.AnythingOfType("string")).
Return([]dtclient.MonitoredEntity{}, monitoredEntitiesError)
mockClient.On("GetSettingsForMonitoredEntities",
mockClient.On("GetSettingsForMonitoredEntity",
mock.AnythingOfType("context.backgroundCtx"),
mock.AnythingOfType("[]dynatrace.MonitoredEntity"),
mock.AnythingOfType("*dynatrace.MonitoredEntity"),
mock.AnythingOfType("string")).
Return(dtclient.GetSettingsResponse{}, getSettingsResponseError)
mockClient.On("CreateOrUpdateKubernetesSetting", mock.AnythingOfType("context.backgroundCtx"), testName, testUID, "KUBERNETES_CLUSTER-119C75CCDA94799F").
Expand All @@ -118,8 +118,8 @@ func createReconcilerWithError(t *testing.T, dk *dynakube.DynaKube, monitoredEnt

func createMonitoredEntities() []dtclient.MonitoredEntity {
return []dtclient.MonitoredEntity{
{EntityId: "KUBERNETES_CLUSTER-0E30FE4BF2007587", DisplayName: "operator test entity 1", LastSeenTms: 1639483869085},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F", DisplayName: "operator test entity 2", LastSeenTms: 1639034988126},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F", DisplayName: "operator test entity 1", LastSeenTms: 1639483869085},
{EntityId: "KUBERNETES_CLUSTER-0E30FE4BF2007587", DisplayName: "operator test entity 2", LastSeenTms: 1639034988126},
}
}

Expand All @@ -140,7 +140,7 @@ func TestReconcile(t *testing.T) {

t.Run("create setting when no monitored entities are existing", func(t *testing.T) {
// arrange
r := createReconciler(t, dk, []dtclient.MonitoredEntity{}, dtclient.GetSettingsResponse{}, testObjectID, "")
r := createReconciler(t, dk, []dtclient.MonitoredEntity{}, nil, dtclient.GetSettingsResponse{}, testObjectID, "")

// act
actual, err := r.createObjectIdIfNotExists(ctx)
Expand All @@ -153,7 +153,7 @@ func TestReconcile(t *testing.T) {
t.Run("create setting when no settings for the found monitored entities are existing", func(t *testing.T) {
// arrange
entities := createMonitoredEntities()
r := createReconciler(t, dk, entities, dtclient.GetSettingsResponse{}, testObjectID, "")
r := createReconciler(t, dk, entities, &entities[0], dtclient.GetSettingsResponse{}, testObjectID, "")

// act
actual, err := r.createObjectIdIfNotExists(ctx)
Expand All @@ -166,7 +166,7 @@ func TestReconcile(t *testing.T) {
t.Run("don't create setting when settings for the found monitored entities are existing", func(t *testing.T) {
// arrange
entities := createMonitoredEntities()
r := createReadOnlyReconciler(t, dk, entities, dtclient.GetSettingsResponse{TotalCount: 1})
r := createReadOnlyReconciler(t, dk, entities, &entities[0], dtclient.GetSettingsResponse{TotalCount: 1})

// act
actual, err := r.createObjectIdIfNotExists(ctx)
Expand All @@ -183,7 +183,7 @@ func TestReconcileErrors(t *testing.T) {

t.Run("don't create setting when no kube-system uuid is given", func(t *testing.T) {
// arrange
r := createReconciler(t, dk, []dtclient.MonitoredEntity{{EntityId: "test-MEID"}}, dtclient.GetSettingsResponse{}, testObjectID, "")
r := createReconciler(t, dk, []dtclient.MonitoredEntity{{EntityId: "test-MEID"}}, &dtclient.MonitoredEntity{EntityId: "test-MEID"}, dtclient.GetSettingsResponse{}, testObjectID, "")
dk.Status.KubeSystemUUID = ""

// act
Expand Down Expand Up @@ -249,26 +249,22 @@ func TestHandleKubernetesAppEnabled(t *testing.T) {

t.Run("don't create app setting due to empty MonitoredEntitys", func(t *testing.T) {
// arrange
r := createReconciler(t, dk, []dtclient.MonitoredEntity{{EntityId: "test-MEID"}},
dtclient.GetSettingsResponse{}, "", "")
r := createReconciler(t, dk, []dtclient.MonitoredEntity{}, &dtclient.MonitoredEntity{}, dtclient.GetSettingsResponse{}, "", "")

// act
_, err := r.handleKubernetesAppEnabled(ctx, []dtclient.MonitoredEntity{{EntityId: "test-MEID"}})
_, err := r.handleKubernetesAppEnabled(ctx, &dtclient.MonitoredEntity{})

// assert
require.NoError(t, err)
})

t.Run("don't create app setting as settings already exist", func(t *testing.T) {
// arrange
entities := []dtclient.MonitoredEntity{
{EntityId: "KUBERNETES_CLUSTER-0E30FE4BF2007587", DisplayName: "operator test entity newest", LastSeenTms: 1639483869085},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F", DisplayName: "operator test entity 1", LastSeenTms: 1639034988126},
}
r := createReconciler(t, dk, entities, dtclient.GetSettingsResponse{TotalCount: 1}, "", "")
entities := createMonitoredEntities()
r := createReconciler(t, dk, entities, &entities[0], dtclient.GetSettingsResponse{TotalCount: 1}, "", "")

// act
_, err := r.handleKubernetesAppEnabled(ctx, entities)
_, err := r.handleKubernetesAppEnabled(ctx, &entities[0])

// assert
require.NoError(t, err)
Expand All @@ -279,7 +275,7 @@ func TestHandleKubernetesAppEnabled(t *testing.T) {
r := createReconcilerWithError(t, dk, nil, errors.New("could not get monitored entities"), nil, nil)

// act
_, err := r.handleKubernetesAppEnabled(ctx, []dtclient.MonitoredEntity{})
_, err := r.handleKubernetesAppEnabled(ctx, &dtclient.MonitoredEntity{})

// assert
require.Error(t, err)
Expand All @@ -289,11 +285,10 @@ func TestHandleKubernetesAppEnabled(t *testing.T) {
// arrange
r := createReconcilerWithError(t, dk, nil, nil, nil, errors.New("could not get monitored entities"))
meID := "KUBERNETES_CLUSTER-0E30FE4BF2007587"
entities := []dtclient.MonitoredEntity{
{EntityId: meID, DisplayName: "operator test entity newest", LastSeenTms: 1639483869085},
}
entity := dtclient.MonitoredEntity{EntityId: meID, DisplayName: "operator test entity newest", LastSeenTms: 1639483869085}

// act
_, err := r.handleKubernetesAppEnabled(ctx, entities)
_, err := r.handleKubernetesAppEnabled(ctx, &entity)

// assert
require.Error(t, err)
Expand All @@ -305,36 +300,15 @@ func TestHandleKubernetesAppEnabled(t *testing.T) {
entities := []dtclient.MonitoredEntity{
{EntityId: meID, DisplayName: "operator test entity newest", LastSeenTms: 1639483869085},
}
r := createReconciler(t, dk, entities, dtclient.GetSettingsResponse{}, "", meID)
r := createReconciler(t, dk, entities, &entities[0], dtclient.GetSettingsResponse{}, "", meID)
// act
id, err := r.handleKubernetesAppEnabled(ctx, entities)
id, err := r.handleKubernetesAppEnabled(ctx, &entities[0])
// assert
require.NoError(t, err)
assert.Equal(t, "transitionSchemaObjectID", id)
})
}

func TestDetermineNewestMonitoredEntity(t *testing.T) {
t.Run("newest monitored entity is correctly calculated", func(t *testing.T) {
// arrange
// explicit create of entities here to visualize that one has the newest LastSeenTimestamp
// here it is the first one
entities := []dtclient.MonitoredEntity{
{EntityId: "KUBERNETES_CLUSTER-0E30FE4BF2007587", DisplayName: "operator test entity newest", LastSeenTms: 1639483869085},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799F", DisplayName: "operator test entity 1", LastSeenTms: 1639034988126},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA947993", DisplayName: "operator test entity 2", LastSeenTms: 1639134988126},
{EntityId: "KUBERNETES_CLUSTER-119C75CCDA94799D", DisplayName: "operator test entity 3", LastSeenTms: 1639234988126},
}

// act
newestEntity := determineNewestMonitoredEntity(entities)

// assert
assert.NotNil(t, newestEntity)
assert.Equal(t, entities[0].EntityId, newestEntity)
})
}

func newDynaKube() *dynakube.DynaKube {
return &dynakube.DynaKube{
TypeMeta: metav1.TypeMeta{
Expand Down
Loading

0 comments on commit ee30092

Please sign in to comment.