-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
metricName
should be deprecated for Cassandra Scaler
#4227
Comments
I'd like to give it a shot. It'll be my first time to KEDA. I've read --- a/pkg/scalers/cassandra_scaler.go
+++ b/pkg/scalers/cassandra_scaler.go
@@ -48,7 +48,7 @@ func NewCassandraScaler(config *ScalerConfig) (Scaler, error) {
logger := InitializeLogger(config, "cassandra_scaler")
- meta, err := parseCassandraMetadata(config)
+ meta, err := parseCassandraMetadata(config, logger)
if err != nil {
return nil, fmt.Errorf("error parsing cassandra metadata: %w", err)
}
@@ -67,7 +67,7 @@ func NewCassandraScaler(config *ScalerConfig) (Scaler, error) {
}
// parseCassandraMetadata parses the metadata and returns a CassandraMetadata or an error if the ScalerConfig is invalid.
-func parseCassandraMetadata(config *ScalerConfig) (*CassandraMetadata, error) {
+func parseCassandraMetadata(config *ScalerConfig, logger logr.Logger) (*CassandraMetadata, error) {
meta := CassandraMetadata{}
if val, ok := config.TriggerMetadata["query"]; ok {
@@ -150,6 +150,7 @@ func parseCassandraMetadata(config *ScalerConfig) (*CassandraMetadata, error) {
if val, ok := config.TriggerMetadata["metricName"]; ok {
meta.metricName = kedautil.NormalizeString(fmt.Sprintf("cassandra-%s", val))
+ logger.Info("metricName is deprecated and shouldn't be explicitly set in scaler trigger metadata")
} else {
meta.metricName = kedautil.NormalizeString(fmt.Sprintf("cassandra-%s", meta.keyspace))
} |
Yeah, that draft looks nice. The idea is to announce to end-users that the feature will be removed in v2.12 so maybe I'd include that info in the log, but that's all |
Do I need to make any changes to |
I think that we can refer the main issue directly in the changelog to reduce the noise: But yes, the first one who fixes one of the sub issues should update the changelog xD |
will be handled by #4220 |
Proposal
Currently, Cassandra scaler requires a
metricName
which is used only for KEDA internal things (it's not sent to the prom server).This requirement isn't necessary since we introduced mechanisms to ensure the unique metrics names inside the ScaledObject and we should make it optional and also deprecate it in favor of self-generated internal name
https://github.com/kedacore/keda/blob/main/pkg/scalers/cassandra_scaler.go#L151-L155
The text was updated successfully, but these errors were encountered: