Circuit breaker at Route level in Kong is not working as expected

[natesh1221]

Summary

I have an API endpoint as follows: https://test-cb.example.com/kong-circuit-breaker/api/v2/<-user-id-as-a-path-variable->
and I have an ingress for this endpoint as mentioned below:

kind: Ingress
apiVersion: extensions/v1beta1
metadata:
  name: test-ingress
  namespace: stage
  annotations:
    konghq.com/plugins: circuit-breaker-stage
spec:
  ingressClassName: kong-internal
  tls:
    - hosts:
        - test-cb.example.com
  rules:
    - host: test-cb.example.com
      http:
        paths:
          - path: /kong-circuit-breaker/api/v2/
            pathType: ImplementationSpecific
            backend:
              serviceName: stage-user-service
              servicePort: 80

and circuit-breaker config as below:

apiVersion: configuration.konghq.com/v1
kind: KongPlugin
metadata:
  namespace: stage
  name: circuit-breaker-stage
config:
  window_time: 30
  api_call_timeout_ms: 2000
  min_calls_in_window: 10
  failure_percent_threshold: 51
  wait_duration_in_open_state: 60
  wait_duration_in_half_open_state: 90
  half_open_min_calls_in_window: 5
  half_open_max_calls_in_window: 10
  error_status_code: 599
  excluded_apis: '{"GET_/kong-circuit-breaker/actuator/health": true}'
plugin: circuit-breaker

I'm have added kong-circuit-breaker for the above route and tried testing it and found out that each request URL for this route is having independent circuit-breaker state maintained where my expectation was that single circuit breaker state will be maintained for the route/routes configured in ingress instead for each request URL.

Is there a way to specify in kong-circuit-breaker to maintain the state of circuit-breaker for aggregated count of all the requests made to the API with the path specified in Ingress.

Example: Irrespective of user-Id in the following path: kong-circuit-breaker/api/v2/<-user-id-as-a-path-variable->, the circuit-breaker should open, if the failure percentage reaches the threshold for this API.
Whereas the current behaviour is that the circuit-breaker for the API with path: kong-circuit-breaker/api/v2/adam123 is open, while the same API with path: kong-circuit-breaker/api/v2/maria456 is allowing the requests to the service.

PS: I'm not sure if this issue has to be submitted as a bug or feature request.

Thanks.

[chirag-manwani]

You are completely correct in your observation. But I would say this is sort of the expected behavior.
The plugin is expected to create a circuit breaker for each URI and since path params are part of the URI, the plugin creates a separate circuit breaker for it.

One way that I can think of to solve this problem is to create circuit breakers on (one of) the paths that are configured for the said route.

For example, if there is a route R which is listening on two paths- /r1 and /r2
Then create a circuit breaker on

1. /r1 if request is on /r1 and /r2 if request is on /r2
2. /r1 irrespective of which path the request matched with

I am in favor of the second approach.

