Making kube-state-metrics a library

[lilic]

What this PR does / why we need it:
kube-state-metrics has many useful parts which could be used as library, this is the first step towards that.

For now only the functions are exposed and some helper functions moved to a different place. In the long term I would prefer to end with the following structure:

pkg
├── collector // functions needed to create a collector, this would be used in the builder pkg and externally 
├── internal
│   └── builder // content of the current upstream resource collectors which would consume the collector pkg
├── metrics // what is currently there but also move the utils/helper functions from collectors. Also the metricHandler interface as that would be useful to expose and reuse elsewhere.

Any thoughts on the above are welcome, I can also open an issue to discuss it?

[lilic]

cc @mxinden

[brancz]

This looks pretty good to me. Giving @mxinden the last call.

[brancz]

I'm expecting that people are going to want to use these as they are rather common cases, but let's start with a minimal API rather than adding helpers we're unsure about.

[brancz]

I also like the suggestion of making the actual kube-state-metrics part an internal package to ensure users don't ever import it. Naming we can discuss once we re-organize 🙂 .

Thanks for taking care of this!

[mxinden]

Thanks a bunch for looking into this. As there are no major changes included here I think this is pretty much good to go. I can do any follow ups in #577.

[mxinden]

To test out the API of the library I wrote this little test in tests/as_library/as_library_test.go:

package aslibrary

import (
	"context"
	"strings"
	"testing"
	"time"

	"k8s.io/api/core/v1"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"k8s.io/apimachinery/pkg/runtime"
	"k8s.io/apimachinery/pkg/watch"
	clientset "k8s.io/client-go/kubernetes"
	"k8s.io/client-go/kubernetes/fake"
	"k8s.io/client-go/tools/cache"
	"k8s.io/kube-state-metrics/pkg/collectors"
	"k8s.io/kube-state-metrics/pkg/metrics"
	metricsstore "k8s.io/kube-state-metrics/pkg/metrics_store"
)

func TestAsLibrary(t *testing.T) {
	kubeClient := fake.NewSimpleClientset()

	service := v1.Service{
		ObjectMeta: metav1.ObjectMeta{
			Name:            "my-service",
			ResourceVersion: "123456",
		},
	}

	_, err := kubeClient.CoreV1().Services(metav1.NamespaceDefault).Create(&service)
	if err != nil {
		t.Fatal(err)
	}

	c := serviceCollector(kubeClient)

	// Wait for informers to sync
	time.Sleep(time.Second)

	m := c.Collect()

	if len(m) != 1 {
		t.Fatalf("expected one metric to be returned but got %v", len(m))
	}

	if !strings.Contains(string(*m[0]), service.ObjectMeta.Name) {
		t.Fatal("expected string to contain service name")
	}
}

func serviceCollector(kubeClient clientset.Interface) *collectors.Collector {
	store := metricsstore.NewMetricsStore(generateServiceMetrics)

	lw := cache.ListWatch{
		ListFunc: func(opts metav1.ListOptions) (runtime.Object, error) {
			return kubeClient.CoreV1().Services(metav1.NamespaceDefault).List(opts)
		},
		WatchFunc: func(opts metav1.ListOptions) (watch.Interface, error) {
			return kubeClient.CoreV1().Services(metav1.NamespaceDefault).Watch(opts)
		},
	}

	r := cache.NewReflector(&lw, &v1.Service{}, store, 0)

	go r.Run(context.TODO().Done())

	return collectors.NewCollector(store)
}

func generateServiceMetrics(obj interface{}) []*metrics.Metric {
	sPointer := obj.(*v1.Service)
	s := *sPointer

	m, err := metrics.NewMetric("test_metric", []string{"name"}, []string{s.Name}, 1)
	if err != nil {
		panic(err)
	}

	return []*metrics.Metric{m}
}

This still seems like a lot of code a user would need to write. I will follow up with some ideas how we can improve this. Some of your proposed future restructurings definitely sound good.

Please keep in mind, that #577 will introduce a couple of breaking changes that would also affect the library API. In addition as mentioned above I would like to improve the usability of the libraries API. Thereby the library would currently have no stability guarantees. Maybe we can include something like my test above in this patch to make sure I am not breaking too much with #577. I am also happy to follow up with a Golang example.

What are your thoughts @lilic?

[lilic]

This still seems like a lot of code a user would need to write. I will follow up with some ideas how we can improve this. Some of your proposed future restructurings definitely sound good.

Yes agreed. Also, there is serving metrics, I initially exposed the metricHandler as well, and the ServeHTTP, but the problem was cycling dependencies, as for me it made more sense to have that in the metrics pkg. Which is why I suggested the above internal pkg that would just consume these pkgs.

Thereby the library would currently have no stability guarantees.

That is fine and makes perfect sense.

[lilic]

Added the test you suggested, thanks! Not sure what you had in mind about the example, as this already serves an example. But maybe some docs when the API is more stable?

@mxinden @brancz Please have another look, thanks!

[brancz]

But maybe some docs when the API is more stable?

I think that's a good idea once #577 lands.

Thanks for the contributions!

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, LiliC

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [brancz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment