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

Remove deprecated field metricName from Scalers. #4240

Closed
OsamaNabih opened this issue Feb 15, 2023 · 5 comments
Closed

Remove deprecated field metricName from Scalers. #4240

OsamaNabih opened this issue Feb 15, 2023 · 5 comments

Comments

@OsamaNabih
Copy link

OsamaNabih commented Feb 15, 2023

Proposal

As per the discussion here #4220
The metricName field will become optional and will be deprecated from Scalers in v2.12.
Label breaking-change should be added to this issue.

Use-Case

No response

Is this a feature you are interested in implementing yourself?

Yes

Anything else?

No response

@OsamaNabih OsamaNabih added feature-request All issues for new features that have not been committed to needs-discussion labels Feb 15, 2023
@tomkerkhove tomkerkhove moved this from Proposed to To Triage in Roadmap - KEDA Core Feb 16, 2023
@zroubalik zroubalik added breaking-change and removed needs-discussion feature-request All issues for new features that have not been committed to labels Feb 21, 2023
@JorTurFer JorTurFer moved this from To Triage to To Do in Roadmap - KEDA Core Mar 13, 2023
@solsson
Copy link
Contributor

solsson commented Jun 18, 2023

#4220 says

since we introduced mechanisms to ensure the unique metrics names inside the ScaledObject

Which mechanism is this this? For example with the Prometheus scaler, I can't find any docs on how to get a more descriptive name than for example s0-prometheus-prometheus.

@zroubalik
Copy link
Member

Please use trigger.name to optionally name your trigger. https://keda.sh/docs/2.10/concepts/scaling-deployments/#triggers

The previous metricName was confusing and used only internally.

@amirschw
Copy link
Contributor

We are in the process of removing this field as a prerequisite before upgrading to 2.12 and noticed we use the same metricName field in one of our external scalers. Our current plan is to rename it, while keeping support for the existing name for the duration of the migration, but just mentioning it here to point you to the fact that this statement is not 100% accurate:

The previous metricName was confusing and used only internally.

I'd consider highlighting this in the deprecation notice, or maybe even consider excluding external scaler from the deprecation, but that might be too much.

@hatfarm
Copy link

hatfarm commented Aug 1, 2023

I have a couple of concerns/question on this.

So, for the Prometheus scaler, there's no mention of using the name field to call out the specific name of the metric you're scaling on (but metricName is still used in the example).

The documentation is a bit unclear, but if we're using Name to be the name of the metric we're READING, would it also be used as the name of the metric being written by KEDA?

@zroubalik
Copy link
Member

@hatfarm trigger.name is being used by internal KEDA metrics (Prometheus, OTEL) to identify the specific metrics of a scaler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

6 participants