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

External metric provider via HTTP #929

Closed
turbaszek opened this issue Jul 10, 2020 · 46 comments · Fixed by #1026
Closed

External metric provider via HTTP #929

turbaszek opened this issue Jul 10, 2020 · 46 comments · Fixed by #1026
Labels
feature All issues for new features that have been committed to scaler scaler-metrics-api All issues related to our Metrics API scaler
Milestone

Comments

@turbaszek
Copy link
Contributor

  • 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:

{"stats": {"magic_resource": {"value": 42}}}

To access this value I have to specify valueLocation (as in jq). Example ScaledObject:

apiVersion: keda.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: api-scaledobject
  namespace: my-project
spec:
  scaleTargetRef:
    deploymentName: worker
  triggers:
  - type: api-request
    metadata:
      targetValue: 42
      url: http://my-resource:3001/some/stats/endpoint
      valueLocation: stats.magic_resource.value

This scaler is inspired by slack question:
https://kubernetes.slack.com/archives/C09R1LV8S/p1594244628163800

@turbaszek
Copy link
Contributor Author

@tomkerkhove @zroubalik what do you think? I should be able to implement this one 😉

@tomkerkhove
Copy link
Member

tomkerkhove commented Jul 10, 2020

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?

@turbaszek
Copy link
Contributor Author

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:

Does anyone know if there is something out there for expanding the HPA to use custom URL endpoints that return json? I'd like to write my own app that has various endpoints for different things at my company then have the HPA hit those endpoints and scale based on the results. This way I could have one app where I setup scaling triggers (time of day, entries into db tables by other teams, when another team does X, trigger scaling, etc....) and all the apps could have an HPA that hits a different endpoint on my app. I'm fine with writing something if there's nothing already out there

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.

@ahmelsayed
Copy link
Contributor

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.
I think this generic approach for an HTTP query like this would be useful as it complements the prometheus scaler, but more flexible/developer-friendly. Though I have a couple of questions.

  • Authentication: assuming this is an HTTP api, there are few common authentication mechanisms for http apis that we might wanna consider. There are certain standard mechanisms that are straightforward (basic auth, client cert, api key) and ones that might be a bit more involved (oath). There is also the pod serviceAccount token, and tokens we can get from the various pod identity providers. Just wondering what your thoughts are around this.
  • I think supporting JSON or statsd format would be a plus (statsd format <metricName>:<metricValue>|<metricType>)
  • jq vs jsonpath: Not sure how you were thinking of implementing jq query syntax here (exec ./jq?). Kubernetes client-go/utils uses jsonpath so it might be more consistent, but I don't have a strong preference.

@turbaszek
Copy link
Contributor Author

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 think this generic approach for an HTTP query like this would be useful as it complements the prometheus scaler, but more flexible/developer-friendly. Though I have a couple of questions.

  • Authentication: assuming this is an HTTP api, there are few common authentication mechanisms for http apis that we might wanna consider. There are certain standard mechanisms that are straightforward (basic auth, client cert, api key) and ones that might be a bit more involved (oath). There is also the pod serviceAccount token, and tokens we can get from the various pod identity providers. Just wondering what your thoughts are around this.

I would say that BA is good starting point

  • I think supporting JSON or statsd format would be a plus (statsd format <metricName>:<metricValue>|<metricType>)

+1 for stastd, in that case I would add pathType to spec to ease resolving statsd vs json with json as default.

  • jq vs jsonpath: Not sure how you were thinking of implementing jq query syntax here (exec ./jq?). Kubernetes client-go/utils uses jsonpath so it might be more consistent, but I don't have a strong preference.

jq was just the first idea I had. I think jsonpath is better. Additionally, I was thinking about adding some estimators like len, sum or avg so user can do $.stats[:].value|avg WDYT?

@tomkerkhove
Copy link
Member

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:

  • Report metric values with api/v1/metric/{name}
  • Users can discover metrics and KEDA can validate specs via api/v1/metrics
  • KEDA can check health of it via api/v1/health

@ahmelsayed @zroubalik Now we need to rethink things a bit if we add this because we have external, external-push and now external-http or so? We should revise if these are the right names for 2.0 or if we change them.

For example:

  • external -> external-scaler - Implements a scaler
  • external-push -> external-scale-trigger/external-trigger-push - Trigger on demand
  • external-metric-source - This new HTTP based approach

@tomkerkhove
Copy link
Member

Maybe we should move this to a design doc?

@tomkerkhove tomkerkhove changed the title HTTP request scaler External metric provider via HTTP Jul 15, 2020
@turbaszek
Copy link
Contributor Author

What do you think of this API spec?

@tomkerkhove do you mean for test purposes or as API requirements for the scaler to work?

@tomkerkhove
Copy link
Member

I'd use that as an API requirement so that people know what to comply with.

@lonnix
Copy link

lonnix commented Jul 15, 2020

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:

  • the number of desired instances
  • a "usage" to be compared to a limit. Example: the ScaledObject takes additional values called "limit", "percent" and "maxInstances" and will scale up to in increments of 1 if the returned value is at least of , similar to how the HPA handles cpu and memory scaling
  • a "up/down" indicator, specifying scale up or scale down. Anything > 0 means scale up that many instances, anything < 0 means scale down that many instances. Use case: This lets the app reporting the value to specify larger scaling factors if there's a spike in traffic

@tomkerkhove
Copy link
Member

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.

@lonnix
Copy link

lonnix commented Jul 23, 2020

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.

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.

@tomkerkhove
Copy link
Member

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)

@turbaszek
Copy link
Contributor Author

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 ):
https://flower.readthedocs.io/en/latest/api.html

