[Autoscaler] Refactor metrics client by weilerN · Pull Request #83 · v3io/scaler

weilerN · 2026-01-06T09:39:00Z

📝 Description

This change refactors autoscaler metrics handling by introducing a dedicated MetricsClient abstraction and factory. The autoscaler no longer depends directly on Kubernetes custom metrics APIs and instead consumes metrics via a pluggable client interface.

🛠️ Changes Made

Introduced MetricsClient interface in scalertypes to abstract resource metrics retrieval.
Added MetricsClientOptions to autoscaler configuration to control metrics client configuration and selection.
Implemented CustomMetricsClient under pkg/autoscaler/metricsclient, encapsulating Kubernetes custom metrics logic.
Added a metrics client factory under pkg/autoscaler/metricsclient to construct the appropriate client based on configuration.
Refactored autoscaler to consume the MetricsClient interface instead of directly using Kubernetes custom metrics APIs.

✅ Checklist

I have tested the changes in this PR

🧪 Testing

Dev tests- deployed the nuclio-scaler and tested STZ
- Tested STZ on a function that created with the new scaler
- Backward compatibility test: Created a function, then replaced the scaler and validated that STZ works as expected.

🔗 References

Ticket link: https://iguazio.atlassian.net/browse/NUC-709
Design docs links:
External links: https://iguazio.atlassian.net/wiki/spaces/PLAT/pages/627572831/Nuclio+STZ+using+Prometheus+client+-+HLD#Refactor-existing-code-for-accepting-interface-for-GetResources

🚨 Breaking Changes?

Yes (explain below)
No

🔍️ Additional Notes

rokatyy

Very well! Just a couple of suggestions and questions

rokatyy · 2026-01-07T16:51:45Z

pkg/autoscaler/metricsclient/k8s.go

not sure I like the name, how about just custom? Because this are not metrics, the struct is for a client (a custom one), so either custom.go or customclient.go

it can also just be k8s.go, and the client name K8sCustomMetricsClient

Agreed that the naming here can be improved. fixing

Fixed here - CR1 - renamings and docstring

rokatyy · 2026-01-07T16:53:36Z

pkg/autoscaler/metricsclient/custommetrics.go

+type CustomMetricsClient struct {
+	k8scustommetrics.CustomMetricsClient
+	namespace string
+	groupKind schema.GroupKind
+	logger    logger.Logger
+}


I assume some of those values are expected to be common across all the interface implementations? Do we want to have an abstract class or it would be redundant?

@rokatyy Great question!
At the moment, the only shared values across implementations are the namespace and the logger, so I prefer to keep the current structure as is.

That said, if we start seeing duplicated state or logic (during the Prometheus client implementation), I’ll definitely consider introducing a common abstract struct to avoid repetition.

rokatyy · 2026-01-07T16:55:35Z

pkg/autoscaler/metricsclient/factory.go

+			autoScalerConf.Namespace,
+			autoScalerConf.GroupKind), nil
+	default:
+		return nil, fmt.Errorf("unsupported metrics client kind: %s", autoScalerConf.MetricsClientOptions.Kind)


use errors instead of fmt

Fixed here - CR1 - renamings and docstring

rokatyy · 2026-01-07T16:59:01Z

pkg/scalertypes/types.go

 	"k8s.io/client-go/kubernetes"
 )

+type Kind string


Kind is too generic, let's change to MetricsClientKind or something more informative

Fixed here - CR1 - renamings and docstring

TomerShor · 2026-01-08T07:14:50Z

cmd/autoscaler/app/autoscaler.go

 			Group: metricsResourceGroup,
 		},
