Skip to content
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.

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

prefix GVK labels in CustomResourceMonitoring #1942

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion docs/customresourcestate-metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ spec:
- --resources=certificatesigningrequests,configmaps,cronjobs,daemonsets,deployments,endpoints,foos,horizontalpodautoscalers,ingresses,jobs,limitranges,mutatingwebhookconfigurations,namespaces,networkpolicies,nodes,persistentvolumeclaims,persistentvolumes,poddisruptionbudgets,pods,replicasets,replicationcontrollers,resourcequotas,secrets,services,statefulsets,storageclasses,validatingwebhookconfigurations,volumeattachments,verticalpodautoscalers
```

NOTE: The `group`, `version`, and `kind` common labels are reserved, and will be overwritten by the values from the `groupVersionKind` field.
NOTE: The `customresource_group`, `customresource_version`, and `customresource_kind` common labels are reserved, and will be overwritten by the values from the `groupVersionKind` field.

### Examples

Expand Down
6 changes: 5 additions & 1 deletion pkg/customresourcestate/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ import (
"k8s.io/kube-state-metrics/v2/pkg/customresource"
)

// customResourceState is used to prefix the auto-generated GVK labels as well as an appendix for the metric itself
// if no custom metric name is defined
const customResourceState string = "customresource"

// Metrics is the top level configuration object.
type Metrics struct {
Spec MetricsSpec `yaml:"spec" json:"spec"`
Expand Down Expand Up @@ -64,7 +68,7 @@ type Resource struct {
func (r Resource) GetMetricNamePrefix() string {
p := r.MetricNamePrefix
if p == nil {
return "kube_crd"
return "kube_" + customResourceState
mrueg marked this conversation as resolved.
Show resolved Hide resolved
}
return *p
}
Expand Down
188 changes: 180 additions & 8 deletions pkg/customresourcestate/custom_resource_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,87 @@ limitations under the License.
package customresourcestate

import (
"encoding/json"
"reflect"
"testing"

"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/utils/pointer"
)

func TestNewCustomResourceMetrics(t *testing.T) {

tests := []struct {
r Resource
wantErr bool
name string
r Resource
wantErr bool
wantResult *customResourceMetrics
name string
}{
{
// https://github.com/kubernetes/kube-state-metrics/issues/1886
name: "dynamic metric type (not just hardcoded to gauge)",
name: "cr metric with dynamic metric type",
r: Resource{
GroupVersionKind: GroupVersionKind{
Group: "apps",
Version: "v1",
Kind: "Deployment",
},
Labels: Labels{
LabelsFromPath: map[string][]string{
"name": {"metadata", "name"},
},
CommonLabels: map[string]string{
"hello": "world",
},
},
Metrics: []Generator{
{
Name: "test_metrics",
Help: "metrics for testing",
Each: Metric{
Type: MetricTypeInfo,
Info: &MetricInfo{
MetricMeta: MetricMeta{
Path: []string{
"metadata",
"annotations",
},
},
LabelFromKey: "test",
},
},
},
},
},
wantErr: false,
wantResult: &customResourceMetrics{
MetricNamePrefix: "kube_customresource",
GroupVersionKind: schema.GroupVersionKind{
Group: "apps",
Version: "v1",
Kind: "Deployment",
},
ResourceName: "deployments",
Families: []compiledFamily{
{
Name: "kube_customresource_test_metrics",
Help: "metrics for testing",
Each: &compiledInfo{},
Labels: map[string]string{
"customresource_group": "apps",
"customresource_kind": "Deployment",
"customresource_version": "v1",
"hello": "world",
},
LabelFromPath: map[string]valuePath{
"name": mustCompilePath(t, "metadata", "name"),
},
},
},
},
},
{
name: "cr metric with custom prefix",
r: Resource{
GroupVersionKind: GroupVersionKind{
Group: "apps",
Expand All @@ -39,6 +108,9 @@ func TestNewCustomResourceMetrics(t *testing.T) {
LabelsFromPath: map[string][]string{
"name": {"metadata", "name"},
},
CommonLabels: map[string]string{
"hello": "world",
},
},
Metrics: []Generator{
{
Expand All @@ -58,18 +130,118 @@ func TestNewCustomResourceMetrics(t *testing.T) {
},
},
},
MetricNamePrefix: pointer.String("apps_deployment"),
},
wantErr: false,
wantResult: &customResourceMetrics{
MetricNamePrefix: "apps_deployment",
GroupVersionKind: schema.GroupVersionKind{
Group: "apps",
Version: "v1",
Kind: "Deployment",
},
ResourceName: "deployments",
Families: []compiledFamily{
{
Name: "apps_deployment_test_metrics",
Help: "metrics for testing",
Each: &compiledInfo{},
Labels: map[string]string{
"customresource_group": "apps",
"customresource_kind": "Deployment",
"customresource_version": "v1",
"hello": "world",
},
LabelFromPath: map[string]valuePath{
"name": mustCompilePath(t, "metadata", "name"),
},
},
},
},
},
{
name: "cr metric with custom prefix - expect error",
r: Resource{
GroupVersionKind: GroupVersionKind{
Group: "apps",
Version: "v1",
Kind: "Deployment",
},
Labels: Labels{
LabelsFromPath: map[string][]string{
"name": {"metadata", "name"},
},
CommonLabels: map[string]string{
"hello": "world",
},
},
Metrics: []Generator{
{
Name: "test_metrics",
Help: "metrics for testing",
Each: Metric{
Type: MetricTypeInfo,
Info: &MetricInfo{
MetricMeta: MetricMeta{
Path: []string{
"metadata",
"annotations",
},
},
LabelFromKey: "test",
},
},
},
},
MetricNamePrefix: pointer.String("apps_deployment"),
},
wantErr: true,
wantResult: &customResourceMetrics{
GroupVersionKind: schema.GroupVersionKind{
Group: "apps",
Version: "v1",
Kind: "Deployment",
},
ResourceName: "deployments",
Families: []compiledFamily{
{
Name: "apps_deployment_test_metrics",
Help: "metrics for testing",
Each: &compiledInfo{},
Labels: map[string]string{
"customresource_group": "apps",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customresource_group to cr_group?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comming from this comment

I agree, would be nice to have one thing. I believe right now we mix Custom Resource, Custom Resource Definition and Custom Resource State. kube_customresource would align with kube_horizontalpodautoscaler (no additional underscores).

Should we rename everything custom resource state into simply custom resource and abbreviate it with "cr" where necessary?

i've started with customresource (because i like it a bit more) but as i'm not very opinionated i'm also fine to switch to cr

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @mrueg which label name do you prefer? cr_group or customresource_group

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal preference is the second one, two letter acronyms can become ambiguous

"customresource_kind": "Deployment",
"customresource_version": "v1",
"hello": "world",
},
LabelFromPath: map[string]valuePath{
"name": mustCompilePath(t, "metadata", "name"),
},
},
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
v, err := NewCustomResourceMetrics(tt.r)
expectedError := v.(*customResourceMetrics).Families[0].Each.Type() != "info"
if (err != nil) != tt.wantErr || expectedError {
t.Errorf("NewCustomResourceMetrics() error = %v, wantErr %v", err, tt.wantErr)
return
if err != nil {
t.Errorf(err.Error())
}

// convert to JSON for easier nil comparison
ttWantJSON, _ := json.Marshal(tt.wantResult)
customResourceMetricJSON, _ := json.Marshal(v.(*customResourceMetrics))

if !tt.wantErr && !reflect.DeepEqual(ttWantJSON, customResourceMetricJSON) {
t.Errorf("NewCustomResourceMetrics: error expected %v", tt.wantErr)
t.Errorf("NewCustomResourceMetrics:\n %#v\n is not deep equal\n %#v", v, tt.wantResult)
}

if tt.wantErr && reflect.DeepEqual(ttWantJSON, customResourceMetricJSON) {
t.Errorf("NewCustomResourceMetrics: error expected %v", tt.wantErr)
t.Errorf("NewCustomResourceMetrics:\n %#v\n is not deep equal\n %#v", v, tt.wantResult)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/customresourcestate/registry_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ func compile(resource Resource) ([]compiledFamily, error) {
if resource.CommonLabels == nil {
resource.CommonLabels = map[string]string{}
}
resource.CommonLabels["group"] = resource.GroupVersionKind.Group
resource.CommonLabels["version"] = resource.GroupVersionKind.Version
resource.CommonLabels["kind"] = resource.GroupVersionKind.Kind
resource.CommonLabels[customResourceState+"_group"] = resource.GroupVersionKind.Group
resource.CommonLabels[customResourceState+"_version"] = resource.GroupVersionKind.Version
resource.CommonLabels[customResourceState+"_kind"] = resource.GroupVersionKind.Kind
for _, f := range resource.Metrics {
family, err := compileFamily(f, resource)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/customresourcestate/registry_factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func Test_fullName(t *testing.T) {
resource: r(nil),
f: count,
},
want: "kube_crd_count",
want: "kube_customresource_count",
},
{
name: "no prefix",
Expand Down