[AGENTONB-2555][DatadogAgentInternal] Fix DDAI metric forwaders errors (#2214)

* Ensure we determine type before fallback to last-applied-config annotation

* Initialize error to nil in metricsforwader

* update tests based on new behaviour

* refactor in single if statement + remove unknown fallback + use unstructured to properly test annotation fallback

Author: tbavelier

internal/controller/datadogagent/ddai.go

@@ -46,11 +46,16 @@ func (r *Reconciler) generateDDAIFromDDA(dda *v2alpha1.DatadogAgent) (*v1alpha1.
 }
 
 func generateObjMetaFromDDA(dda *v2alpha1.DatadogAgent, ddai *v1alpha1.DatadogAgentInternal, scheme *runtime.Scheme) error {
+	// Copy ddaiAnnotations but strip kubectl last-applied-configuration to avoid confusing kind detection for metrics forwarder
+	// Moreover, the applied configuration is the one for DDA, not DDAI, so it doesn't make sense.
+	ddaiAnnotations := maps.Clone(dda.Annotations)
+	delete(ddaiAnnotations, "kubectl.kubernetes.io/last-applied-configuration")
+
 	ddai.ObjectMeta = metav1.ObjectMeta{
 		Name:        dda.Name,
 		Namespace:   dda.Namespace,
 		Labels:      getDDAILabels(dda),
-		Annotations: dda.Annotations,
+		Annotations: ddaiAnnotations,
 	}
 	if err := object.SetOwnerReference(dda, ddai, scheme); err != nil {
 		return err

pkg/controller/utils/datadog/common.go

@@ -9,6 +9,8 @@ import (
 	"encoding/json"
 	"fmt"
 
+	v1alpha1 "github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1"
+	v2alpha1 "github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -21,27 +23,32 @@ func getObjKind(obj client.Object) string {
 	// First try to get the kind from GVK
 	objKind := obj.GetObjectKind().GroupVersionKind().Kind
 
-	// There is a known bug where the object frequently has empty GVK info. This is a workaround to get object Kind if that happens.
+	// There is a known bug where the object frequently has empty GVK info.
 	// Ref: https://github.com/kubernetes-sigs/controller-runtime/issues/1735
+	// If GVK is empty, prefer inspecting the concrete type to avoid annotation-induced misclassification
 	if objKind == "" {
-		// Try to get it from the last-applied-configuration annotation
-		if annotations := obj.GetAnnotations(); annotations != nil {
-			if lastConfig, exists := annotations["kubectl.kubernetes.io/last-applied-configuration"]; exists {
-				var config map[string]any
-				if err := json.Unmarshal([]byte(lastConfig), &config); err == nil {
-					if kind, ok := config["kind"].(string); ok {
-						objKind = kind
+		switch obj.(type) {
+		case *v2alpha1.DatadogAgent:
+			objKind = datadogAgentKind
+		case *v1alpha1.DatadogAgentInternal:
+			objKind = datadogAgentInternalKind
+		case *v1alpha1.DatadogMonitor:
+			objKind = datadogMonitorKind
+		default:
+			// As a fallback, get it from the last-applied-configuration annotation.
+			if annotations := obj.GetAnnotations(); annotations != nil {
+				if lastConfig, exists := annotations["kubectl.kubernetes.io/last-applied-configuration"]; exists {
+					var config map[string]any
+					if err := json.Unmarshal([]byte(lastConfig), &config); err == nil {
+						if kind, ok := config["kind"].(string); ok {
+							objKind = kind
+						}
 					}
 				}
 			}
 		}
 	}
 
-	// Last fallback after GVK is empty and last-applied-configuration annotation is not present
-	if objKind == "" {
-		objKind = "Unknown"
-	}
-
 	return objKind
 }
 

pkg/controller/utils/datadog/common_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/DataDog/datadog-operator/api/datadoghq/v2alpha1"
 	"github.com/DataDog/datadog-operator/pkg/kubernetes/rbac"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -66,17 +67,15 @@ func TestGetObjKind(t *testing.T) {
 			expected: datadogAgentInternalKind,
 		},
 		{
-			name: "DatadogAgent with last-applied-configuration annotation - no GVK set",
+			name: "Generic object with last-applied-configuration annotation - no GVK set",
 			obj: func() client.Object {
-				return &v2alpha1.DatadogAgent{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      testAgentName,
-						Namespace: fooNamespace,
-						Annotations: map[string]string{
-							"kubectl.kubernetes.io/last-applied-configuration": `{"kind":"` + datadogAgentKind + `"}`,
-						},
-					},
-				}
+				obj := &unstructured.Unstructured{}
+				obj.SetName(testAgentName)
+				obj.SetNamespace(fooNamespace)
+				obj.SetAnnotations(map[string]string{
+					"kubectl.kubernetes.io/last-applied-configuration": `{"kind":"` + datadogAgentKind + `"}`,
+				})
+				return obj
 			},
 			expected: datadogAgentKind,
 		},

pkg/controller/utils/datadog/metrics_forwarder.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"hash/fnv"
 	"os"
-	"reflect"
 	"strings"
 	"sync"
 	"time"
@@ -389,6 +388,11 @@ func (mf *metricsForwarder) connectToDatadogAPI() (bool, error) {
 		mf.logger.Error(err, "cannot retrieve Datadog metrics forwarder to send deployment metrics, will retry later...")
 		return false, nil
 	}
+	// Initialize lastReconcileErr to nil after successful setup so the first
+	// reconcile metric reports success instead of an initialization error.
+	if errors.Is(mf.getLastReconcileError(), errInitValue) {
+		mf.setLastReconcileError(nil)
+	}
 	return true, nil
 }
 
@@ -452,7 +456,7 @@ func (mf *metricsForwarder) forwardMetrics() error {
 // processReconcileError updates lastReconcileErr
 // and sends reconcile metrics based on the reconcile errors
 func (mf *metricsForwarder) processReconcileError(reconcileErr error) error {
-	if reflect.DeepEqual(mf.getLastReconcileError(), reconcileErr) {
+	if errors.Is(reconcileErr, mf.getLastReconcileError()) {
 		// Error didn't change
 		return nil
 	}

pkg/controller/utils/datadog/metrics_forwarder_test.go

@@ -714,19 +714,17 @@ func Test_metricsForwarder_processReconcileError(t *testing.T) {
 			},
 		},
 		{
-			name: "last error not nil and not init value, new error equals last error => don't send metric",
+			name: "last error not nil and not init value, new error equals last error => send unsuccess metric",
 			loadFunc: func() (*metricsForwarder, *fakeMetricsForwarder) {
 				f := &fakeMetricsForwarder{}
+				f.On("delegatedSendReconcileMetric", ctx, 0.0, []string{"kube_namespace:foo", "resource_name:bar", "reconcile_err:Unauthorized", "cr_preferred_version:null"}).Once()
 				mf.delegator = f
 				mf.lastReconcileErr = apierrors.NewUnauthorized("Auth error")
 				return mf, f
 			},
 			err:     apierrors.NewUnauthorized("Auth error"),
 			wantErr: false,
 			wantFunc: func(f *fakeMetricsForwarder) error {
-				if !f.AssertNumberOfCalls(t, "delegatedSendReconcileMetric", 0) {
-					return errors.New("Wrong number of calls")
-				}
 				f.AssertExpectations(t)
 				return nil
 			},