+		MetricsClientOptions: scalertypes.MetricsClientOptions{
+			Kind: scalertypes.KindCustomMetrics,


we not getting this as an input instead of hardcoding it to custom metrics?

Discussed F2F, comments will be added to improve readiness

Fixed here - CR1 - renamings and docstring

TomerShor · 2026-01-08T07:17:22Z

pkg/autoscaler/metricsclient/k8s.go

it can also just be k8s.go, and the client name K8sCustomMetricsClient

pkg/autoscaler/metricsclient/k8s.go

TomerShor · 2026-01-08T07:20:21Z

pkg/autoscaler/metricsclient/custommetrics.go

+	namespace string,
+	groupKind schema.GroupKind) *CustomMetricsClient {
+	return &CustomMetricsClient{
+		logger:              logger,


Suggested change

logger: logger,

logger: parentLogger.GetChild("custom-metrics"),

Fixed here - CR1 - renamings and docstring

pkg/scalertypes/types.go

TomerShor · 2026-01-08T13:40:46Z

pkg/autoscaler/metricsclient/factory.go

+	case scalertypes.KindK8sMetricsClient:
+		discoveryClient, err := discovery.NewDiscoveryClientForConfig(restConfig)
+		if err != nil {
+			return nil, errors.Wrap(err, "Failed to create k8s metrics client")
+		}
+		availableAPIsGetter := k8scustommetrics.NewAvailableAPIsGetter(discoveryClient)
+		restMapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(discoveryClient))
+		customMetricsClient := k8scustommetrics.NewForConfig(restConfig, restMapper, availableAPIsGetter)
+		return NewCustomMetricsClient(
+			logger,
+			customMetricsClient,
+			autoScalerConf.Namespace,
+			autoScalerConf.GroupKind), nil


No, Move all of this into NewCustomMetricsClient.
The factory needs to be simple - call the constructor, return the instance.
The constructor tells what it needs the factory to pass as arguments, and it defines any intermediate types etc.

Fixed here - CR2 - init custom metrics client in the proper location

rokatyy

LGTM

TomerShor

Good job 👍🏼

weilerN added 5 commits January 6, 2026 10:56

Add Metrics client interface

39177e8

Refactor custom metrics client

1a81301

Refactor metrics client param in Autoscaler struct

c8e45f0

Use factory to create MetricsClient instance

150c33b

Update metrics client options

2db6008

weilerN force-pushed the NUC-709-refactor_metrics_client_in_autoscaler branch 3 times, most recently from 779eb29 to 521e536 Compare January 7, 2026 15:40

Renamings

44b998c

weilerN force-pushed the NUC-709-refactor_metrics_client_in_autoscaler branch from d1ed7fe to 44b998c Compare January 7, 2026 15:44

weilerN requested review from TomerShor and rokatyy January 7, 2026 16:27

weilerN marked this pull request as ready for review January 7, 2026 16:27

rokatyy reviewed Jan 7, 2026

View reviewed changes

TomerShor changed the title ~~Nuc 709 refactor metrics client in autoscaler~~ [Autoscaler] Refactor metrics client Jan 8, 2026

TomerShor requested changes Jan 8, 2026

View reviewed changes

CR1 - renamings and docstring

ce5c46b

weilerN requested review from TomerShor and rokatyy January 8, 2026 13:16

TomerShor requested changes Jan 8, 2026

View reviewed changes

CR2 - init custom metrics client in the proper location

22b5f89

weilerN requested a review from TomerShor January 8, 2026 14:28

rokatyy approved these changes Jan 8, 2026

View reviewed changes

TomerShor approved these changes Jan 8, 2026

View reviewed changes

TomerShor merged commit 72bd925 into v3io:development Jan 8, 2026
3 checks passed

	logger: logger,
	logger: parentLogger.GetChild("custom-metrics"),

Conversation

weilerN commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

🛠️ Changes Made

✅ Checklist

🧪 Testing

🔗 References

🚨 Breaking Changes?

🔍️ Additional Notes

Uh oh!

rokatyy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rokatyy left a comment

Choose a reason for hiding this comment

Uh oh!

TomerShor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

weilerN commented Jan 6, 2026 •

edited

Loading