Read metrics Datasource configuration from config file by Mohamedma96 · Pull Request #2441 · kubernetes-sigs/gateway-api-inference-extension

Mohamedma96 · 2026-03-01T02:14:31Z

Fixes:
issue #2440

What this PR does
the metrics-data-source DataLayer plugin read its configuration (scheme, path, insecureSkipVerify) from CLI flags (--model-server-metrics-scheme, --model-server-metrics-path, --model-server-metrics-https-insecure-skip-verify) via flag.Lookup(). This tightly coupled the plugin to the CLI layer .

This PR decouples the plugin from CLI flags by reading its configuration from the plugin's parameters field in the config.

Changes:

Removed direct pflag / flag.Lookup() usage.
CLI flags deprecated (but still supported)
add default values when the values are missing in the config.

TODO:

Add specific test case to check this behavior.

netlify · 2026-03-01T02:14:37Z

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	`68eb26f`
🔍 Latest deploy log	https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69a6e1467f84e100082e5bdd
😎 Deploy Preview	https://deploy-preview-2441--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2026-03-01T02:14:41Z

Hi @Mohamedma96. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

nirrozenbaum · 2026-03-01T07:33:50Z

/ok-to-test
/cc @elevran

shmuelk · 2026-03-01T16:31:13Z

This PR looks quite good. I have two questions:

Usually when something is deprecated, it still works, just gives a message, usually for a release. This PR removes the old command line argument support. Why wasn't the support left in?
Why were the go.mod and go.sum files updated?

elevran · 2026-03-01T17:00:35Z

nit: @Mohamedma96 I think PR description comment should say Fixes or Fix if you want to automatically close an issue, not Solves.

Usually when something is deprecated, it still works, just gives a message, usually for a release. This PR removes the old command line argument support. Why wasn't the support left in?

The command line options are marked deprecated in options.go, so agree with @shmuelk that we should still refer to them.
The precedence shuold be default < cli < config file.

Why were the go.mod and go.sum files updated?

I presume it is not intentional. @Mohamedma96 ?

ahg-g · 2026-03-01T18:47:32Z

+        scheme: {{ .Values.inferenceExtension.metricsDataSource.scheme | default "http" | quote }}
+        path: {{ .Values.inferenceExtension.metricsDataSource.path | default "/metrics" | quote }}
+        insecureSkipVerify: {{ .Values.inferenceExtension.metricsDataSource.insecureSkipVerify | default true }}
+    - type: model-server-protocol-metrics


@elevran should we rename the extractor plugin to metrics-extractor?

It is for a well scoped specific set of metrics so I think the MSP prefix makes sense.
If we need other metrics extracted, they shuold be using a different Extractor over the same source and setting the attributes on the endpoint.

At least the naming can be consistent with all other plugins suffixed with their main function, in this case suffixed with "metrics-extractor". But then it will be too long model-server-protocol-metrics-extractor, so how about core-metrics-extractor?

yeah, +1 for consistency. Either core-metrics-extractor or msp-metrics-extractor.

msp is not a well established/known term, and generally we better avoid abbreviations in ux, so I would lean towards "core" as the prefix with good documentation

I'm fine with core

@Mohamedma96 can we pls change the plugin name here in the config and the code from model-server-protocol-metrics to core-metrics-extractor

Done.
Please re-review.

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

ahg-g · 2026-03-02T14:00:14Z

/lgtm
/approve

k8s-ci-robot · 2026-03-02T14:00:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, Mohamedma96

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ahg-g]

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

ahg-g · 2026-03-02T14:00:25Z

/hold

ahg-g · 2026-03-02T14:01:38Z

this got reverted already

… set/unset Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

elevran

left two comments, neither is a blocker. Can be done subsequently from my point of view.

elevran · 2026-03-03T13:11:39Z