So you don't provide the metric value, but the instance count.

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.

@auvipy
Copy link

auvipy commented Aug 6, 2020

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 ):
https://flower.readthedocs.io/en/latest/api.html

So you don't provide the metric value, but the instance count.

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.

@tomkerkhove
Copy link
Member

I can see value in a "we provide the metric, you'll handle the rest" API endpoint.

@mik-laj
Copy link

mik-laj commented Aug 9, 2020

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.
Here is docs about it: https://www.zabbix.com/documentation/5.2/manual/config/items/itemtypes/http

If someone wants to use the token for authorization, they must add Authorization header.

It's worth taking a look at the "Postprocessing" section, which has no documentation, but I attach a screenshot of the interface.
Screenshot 2020-08-07 at 03 25 08
Screenshot 2020-08-07 at 03 25 14
Postprocessing using Custom Javascript seems to be the most powerful. We can use goja to implement it.

@tomkerkhove
Copy link
Member

What concerns do you have with #929 (comment) @mik-laj ?

@turbaszek
Copy link
Contributor Author

I can see value in a "we provide the metric, you'll handle the rest" API endpoint.

@tomkerkhove I will draft a PR soon

@tomkerkhove
Copy link
Member

I presume you agree on the proposed API spec then or?

@turbaszek
Copy link
Contributor Author

Yup, that makes sense to me 👍

@auvipy
Copy link

auvipy commented Aug 17, 2020

Just want to know, how keda could be useful with upcoming airship 2.0 ? https://www.airshipit.org/blog/airship2-is-alpha/

@turbaszek
Copy link
Contributor Author

@tomkerkhove @ahmelsayed I just realized that the openapi spec excludes the possibility of using jsonpath/statsd as mentioned #929 (comment)

turbaszek added a commit to turbaszek/keda that referenced this issue Aug 22, 2020
Closes: kedacore#929

Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
turbaszek added a commit to turbaszek/keda that referenced this issue Aug 22, 2020
Closes: kedacore#929

Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
turbaszek added a commit to turbaszek/keda that referenced this issue Aug 22, 2020
Closes: kedacore#929

Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
@tomkerkhove
Copy link
Member

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 scaling/v1/metric/{name} if you want! But it has to provide the correct payload and status codes. @turbaszek is already working on this. (thanks!)

@lonnix
Copy link

lonnix commented Aug 27, 2020

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

  1. /api/v1/health
  2. /api/v1/metrics
  3. /api/v1/metric/{name}

I have seen companies use /health, /healthcheck and /heartbeat for health checks of their app, so asking someone to provide another endpoint of /api/v1/health when they already have /heartbeat for the same thing raises concern. Also, where is the health endpoint getting used? Is the scaler regularly checking the health endpoint before making calls to the metric endpoint? If so this is probably the same result as calling the metric endpoint and not getting a 2XX back

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:

metadata:
        targetValue: "1"
        apiURL: "http://api:1234/v1/scale"
        metricName: "key3"

vs

metadata:
        targetValue: "1"
        apiURL: "http://api:1234/v1/scale/key3"

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.

@turbaszek
Copy link
Contributor Author

turbaszek commented Aug 27, 2020

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 url and retrieve the nested value.

Advantages of this approach are:

  1. complies with any Open API spec
  2. allows users to use any API meaning that anyone can use KEDA without any additional development

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.

@lonnix
Copy link

lonnix commented Aug 27, 2020

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.

@tomkerkhove
Copy link
Member

Yes, that's something we've agreed upon during the implementation.

The spec here was just a proposal but we will:

  1. Remove health endpoint
  2. Remove get metrics endpoint
  3. Only have the OpenAPI spec to define what the API endpoint should return and what status codes we expect, but the URL path itself will be documented as a suggestion and not a must given it's going to be part of the ScaledObject indeed.

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.

@turbaszek
Copy link
Contributor Author

But we have to provide something so that people know what the API has to return

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.

@tomkerkhove
Copy link
Member

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.

@turbaszek
Copy link
Contributor Author

These decrease adoption in my opinion

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 url=http://app/api/v1/metric/myMetric and valueLocation=$.value. If other have more complicated responses then using jsonPath will help them.

With good documentation, everyone should be happy :)

@lonnix
Copy link

lonnix commented Aug 28, 2020

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.

@turbaszek
Copy link
Contributor Author

@ahmelsayed @zroubalik what is your opinion? Should we stick to open api or allow more flexibility as discussed above?

@tomkerkhove
Copy link
Member

Use json path then. We can still make it optional if we want to go the OpenAPI route.

@turbaszek
Copy link
Contributor Author

@tomkerkhove thanks! I will do this in the next few days

@turbaszek
Copy link
Contributor Author

turbaszek commented Sep 2, 2020

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.

turbaszek added a commit to turbaszek/keda that referenced this issue Sep 3, 2020
Closes: kedacore#929
Signed-off-by: Tomek Urbaszek <tomasz.urbaszek@polidea.com>
@tomkerkhove
Copy link
Member

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?

@tomkerkhove
Copy link
Member

Re-opening as docs are not done yet

@tomkerkhove
Copy link
Member

Here we go, added some basic auth scenarios. THanks for the JWT one @turbaszek!

@tomkerkhove tomkerkhove added feature All issues for new features that have been committed to scaler-metrics-api All issues related to our Metrics API scaler and removed needs-discussion labels Sep 4, 2020
@turbaszek
Copy link
Contributor Author

Closing as docs are merged and issues for auth are created 🚀

SpiritZhou pushed a commit to SpiritZhou/keda that referenced this issue Jul 18, 2023
* 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>
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 scaler-metrics-api All issues related to our Metrics API scaler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants