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

Improve metric name creation to be unique #2123

Closed
39 tasks
tomkerkhove opened this issue Sep 28, 2021 · 6 comments · Fixed by #2161
Closed
39 tasks

Improve metric name creation to be unique #2123

tomkerkhove opened this issue Sep 28, 2021 · 6 comments · Fixed by #2161
Labels
enhancement New feature or request

Comments

@tomkerkhove
Copy link
Member

tomkerkhove commented Sep 28, 2021

Proposal

Improve metric name creation to be unique by:

  • Using a hash of the complete trigger metadata, or
  • Define a custom name through metricName

Use-Case

This is required when using multiple triggers in the same ScaledObject so that they are unique, but end-users still have control.

Scalers

  • ActiveMQ Artemis
  • Apache Kafka
  • AWS CloudWatch
  • AWS Kinesis Stream
  • AWS SQS Queue
  • Azure Blob Storage
  • Azure Event Hubs
  • Azure Log Analytics
  • Azure Monitor
  • Azure Pipelines
  • Azure Service Bus
  • Azure Storage Queue
  • CPU
  • Cron
  • External
  • External Push
  • Google Cloud Platform‎ Pub/Sub
  • Huawei Cloudeye
  • IBM MQ
  • InfluxDB
  • Kubernetes Workload
  • Liiklus Topic
  • Memory
  • Metrics API
  • MongoDB
  • MSSQL
  • MySQL
  • NATS Streaming
  • OpenStack Metric
  • OpenStack Swift
  • PostgreSQL
  • Prometheus
  • RabbitMQ Queue
  • Redis Lists
  • Redis Lists (supports Redis Cluster)
  • Redis Streams
  • Redis Streams (supports Redis Cluster)
  • Selenium Grid Scaler
  • Solace PubSub+ Event Broker

Anything else?

We need to verify what scalers already allow to provide a metric name and add the same behavior there.

Relates to #2100

@JorTurFer
Copy link
Member

If there is not any volunteer, I can help with this :)

@tomkerkhove
Copy link
Member Author

Certainly, thanks a ton!

@zroubalik
Copy link
Member

I am not sure hash is the best approach, the metric name becomes quite ugly. What if we index the scalers in the ScaledObject 0-x and use this index in the metric name?

@JorTurFer
Copy link
Member

JorTurFer commented Oct 4, 2021

Honestly, this sounds better to me. We were talking about it and we decided to let users give their own name because the hashing method generates weird names. Maybe we can try to do something like Ix-currentName
Another good point is that if we can ensure that metric name is unique, maybe we'd be able to deprecate the support of setting custom metric names in the future if we'd want

@JorTurFer
Copy link
Member

I will try this new approach in the afternoon :)

@JorTurFer
Copy link
Member

I have tried it and it seems doable.
My only concern is if we should add for example the scaledobject name as part of metric name. I mean, if we have the same metric in different scaledobject (same trigger, etc) we are going to have 2 metrics with the same name. The HPA handles it using extra info like match labels, but this could be a bit ugly if you need to query the metric directly using kubectl for debugging purposes.
WDTY?
In any case, I will create a draft with this new approach in several scalers (adding or not the scaler name is not relevant). You can take a look at it when it is ready and share your opinion

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