Skip to content

remove unnecessary factory function wrappers #1429

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamescripter wants to merge 3 commits into
base: main
Choose a base branch
from
Open

remove unnecessary factory function wrappers #1429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6117c8
remove unnecessary factory function wrappers
jamescripter Mar 5, 2026
9ef112d
changelog
jamescripter Mar 5, 2026
63bcb37
Merge branch 'main' into remove-unnecessary-factory-function-wrappers
jamescripter Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

## Unreleased

### bugfix
- Remove unnecesary factory function wrappers @jamescripter [#1429](https://github.com/newrelic/nri-kubernetes/pull/1429)

Check failure on line 11 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / Codespell 

unnecesary ==> unnecessary

### bugfix
- Use different metric to get deployment name for ReplicaSet @jamescripter [#1421](https://github.com/newrelic/nri-kubernetes/pull/1421)

Expand Down
102 changes: 49 additions & 53 deletions src/ksm/metric/metric.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,65 +19,61 @@
)

// 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) {

Check failure on line 23 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

GetDeploymentNameForReplicaSet returns interface (github.com/newrelic/nri-kubernetes/v3/src/definition.FetchedValue) (ireturn)
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) {

Check failure on line 57 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

GetDeploymentNameForPod returns interface (github.com/newrelic/nri-kubernetes/v3/src/definition.FetchedValue) (ireturn)
creatorKind, err := prometheus.FromLabelValue("kube_pod_info", "created_by_kind")(groupLabel, entityID, groups)
if err != nil {
return nil, err
}

if creatorKind.(string) == "" {

Check failure on line 63 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

type assertion must be checked (forcetypeassert)
return nil, errors.New("error generating deployment name for pod. created_by_kind field is empty")

Check failure on line 64 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

do not define dynamic errors, use wrapped static errors instead: "errors.New(\"error generating deployment name for pod. created_by_kind field is empty\")" (err113)
}

creatorName, err := prometheus.FromLabelValue("kube_pod_info", "created_by_name")(groupLabel, entityID, groups)
if err != nil {
return nil, err
}

if creatorName.(string) == "" {

Check failure on line 72 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

type assertion must be checked (forcetypeassert)
return nil, errors.New("error generating deployment name for pod. created_by_name field is empty")

Check failure on line 73 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

do not define dynamic errors, use wrapped static errors instead: "errors.New(\"error generating deployment name for pod. created_by_name field is empty\")" (err113)
}

return deploymentNameBasedOnCreator(creatorKind.(string), creatorName.(string)), nil

Check failure on line 76 in src/ksm/metric/metric.go

View workflow job for this annotation

GitHub Actions / Static analysis and linting 

type assertion must be checked (forcetypeassert)
}

func deploymentNameBasedOnCreator(creatorKind, creatorName string) string {
Expand Down
18 changes: 9 additions & 9 deletions src/ksm/metric/metric_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)

Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions src/metric/definition.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down
Loading