-
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
External metric provider via HTTP #929
Comments
@tomkerkhove @zroubalik what do you think? I should be able to implement this one 😉 |
Just playing devils advocate but how would it be different from the external scaler? Because you just provide the current value to scale on instead of making the decision (and http instead of GRPC) ? What's your usecase if I may ask? |
TBH I'm not sure if I get exactly how the external scaler works (the example and docs are quite vague with the redis example). Usecase from sig-autoscaling slack channel:
But apart from that, I was thinking about similar usecases. Moreover, for some users, it may be more secure to add an endpoint than for example query Postgres/MySQL with arbitrary sql. |
An external scaler should be able to implement the same Scaler interface we have in the repo. It adds an extra server that needs to be managed, but it simplifies things like handling a custom authentication mechanism, different metrics schemas, aggregation, etc.
|
Thanks for explaining.
I would say that BA is good starting point
+1 for stastd, in that case I would add
|
API Key is lowest barrier to start with if you ask me, but basic auth is good for me. What do you think of this API spec? openapi: 3.0.1
info:
title: KEDA - External Metric Source Petstore
description: >-
This is the specification to comply with for external KEDA metric sources.
termsOfService: 'http://swagger.io/terms/'
contact:
email: apiteam@swagger.io
license:
name: Apache 2.0
url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
version: 1.0.0
externalDocs:
description: Find out more about Swagger
url: 'http://swagger.io'
servers:
- url: 'https://samples.keda.sh/'
tags:
- name: Health
description: Operations about runtime health
- name: Metrics
description: Operations about providing metrics to KEDA
paths:
/api/v1/metric/{metric}:
get:
summary: Get Metric
description: Get value for a given metric
operationId: GetMetric
parameters:
- name: metric
in: path
required: true
schema:
type: string
description: Name of the metric for which information is requested
responses:
'200':
description: Metric inforation was provided
content:
application/json:
schema:
$ref: '#/components/schemas/ReportedMetric'
'404':
description: Metric is not supported
'401':
$ref: '#/components/responses/UnauthorizedError'
'500':
description: Unable to determine metric information
tags:
- Metrics
security:
- basic_auth: []
- api_key: []
/api/v1/metrics:
get:
summary: Get All Metrics
description: Provides a list of all supported metrics
operationId: GetMetrics
responses:
'200':
description: Metric inforation was provided
content:
application/json:
schema:
$ref: '#/components/schemas/MetricInfo'
'401':
$ref: '#/components/responses/UnauthorizedError'
'500':
description: Unable to determine metric information
tags:
- Metrics
security:
- basic_auth: []
- api_key: []
/api/v1/health:
get:
summary: Get Health
description: Provides information about the health of the runtime
operationId: GetHealth
responses:
'200':
description: External metric source is healthy
'401':
$ref: '#/components/responses/UnauthorizedError'
'500':
description: External metric source is not healthy
tags:
- Health
security:
- basic_auth: []
- api_key: []
components:
schemas:
MetricInfo:
type: array
items:
type: object
properties:
name:
type: string
ReportedMetric:
type: object
required:
- name
- value
properties:
name:
type: string
value:
type: number
responses:
UnauthorizedError:
description: Authentication information is missing or invalid
securitySchemes:
basic_auth:
type: http
scheme: basic
api_key:
type: apiKey
name: X-API-Key
in: header
security:
- basic_auth: []
- api_key: []
By doing this we can:
@ahmelsayed @zroubalik Now we need to rethink things a bit if we add this because we have For example:
|
Maybe we should move this to a design doc? |
@tomkerkhove do you mean for test purposes or as API requirements for the scaler to work? |
I'd use that as an API requirement so that people know what to comply with. |
Hey guys, I'm the source of this request so I'll help in anyway I can, although I have not worked with Keda internals at all so my help may be limited to testing :) First of all, a big thanks to @turbaszek for starting this. The example given in the top comment is pretty much what I had in mind. I personally like the idea of using API keys for auth, but BA would be fine, too. My use case for this is purely internal right now so either of those is easy to start with. I can't tell if this has been decided or not, but what are everyone's thoughts on how the returned value should be used? Some possibilities I've thought of:
|
Thanks for jumping in @lonnix! We already have an external scaler which does the calculation of the desired instances by using gRPC which we are improving the docs for, but that should fix your scenario. For this one, I personally see it more as the metric provider and we'll make sure that it scales based on that so you don't have to jump through the hoops or we can apply the same pattern. If that is not ideal, then I'm not sure if we should support this in favor of the current external scaler. |
Could you elaborate on this (or just let me know when the new docs are there)? My original thought was HTTP because that's easy to add to our REST APIs but I'm not against a different protocol if it gets the job done. |
We currently allow you to bring your own scaler which integrates with a system but our docs are poor for now (see issue) but the idea is that you have to determine how many instances are required, etc. So you don't provide the metric value, but the instance count. Next to that, we will allow you to use external push triggers in 2.0 for which we are adding docs in kedacore/keda-docs#193 (feature) |
Other example where HTTP scaler would be useful are Python apps that use Celery and its monitoring tool called Flower which expose quite interesting API (FYI @auvipy @dimberman ):
This sounds very interesting but it has an overhead of building a scaler if I understand correctly. That for some teams can be a blocker (due to lack of go knowledge, lack of resources, anything). Being able to use an API endpoint sounds more straightforward to me. |
Be back soon to this. Have plan to rewrite flower soon. |
I can see value in a "we provide the metric, you'll handle the rest" API endpoint. |
As for what the spec and expected requirements for such a scaler might look like, we can take a look at Zabbix, which allows you to collect HTTP metrics. If someone wants to use the token for authorization, they must add It's worth taking a look at the "Postprocessing" section, which has no documentation, but I attach a screenshot of the interface. |
What concerns do you have with #929 (comment) @mik-laj ? |
@tomkerkhove I will draft a PR soon |
I presume you agree on the proposed API spec then or? |
Yup, that makes sense to me 👍 |
Just want to know, how keda could be useful with upcoming airship 2.0 ? https://www.airshipit.org/blog/airship2-is-alpha/ |
@tomkerkhove @ahmelsayed I just realized that the openapi spec excludes the possibility of using jsonpath/statsd as mentioned #929 (comment) |
Closes: kedacore#929 Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
Closes: kedacore#929 Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
Closes: kedacore#929 Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
We've agreed on the PR that the path can be different but that the details of the operation in the OpenAPI spec have to comply. So you can use |
Dealing with OpenAPI specs is something fairly new to me, so forgive any misconceptions I may have here, but it seems like the spec is being enforced but doesn't really need to be. The api spec has 3 endpoints
I have seen companies use I don't understand where #2 comes into play. Why would you need a list of all metrics if the metric name is provided in the ScaledObject and you have an endpoint to fetch that metric? And why would 2 endpoints be needed if one provides all the metrics and you know which metric to parse via the ScaledObject? For number 3, why not just have the ScaledObject take a single parameter, the url, that returns something of a specified format? Example:
vs
To me it seems like if we're trying to write a scaler for an external source we can't rely on that external source following a spec we defined because it may not be something we control. The original proposal of providing an endpoint and metric name would cover literally any api that returns json. I think it is fair to say we require the response to be of a certain format so that we can ensure parsing, but if we're doing json parsing then even that should be very flexible. Requiring a specific url path when the path is something that is provided to the scaler seems contradictory. |
In #929 (comment) I supported the current approach but the longer I think about it the more doubts I have about the open api spec. It's a nice pattern but it enforces users to structure their API in a specific way. The main advantage of this approach is simplicity of the scaler. However, seeing how many doubts it rises I would like to suggest to consider my initial proposal where users provide: url: http://my-resource:3001/some/stats/endpoint
valueLocation: stats.magic_resource.value Then all scalers do is send GET request to Advantages of this approach are:
In my opinion 2nd point is crucial from the KEDA adoption standpoint. @tomkerkhove @zroubalik @ahmelsayed I think this is something we should consider making the final decision. The scaler will be slightly more complicated (but far from rocket science) but I think this is a low price fo ease of adoption. |
This approach satisfies it initial request (made by me) and is super flexible. The nested json means some apis won't even have to create new endpoints, they can immediately start using any data that they're already publishing via a json endoint. It will have to be clear what that data needs to represent and I think what we've agreed upon already is a great solution. |
Yes, that's something we've agreed upon during the implementation. The spec here was just a proposal but we will:
So it will be used as guidance and not being enforced. But we have to provide something so that people know what the API has to return for which OpenAPI is a good way of documenting given it's an industry standard for that. |
I think using jsonpath to "tell us where in the response is the number we should use as input" is still a lower barrier than enforcing object structure. |
These decrease adoption in my opinion as they have a certain barrier for new people but wondering what @zroubalik @ahmelsayed think. I think it's fair to have a response structure to comply with but might be just me. |
I think it's exactly opposite because users are not bound to KEDA defined response structure. If someone wants to follow Open API spec then they simply specify With good documentation, everyone should be happy :) |
I think this is key. I don't see a problem for both keda-familiar and keda-unfamiliar users adopting this as long as we have clear documentation on what needs to be returned and how it works. |
@ahmelsayed @zroubalik what is your opinion? Should we stick to open api or allow more flexibility as discussed above? |
Use json path then. We can still make it optional if we want to go the OpenAPI route. |
@tomkerkhove thanks! I will do this in the next few days |
It seems that there's no "popular" library to support jsonpath in Golang. However, there's a https://github.com/tidwall/gjson which proposes similar path syntax which in our case maybe even better because it allows also to get a length of a list. What is more, we may consider adding custom modifiers that will allow users to get average, max, or any other statistic. |
Closes: kedacore#929 Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
We haven't covered the auth side of this scaler in the implementation. We can park it to later but maybe good to open some issues then. Thoughts? |
Re-opening as docs are not done yet |
Here we go, added some basic auth scenarios. THanks for the JWT one @turbaszek! |
Closing as docs are merged and issues for auth are created 🚀 |
* chore: align external scaler formatting with built-in Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: format external scaler to maintain same style with built-in Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: make result count change when toggled Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: make search work for external scalers Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: fix layout to be 2 columns instead Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: hide banner during external scaler search Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: nit fix Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: prevent built-in search from populating external scaler section Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: some nit fixes Signed-off-by: thisisobate <obasiuche62@gmail.com> * chore: nit fixes Signed-off-by: thisisobate <obasiuche62@gmail.com> Signed-off-by: thisisobate <obasiuche62@gmail.com>
Scaler Source:
This scaler will send a GET request to an API that returns a JSON response.
How do you want to scale:
Users can access numeric value in the API response that will be used as a current value.
Authentication:
Not sure about this one but probably some headers. Not sure if we want to authenticate with each request. Eventually, we can start with public endpoints.
Let's consider an example. My application has an endpoint that returns some useful statistics which I would love to use as a source of information for HPA. When requested it returns the following response:
To access this value I have to specify
valueLocation
(as injq
). ExampleScaledObject
:This scaler is inspired by slack question:
https://kubernetes.slack.com/archives/C09R1LV8S/p1594244628163800
The text was updated successfully, but these errors were encountered: