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

Incompatible changes about custom resource metrics #2013

Open
Garrybest opened this issue Mar 1, 2023 · 10 comments
Open

Incompatible changes about custom resource metrics #2013

Garrybest opened this issue Mar 1, 2023 · 10 comments
Assignees
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@Garrybest
Copy link
Member

I have introduced a feature in #1644, the intent about this interface is to allow users build their own repos and make KSM as a dependency library in their go.mod. So all the custom metrics could be customized by implement these predefined interfaces and pass the factories to run KSM.

After that PR, #1710 is contributed for collecting custom metrics by merely providing a config without building their own repo. It's a great step. I believe users could leverge KSM by form of dependency library or direct config.

Now, according to #1928 (comment), this PR has already removed this parameter factories ...customresource.RegistryFactory. So we could never leverage KSM as a dependency library. I believe some users have already done this but now it's incompatible, e.g., #1644 (comment). So I suggest we move back to adding this parameter in server.go again.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 1, 2023
@CatherineF-dev
Copy link
Contributor

CatherineF-dev commented Mar 3, 2023

Could you provide a simple example which makes KSM as a library? And it couldn't work with latest KSM version.

Trying to reproduce this issue.

@Garrybest
Copy link
Member Author

Hi @CatherineF-dev. I introduce #1644 and want to extend KSM by using it as a dependency library. So we could build our own repo like this. Here is my case, this code belongs to a private library.

package main

import (
	"fmt"
	"os"

	"github.com/prometheus/common/version"
	apiserver "k8s.io/apiserver/pkg/server"
	"k8s.io/klog/v2"
	"k8s.io/kube-state-metrics/v2/pkg/app"
	"k8s.io/kube-state-metrics/v2/pkg/customresource"
	"k8s.io/kube-state-metrics/v2/pkg/options"

	"git.woa.com/c.oa.com/karmada-state-metrics/pkg/karmada"
)

var (
	// This is a user-defined custom resources factory which could be passed to KSM
	// and be initialized by KSM custom resource framework.
	factories = []customresource.RegistryFactory{
		new(karmada.ClusterFactory),
		new(karmada.ResourceBindingFactory),
	}
)

func main() {
	opts := options.NewOptions()
	opts.AddFlags()

	if err := opts.Parse(); err != nil {
		klog.Fatalf("Parsing flag definitions error: %v", err)
	}

	if opts.Version {
		fmt.Printf("%s\n", version.Print("kube-state-metrics"))
		os.Exit(0)
	}

	if opts.Help {
		opts.Usage()
		os.Exit(0)
	}

	ctx :=apiserver.SetupSignalContext()
	// We pass these factories to KSM and compile our own factory with KSM internal metrics.
	if err := app.RunKubeStateMetrics(ctx, opts, factories...); err != nil {
		klog.Fatalf("Failed to run kube-state-metrics: %v", err)
	}
}

We build our own structs by implementing customresource.RegistryFactory and pass them to app.RunKubeStateMetrics, so KSM could initialize metrics as well as our custom metrics. Now according to #1928 (comment), parameters of app.RunKubeStateMetrics have been changed so the code above cannot be compiled.

@CatherineF-dev
Copy link
Contributor

QQ: got it. Why do you want to use KSM as library instead of deploying the KSM agent?

Is it because you only want customresource feature and want to reduce memory usage

@Garrybest
Copy link
Member Author

QQ: got it. Why do you want to use KSM as library instead of deploying the KSM agent?

Is it because you only want customresource feature and want to reduce memory usage

Do you mean https://github.com/kubernetes/kube-state-metrics/blob/main/docs/customresourcestate-metrics.md? Well, I didn't use this method to expose my metrics because these metrics have a complex logic to calculate. So custom resource state metrics cannot meet my demand.

The initial goal of #1644 is to extend KSM as a library for some complex metrics exposing. It is similar with framework of kube-scheduler scheduler-plugins as it does allow users to build some high-level extension.

So inspired by scheduler-plugins, we could add the parameters back and let users introduce their out-of-tree RegistryFactory.

@mrueg
Copy link
Member

mrueg commented Mar 6, 2023

In my personal opinion kube-state-metrics is not meant to be used as a library and rather a standalone deployment.
I also don't know if the team has the desire and capacity to support this use case.
Have you looked at https://github.com/kubernetes-sigs/custom-metrics-apiserver? This might be a viable option for you.

@dgrisonnet
Copy link
Member

/assign
/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 9, 2023
@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 8, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 6, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 6, 2024
@rexagod
Copy link
Member

rexagod commented Jul 15, 2024

/remove-lifecycle rotten
/triage accepted
/assign

AFAICT KSM never explicitly supported stable library APIs, but nonetheless, they may still be in use by the community. I believe this needs some more thought.

@Garrybest Would you be available for bringing this up in next SIG call?

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

7 participants