@@ -115,7 +115,7 @@ func defaultDataSourceConfigParams() (*metricsDatasourceParams, error) {
 // The second return value is false when the flag is not registered; no error is returned in that case.


nit: comment should be changed to reflect ... false when the flag is not registered or not explicitly set by the user?

elevran · 2026-03-03T15:28:40Z

+	"github.com/spf13/pflag"
+)
+
+func resetFlags() {


The global pflag.CommandLine is shared. If one test modifies it, subsequent tests will see those changes. It is cleaner to localize the flagset and revert it (e.g., using t.Cleanup()).

func TestSomeFlags(t *testing.T) { oldCommandLine := flag.CommandLine // save global FlagSet t.Cleanup(func() { // revert to previous at the end of the test pflag.CommandLine = oldCommandLine }) // create a fresh FlagSet for this test pflag.CommandLine = flag.NewFlagSet("test", flag.ContinueOnError) ... define flags, parse them and validate using test logic }```

elevran · 2026-03-04T04:10:42Z

/lgtm

ahg-g · 2026-03-04T11:41:47Z

/hold cancel

…gs#2441) * Read metrics Datasource configuration from config file Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * Add MetricsDataSource to values, update go.mod Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * rename plugin, keep supporting command line flags for depracated flags Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * revert pflag import alias Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * Add test for config precedence, add f.Changed check to check cli flag set/unset Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * rename factory and New method to match new naming Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> --------- Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

Allow configuring a separate port for scraping model server metrics via EndpointPickerConfig plugin parameters. This addresses the gap left by the deprecation of --model-server-metrics-port (PR kubernetes-sigs#1886, kubernetes-sigs#2441) which had no replacement for the port configuration. When metricsPort is set in the metrics-data-source plugin parameters, it overrides the inference port encoded in the endpoint's MetricsHost. This enables deployments where model servers expose metrics on a separate port (e.g., vLLM with --metrics-port) from inference traffic, which is required in Istio mTLS STRICT environments. Related: kubernetes-sigs#1396, kubernetes-sigs#1556

--model-server-metrics-port was fully removed from options.go in kubernetes-sigs#2441, so pflag.Lookup would always return nil — making fromIntFlag a no-op. metricsPort is now configured exclusively via EndpointPickerConfig parameters. Also tighten NewHTTPDataSource comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds MetricsPort to metricsDatasourceParams so the metrics-data-source plugin can scrape a port different from the inference port. When metricsPort is non-zero in EndpointPickerConfig plugin parameters, it overrides the port derived from the endpoint's MetricsHost; zero (default) preserves existing behavior. This restores functionality that was available via --model-server-metrics-port but lost when that flag was removed in kubernetes-sigs#2441 without an equivalent plugin config parameter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds MetricsPort to metricsDatasourceParams so the metrics-data-source plugin can scrape a port different from the inference port. When metricsPort is non-zero in EndpointPickerConfig plugin parameters, it overrides the port derived from the endpoint's MetricsHost; zero (default) preserves existing behavior. This restores functionality that was available via --model-server-metrics-port but lost when that flag was removed in kubernetes-sigs#2441 without an equivalent plugin config parameter.

…gs/gateway-api-inference-extension#2441) * Read metrics Datasource configuration from config file Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * Add MetricsDataSource to values, update go.mod Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * rename plugin, keep supporting command line flags for depracated flags Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * revert pflag import alias Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * Add test for config precedence, add f.Changed check to check cli flag set/unset Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> * rename factory and New method to match new naming Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com> --------- Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

k8s-ci-robot requested review from kfswain and robscott March 1, 2026 02:14

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 1, 2026

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2026

k8s-ci-robot requested a review from elevran March 1, 2026 07:33

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 1, 2026

ahg-g reviewed Mar 1, 2026

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 1, 2026

Mohamedma96 added 3 commits March 2, 2026 15:25

Read metrics Datasource configuration from config file

7576d15

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

Add MetricsDataSource to values, update go.mod

8b45cad

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

rename plugin, keep supporting command line flags for depracated flags

5450baa

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

Mohamedma96 force-pushed the metrics-datasource-config-params branch from ef5c1c6 to 5450baa Compare March 2, 2026 13:32

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 2, 2026

Mohamedma96 requested a review from ahg-g March 2, 2026 13:38

ahg-g reviewed Mar 2, 2026

View reviewed changes

Comment thread pkg/epp/framework/plugins/datalayer/source/metrics/datasource.go Outdated

revert pflag import alias

8470a96

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

k8s-ci-robot assigned ahg-g Mar 2, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 2, 2026

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 2, 2026

ahg-g reviewed Mar 2, 2026

View reviewed changes

Add test for config precedence, add f.Changed check to check cli flag…

62d2fc2

… set/unset Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2026

rename factory and New method to match new naming

68eb26f

Signed-off-by: mohamedmahameed <mohamed.mahameed@ibm.com>

Mohamedma96 requested a review from ahg-g March 3, 2026 13:25

elevran reviewed Mar 3, 2026

View reviewed changes

k8s-ci-robot assigned elevran Mar 4, 2026

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2026

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2026

k8s-ci-robot merged commit 1e9ea89 into kubernetes-sigs:main Mar 4, 2026
9 checks passed

janghyukjin mentioned this pull request Apr 2, 2026

feat(datalayer): add metricsPort parameter to metrics-data-source plugin #2762

Closed

		@@ -115,7 +115,7 @@ func defaultDataSourceConfigParams() (*metricsDatasourceParams, error) {
		// The second return value is false when the flag is not registered; no error is returned in that case.

Conversation

Mohamedma96 commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for gateway-api-inference-extension ready!

Uh oh!

k8s-ci-robot commented Mar 1, 2026

Uh oh!

nirrozenbaum commented Mar 1, 2026

Uh oh!

shmuelk commented Mar 1, 2026

Uh oh!

elevran commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahg-g Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ahg-g commented Mar 2, 2026

Uh oh!

k8s-ci-robot commented Mar 2, 2026

Uh oh!

ahg-g commented Mar 2, 2026

Uh oh!

ahg-g Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elevran 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!

elevran commented Mar 4, 2026

Uh oh!

ahg-g commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Mohamedma96 commented Mar 1, 2026 •

edited

Loading

netlify Bot commented Mar 1, 2026 •

edited

Loading

elevran commented Mar 1, 2026 •

edited

Loading

ahg-g Mar 1, 2026 •

edited

Loading

ahg-g Mar 2, 2026 •

edited

Loading