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

Google pubsub custom metrics support #2226

Closed
fira42073 opened this issue Oct 30, 2021 · 12 comments · Fixed by #2266 or kedacore/keda-docs#579
Closed

Google pubsub custom metrics support #2226

fira42073 opened this issue Oct 30, 2021 · 12 comments · Fixed by #2266 or kedacore/keda-docs#579
Labels
feature All issues for new features that have been committed to scaler-gcp-pub-sub

Comments

@fira42073
Copy link
Contributor

Proposal

As a devops, I want to be able to add custom metrics to my scaledobject like "Oldest unacked message age", which are provided by GCP metrics (stackdriver). The only trigger I can use now is subscriptionSize which is "Unacked message count" in GCP.

Use-Case

I want to scale my deployment if there are a lot of delays in message processing.

Anything else?

No response

@fira42073 fira42073 added feature-request All issues for new features that have not been committed to needs-discussion labels Oct 30, 2021
@tomkerkhove
Copy link
Member

Good suggestion, are you willing to contribute this?

@tomkerkhove tomkerkhove added feature All issues for new features that have been committed to scaler-gcp-pub-sub and removed needs-discussion feature-request All issues for new features that have not been committed to labels Oct 31, 2021
@fira42073
Copy link
Contributor Author

Yep, but I'll need some support to understand the project structure

@zroubalik
Copy link
Member

@Friedrich42 you can check the source code

func (s *pubsubScaler) GetSubscriptionSize(ctx context.Context) (int64, error) {

guides:
https://github.com/kedacore/keda/blob/main/CREATE-NEW-SCALER.md
https://github.com/kedacore/keda/blob/main/CONTRIBUTING.md
https://github.com/kedacore/keda/blob/main/BUILD.md

or feel free to ask on #keda or #keda-dev Slack channel on https://slack.k8s.io/

@fira42073
Copy link
Contributor Author

I think that there should be new parameter in metadata object named oldestUnackedMessageAge.

I wonder if it should be optional or required? Making it required seems unreasonable, because scaler object works without it as well, but what should the validation rules be if it will be optional? Should subscriptionSize be also optional, so user can select which metrics he needs for scaling?

Is there another scaler with several metrics, so I can take a look at it?

@zroubalik
Copy link
Member

https://keda.sh/docs/2.4/scalers/rabbitmq-queue/ has 2 modes.

In this case we can make it optional, but then validate that user specified at least one of the metrics. Do you think draft here how would the triggers spec look like and we can discuss on that?

@fira42073
Copy link
Contributor Author

fira42073 commented Nov 7, 2021

I think this should be ok enough. If one of the target metrics are more than specified, it should trigger scaling.

triggers:
- type: gcp-pubsub
  metadata:
    subscriptionSize: "5" # Optional - Default is 5
    oldestUnackedMessageAge: "1" # Optional - Default is 10 (seconds)
    subscriptionName: "mysubscription" # Required
    credentialsFromEnv: GOOGLE_APPLICATION_CREDENTIALS_JSON # Required

@zroubalik
Copy link
Member

@tomkerkhove WDYT?

@zroubalik
Copy link
Member

@Friedrich42 or we can allow only one of the metrics to be specified in the spec. And if users would like to scale on both of them, they can define multiple triggers in the ScaledObject.

@tomkerkhove
Copy link
Member

I like that proposal, for the rest it looks fine to me

@fira42073
Copy link
Contributor Author

fira42073 commented Nov 9, 2021

@zroubalik So I propose we do the same thing as in rabbitmq and we have mode field. Should look like this =>

triggers:
- type: gcp-pubsub
  metadata:
    mode: "SubscriptionSize" # Optional - Default is SubscriptionSize - either SubscriptionSize or OldestUnackedMessageAge
    value: "5" # Optional (active if mode is SubscriptionSize) - Default is 5 for subscriptionsize | (active if mode is OldestUnackedMessageAge) - Default is 10 (seconds)
    subscriptionName: "mysubscription" # Required
    credentialsFromEnv: GOOGLE_APPLICATION_CREDENTIALS_JSON # Required

@tomkerkhove
Copy link
Member

This might be fine, but we should at least be backwards compatible and deprecate the old field.

Would you mind updating the feature PR & docs to reflect that please?

@fira42073
Copy link
Contributor Author

I'll update both PRs soon to add backwards compatibility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature All issues for new features that have been committed to scaler-gcp-pub-sub
Projects
None yet
3 participants