From d6117c8c95b80a27cfaaed9732d59a2f887d45f1 Mon Sep 17 00:00:00 2001 From: James McGivern Date: Thu, 5 Mar 2026 03:22:06 -0800 Subject: [PATCH 1/2] remove unnecessary factory function wrappers --- src/ksm/metric/metric.go | 102 ++++++++++++++++------------------ src/ksm/metric/metric_test.go | 18 +++--- src/metric/definition.go | 4 +- 3 files changed, 60 insertions(+), 64 deletions(-) diff --git a/src/ksm/metric/metric.go b/src/ksm/metric/metric.go index a9ffec8015..cd7e127cd1 100644 --- a/src/ksm/metric/metric.go +++ b/src/ksm/metric/metric.go @@ -19,65 +19,61 @@ var ( ) // GetDeploymentNameForReplicaSet returns the name of the deployment that owns -// a ReplicaSet, an error if the owner is not a deployment. -func GetDeploymentNameForReplicaSet() definition.FetchFunc { - return func(groupLabel, entityID string, groups definition.RawGroups) (definition.FetchedValue, error) { - ownerKindRawVal, err := prometheus.FromLabelValue("kube_replicaset_owner", "owner_kind")(groupLabel, entityID, groups) - if err != nil { - return nil, fmt.Errorf("failed to fetch owner_kind of ReplicaSet: %w", err) - } - - ownerKind, ok := ownerKindRawVal.(string) - if !ok { - return nil, ErrOwnerKindInvalid - } - - if ownerKind != deploymentOwnerKind { - return nil, ErrNotOwnedByDeployment - } - - ownerNameRawVal, err := prometheus.FromLabelValue("kube_replicaset_owner", "owner_name")(groupLabel, entityID, groups) - if err != nil { - return nil, fmt.Errorf("failed to fetch owner_name of ReplicaSet: %w", err) - } - - ownerName, ok := ownerNameRawVal.(string) - if !ok { - return nil, ErrOwnerNameInvalid - } - - if ownerName == "" { - return nil, ErrOwnerNameEmpty - } - - return ownerName, nil +// a ReplicaSet, or an error if the owner is not a deployment. +func GetDeploymentNameForReplicaSet(groupLabel, entityID string, groups definition.RawGroups) (definition.FetchedValue, error) { + ownerKindRawVal, err := prometheus.FromLabelValue("kube_replicaset_owner", "owner_kind")(groupLabel, entityID, groups) + if err != nil { + return nil, fmt.Errorf("failed to fetch owner_kind of ReplicaSet: %w", err) } + + ownerKind, ok := ownerKindRawVal.(string) + if !ok { + return nil, ErrOwnerKindInvalid + } + + if ownerKind != deploymentOwnerKind { + return nil, ErrNotOwnedByDeployment + } + + ownerNameRawVal, err := prometheus.FromLabelValue("kube_replicaset_owner", "owner_name")(groupLabel, entityID, groups) + if err != nil { + return nil, fmt.Errorf("failed to fetch owner_name of ReplicaSet: %w", err) + } + + ownerName, ok := ownerNameRawVal.(string) + if !ok { + return nil, ErrOwnerNameInvalid + } + + if ownerName == "" { + return nil, ErrOwnerNameEmpty + } + + return ownerName, nil } // GetDeploymentNameForPod returns the name of the deployment has created a // Pod. It returns an empty string if Pod hasn't been created by a deployment. -func GetDeploymentNameForPod() definition.FetchFunc { - return func(groupLabel, entityID string, groups definition.RawGroups) (definition.FetchedValue, error) { - creatorKind, err := prometheus.FromLabelValue("kube_pod_info", "created_by_kind")(groupLabel, entityID, groups) - if err != nil { - return nil, err - } - - if creatorKind.(string) == "" { - return nil, errors.New("error generating deployment name for pod. created_by_kind field is empty") - } - - creatorName, err := prometheus.FromLabelValue("kube_pod_info", "created_by_name")(groupLabel, entityID, groups) - if err != nil { - return nil, err - } - - if creatorName.(string) == "" { - return nil, errors.New("error generating deployment name for pod. created_by_name field is empty") - } - - return deploymentNameBasedOnCreator(creatorKind.(string), creatorName.(string)), nil +func GetDeploymentNameForPod(groupLabel, entityID string, groups definition.RawGroups) (definition.FetchedValue, error) { + creatorKind, err := prometheus.FromLabelValue("kube_pod_info", "created_by_kind")(groupLabel, entityID, groups) + if err != nil { + return nil, err } + + if creatorKind.(string) == "" { + return nil, errors.New("error generating deployment name for pod. created_by_kind field is empty") + } + + creatorName, err := prometheus.FromLabelValue("kube_pod_info", "created_by_name")(groupLabel, entityID, groups) + if err != nil { + return nil, err + } + + if creatorName.(string) == "" { + return nil, errors.New("error generating deployment name for pod. created_by_name field is empty") + } + + return deploymentNameBasedOnCreator(creatorKind.(string), creatorName.(string)), nil } func deploymentNameBasedOnCreator(creatorKind, creatorName string) string { diff --git a/src/ksm/metric/metric_test.go b/src/ksm/metric/metric_test.go index d62b232d1a..91c049f1f8 100644 --- a/src/ksm/metric/metric_test.go +++ b/src/ksm/metric/metric_test.go @@ -78,7 +78,7 @@ var rawGroupWithReplicaSet = definition.RawGroups{ func TestGetDeploymentNameForReplicaSet_ValidName(t *testing.T) { expectedValue := "kube-state-metrics" - fetchedValue, err := GetDeploymentNameForReplicaSet()("replicaset", "kube-state-metrics-4044341274", rawGroupWithReplicaSet) + fetchedValue, err := GetDeploymentNameForReplicaSet("replicaset", "kube-state-metrics-4044341274", rawGroupWithReplicaSet) assert.Nil(t, err) assert.Equal(t, expectedValue, fetchedValue) } @@ -101,7 +101,7 @@ func TestGetDeploymentNameForReplicaSet_ErrorOnNonDeploymentOwnerKind(t *testing }, }, } - fetchedValue, err := GetDeploymentNameForReplicaSet()("replicaset", "kube-state-metrics-4044341274", raw) + fetchedValue, err := GetDeploymentNameForReplicaSet("replicaset", "kube-state-metrics-4044341274", raw) assert.EqualError(t, err, "owner_kind of ReplicaSet is not "+deploymentOwnerKind) assert.Empty(t, fetchedValue) } @@ -124,7 +124,7 @@ func TestGetDeploymentNameForReplicaSet_ErrorOnEmptyOwnerName(t *testing.T) { }, }, } - fetchedValue, err := GetDeploymentNameForReplicaSet()("replicaset", "kube-state-metrics-4044341274", raw) + fetchedValue, err := GetDeploymentNameForReplicaSet("replicaset", "kube-state-metrics-4044341274", raw) assert.EqualError(t, err, "owner_name of ReplicaSet is empty") assert.Empty(t, fetchedValue) } @@ -144,7 +144,7 @@ func TestGetDeploymentNameForReplicaSet_ErrorOnMissingOwnerMetric(t *testing.T) }, }, } - fetchedValue, err := GetDeploymentNameForReplicaSet()("replicaset", "kube-state-metrics-4044341274", raw) + fetchedValue, err := GetDeploymentNameForReplicaSet("replicaset", "kube-state-metrics-4044341274", raw) assert.EqualError(t, err, "failed to fetch owner_kind of ReplicaSet: metric \"kube_replicaset_owner\" not found") assert.Empty(t, fetchedValue) } @@ -173,14 +173,14 @@ func TestGetDeploymentNameForReplicaSet_ErrorOnMissingOwnerNameMetric(t *testing }, }, } - fetchedValue, err := GetDeploymentNameForReplicaSet()("replicaset", "kube-state-metrics-4044341274", raw) + fetchedValue, err := GetDeploymentNameForReplicaSet("replicaset", "kube-state-metrics-4044341274", raw) assert.EqualError(t, err, "failed to fetch owner_name of ReplicaSet: label \"owner_name\" not found on metric \"kube_replicaset_owner\": label not found on metric") assert.Empty(t, fetchedValue) } func TestGetDeploymentNameForPod_CreatedByReplicaSet(t *testing.T) { expectedValue := "fluentd-elasticsearch" - fetchedValue, err := GetDeploymentNameForPod()("pod", "fluentd-elasticsearch-jnqb7", rawGroups) + fetchedValue, err := GetDeploymentNameForPod("pod", "fluentd-elasticsearch-jnqb7", rawGroups) assert.Nil(t, err) assert.Equal(t, expectedValue, fetchedValue) } @@ -201,7 +201,7 @@ func TestGetDeploymentNameForPod_NotCreatedByReplicaSet(t *testing.T) { }, } - fetchedValue, err := GetDeploymentNameForPod()("pod", rawEntityID, raw) + fetchedValue, err := GetDeploymentNameForPod("pod", rawEntityID, raw) assert.Nil(t, err) assert.Empty(t, fetchedValue) } @@ -222,7 +222,7 @@ func TestGetDeploymentNameForPod_ErrorOnEmptyData(t *testing.T) { }, } - fetchedValue, err := GetDeploymentNameForPod()("pod", rawEntityID, raw) + fetchedValue, err := GetDeploymentNameForPod("pod", rawEntityID, raw) assert.EqualError(t, err, "error generating deployment name for pod. created_by_kind field is empty") assert.Empty(t, fetchedValue) @@ -235,7 +235,7 @@ func TestGetDeploymentNameForPod_ErrorOnEmptyData(t *testing.T) { raw["pod"]["kube-addon-manager-minikube"]["kube_pod_info"] = m - fetchedValue, err = GetDeploymentNameForPod()("pod", rawEntityID, raw) + fetchedValue, err = GetDeploymentNameForPod("pod", rawEntityID, raw) assert.EqualError(t, err, "error generating deployment name for pod. created_by_name field is empty") assert.Empty(t, fetchedValue) } diff --git a/src/metric/definition.go b/src/metric/definition.go index b91f8f3810..8a0fd2e52d 100644 --- a/src/metric/definition.go +++ b/src/metric/definition.go @@ -685,7 +685,7 @@ var KSMSpecs = definition.SpecGroups{ // namespace is here for backwards compatibility, we should use the namespaceName {Name: "namespace", ValueFunc: prometheus.FromLabelValue("kube_replicaset_created", "namespace"), Type: sdkMetric.ATTRIBUTE}, {Name: "namespaceName", ValueFunc: prometheus.FromLabelValue("kube_replicaset_created", "namespace"), Type: sdkMetric.ATTRIBUTE}, - {Name: "deploymentName", ValueFunc: ksmMetric.GetDeploymentNameForReplicaSet(), Type: sdkMetric.ATTRIBUTE, Optional: true}, + {Name: "deploymentName", ValueFunc: ksmMetric.GetDeploymentNameForReplicaSet, Type: sdkMetric.ATTRIBUTE, Optional: true}, {Name: "label.*", ValueFunc: prometheus.FromMetricWithPrefixedLabels("kube_replicaset_labels", "label"), Type: sdkMetric.ATTRIBUTE}, {Name: "ownerName", ValueFunc: prometheus.FromLabelValue("kube_replicaset_owner", "owner_name"), Type: sdkMetric.ATTRIBUTE}, {Name: "ownerKind", ValueFunc: prometheus.FromLabelValue("kube_replicaset_owner", "owner_kind"), Type: sdkMetric.ATTRIBUTE}, @@ -943,7 +943,7 @@ var KSMSpecs = definition.SpecGroups{ {Name: "isReady", ValueFunc: definition.Transform(fetchWithDefault(prometheus.FromLabelValue("kube_pod_status_ready", "condition"), "false"), toNumericBoolean), Type: sdkMetric.GAUGE}, {Name: "status", ValueFunc: prometheus.FromLabelValue("kube_pod_status_phase", "phase"), Type: sdkMetric.ATTRIBUTE}, {Name: "isScheduled", ValueFunc: definition.Transform(prometheus.FromLabelValue("kube_pod_status_scheduled", "condition"), toNumericBoolean), Type: sdkMetric.GAUGE}, - {Name: "deploymentName", ValueFunc: ksmMetric.GetDeploymentNameForPod(), Type: sdkMetric.ATTRIBUTE}, + {Name: "deploymentName", ValueFunc: ksmMetric.GetDeploymentNameForPod, Type: sdkMetric.ATTRIBUTE}, {Name: "priorityClassName", ValueFunc: prometheus.FromLabelValue("kube_pod_info", "priority_class"), Type: sdkMetric.ATTRIBUTE, Optional: true}, {Name: "label.*", ValueFunc: prometheus.FromMetricWithPrefixedLabels("kube_pod_labels", "label"), Type: sdkMetric.ATTRIBUTE}, {Name: "annotation.*", ValueFunc: prometheus.FromMetricWithPrefixedLabels("kube_pod_annotations", "annotation"), Type: sdkMetric.ATTRIBUTE}, From 9ef112dae5a41c7953e62feabe174a2509737bc5 Mon Sep 17 00:00:00 2001 From: James McGivern Date: Thu, 5 Mar 2026 10:00:00 -0800 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 245a3c4f20..52f346e4f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### bugfix +- Remove unnecesary factory function wrappers @jamescripter [#1429](https://github.com/newrelic/nri-kubernetes/pull/1429) + ### bugfix - Use different metric to get deployment name for ReplicaSet @jamescripter [#1421](https://github.com/newrelic/nri-kubernetes/pull/1421)