I would have to confirm if this behavior can be applied when the circuit breaker is applied on a global level, or service level since I am not sure if the paths configured for route are available when plugin is applied on higher levels (the route entity should be available, I'll just have to confirm).

Please follow up with your comments/suggestions/concerns if any.

[natesh1221]

Thanks for your quick reply and the suggestions.

If I understood the suggestions correctly, you are asking us to create the circuit-breaker for each path (including the exact userId in the path) separately, which in our case might not work, because each path depends on the userId and we have lakhs of users which leaves us with lakhs of unique paths.

Please correct me If I misinterpreted the suggested solutions.

Thanks.

[natesh1221]

@chirag-manwani Can you please check my reply above.

[chirag-manwani]

You are completely correct in your observation. But I would say this is sort of the expected behavior.
The plugin is expected to create a circuit breaker for each URI and since path params are part of the URI, the plugin creates a separate circuit breaker for it.

One way that I can think of to solve this problem is to create circuit breakers on (one of) the paths that are configured for the said route.

The example that I gave above was wrong. I'll explain what I mean by the above line.

If you have a Route that has this path configured - /abc and you get a request on this path /abc then everything works as expected, and the circuit breaker (if there is any error) gets created using this identifier 'post_/abc'

Now consider you get a request on this path- /abc/user1
This request will also be matched with the same route ( longest prefix rule) but in this case, the circuit breaker object will be created for /abc/user1 and not /abc. This is sort of the expected behavior.

Let's say I change the plugin to create a circuit breaker based on the Route path and not the actual URI of the request. In that case I have another issue if the Route has multiple paths configured. AFAIK you have the following information when writing plugins- URI of the request (all metadata actually but we are only concerned with the URI), and the Service and Route object that the request matched with. For the matched route, there is no information on which path actually matched with the URI, you only know that it is one of the paths.

I am all in for fixing this behavior, but I need to consider all possible solutions first. An ideal solution should consider all possible scenarios such as a route having multiple paths, query params should be excluded or not, path params to be excluded or not, the actual path that matched, or some metadata that can be used to create a unique identifier for the circuit breaker.

I hope I was clear this time. Please suggest solutions if you have any. I will work on solutions and post them here.

One thing that I can think of immediately is that we can use this convention for creating circuit breakers-
method_/route-name

This ensures the uniqueness of the circuit breaker without losing any information. However, some people might be interested in keeping the uri information (or at least the matched route path) somewhere in the name.

[chirag-manwani]

@natesh1221 Your thoughts on this?

I think it would be useful to create internal circuit breakers using the method_/route-name pattern. This would ensure only 1 circuit breaker per route. What do you think?

[natesh1221]

@chirag-manwani This should work and I was also thinking in the same line.

Should we support the existing behaviour (circuit-breaker instance per API_identifier) as the default behaviour and give an option (by using some configuration parameter) for the user to override it (create 1 circuit-breaker instance per route).

And one circuit-breaker per route can also become one circuit-breaker per service if they have the plugin added at the service level.

Please correct me If I'm wrong anywhere.

[chirag-manwani]

@natesh1221 Yes, I think we should make the plugin backward compatible and introduce a couple of configuration parameters that let the user control the scope of the circuit breakers created, but I believe this scope should be limited to just the route level and not the service level. However, I am open to suggestions if you believe that circuit breakers on a service level or even global level could solve some use case.

Your thoughts?

[natesh1221]

Yes, After giving it a thought,I think I am completely in agreement with you on the scope being limited to just the route level as there is no such use-case where we need a single cb object for a whole service, as there can be 100s of APIs in the single service. And the same is applicable for global level also.

[chirag-manwani]

@natesh1221 That's great. Would you be willing to contribute and create a PR that adds this functionality to the plugin?

[natesh1221]

Yes @chirag-manwani , I will be happy to do it.

[natesh1221]

@chirag-manwani When I discussed the approach with my teammates, they highlighted one problem with the current approach which is: If an application has 100+ APIs and I want the circuit-breaker for each API (API route level), I will end up creating 100+ ingress files as we are using the ingress-name (route-name) as the CB identifier.

Any thoughts on this?

[chirag-manwani]

@natesh1221 No, I think the approach will remain the same. In Kong when a request is matched with any route there's a way to get the matched route and service entity (which includes the name and other properties of the route and service).
So even if you use a ClusterPlugin, it will still use the name of the route from the route entity after the request is matched with any route.

Correct me if I am wrong.

[natesh1221]

@chirag-manwani I will try to explain you the scenario with example:
Let's say we have user-details service, which has the following 4 APIs:

• GET_user-details/get-user
• GET_user-details/get-user-address
• POST_user-details/add-user
• DELETE_user-details/delete-user

And I have an existing ingress object for the user-details service in kubernetes as below:

kind: Ingress
apiVersion: extensions/v1beta1
metadata:
  name: user-details
  namespace: stage
spec:
  ingressClassName: kong-internal
  tls:
    - hosts:
        - test.user.com
  rules:
    - host: test.user.com
      http:
        paths:
          - path: /user-details
            pathType: ImplementationSpecific
            backend:
              serviceName: stage-user-service
              servicePort: 80

Considering the behaviour of the circuit-breaker is as we discussed above (one CB object per method_/route-name pattern),

If I add circuit-breaker plugin on the user-details ingress/route object by using the annotation below:

annotations:
    konghq.com/plugins: circuit-breaker-stage

I will have CB objects as below:

{ 
  user-details: {
    get_user-details = cb_object_1,
    post_user-details = cb_object_2,
    delete_user-details = cb_object_3
  }
}

So, If I need to maintain two different CB objects for these GET APIs:

• GET_user-details/get-user
• GET_user-details/get-user-address

I will need to create a different ingress for path: user-details/get-user-address as in the above case, there is only one common cb_object maintained for these two APIs since the route_name is same, thus the cb_object_name.

And If I have all GET APIs in any application. I will need those many ingress files to maintain each cb_objects for each API.

Hope I'm clear this time. Please correct me If I'm wrong anywhere.

Thanks.

[chirag-manwani]

You'll need n Route entities each having a circuit breaker instance for itself. For k8s if that means creating n ingress files, then yes you would need to do that.
Moreover you'll have an option to switch between the old behaviour (one cb per route path) and the new behaviour (one cb per route name) so your use case can still be satisfied using the old behaviour.

[natesh1221]

Yeah you are right @chirag-manwani.
After discussion with my team, I feel creating n Route entities would not be feasible solution.

The plugin's current behaviour will work like a charm for the APIs with only query-param in the request URL,
We have problem only with the URLs with the path-variable for which, I was thinking, if it's possible to group the API_paths and have the CB object for each API_path group.