feat(balancer) add path, uri_capture, and query as hash sources

[flrgh]

This adds path, uri_capture, and query_arg as hash sources for upstreams.

There are some code simplicity vs performance vs absolute correctness tradeoffs to consider here:

• path sources its value from $uri, which is normalized according to NGINX rules and not kong.tools.uri.normalize($request_uri). The balancer hash is not directly exposed via any public API and is much less sensitive to compatibility issues than other areas of the codebase (i.e. kong.router), so I think it's okay to not be 100% consistent here.
• query_arg uses ngx.req.get_uri_args, which is semi-expensive as it creates a new table (similar to hash_on_header now). A feature of an upcoming OpenResty release allows optimizing away the table creation (link), which I've accounted for. The benefit of doing this over using ngx.var.arg_{{name}} is that NGINX handles decoding for us, and that it works for multi-value query string args.

[Tieske]

Why would one use a path as the hash input? that doesn't make sense to me. Wouldn't it be better to enable using a path-parameter?

Say I have 20 paths defined, all starting like this; /myapp/[userid]/.... Now the cardinality of the hash is high because there are 20 individual paths, each getting their own hash.

If we'd use just the captured [userid] element, we'd ensure that all traffic related to a single user would end up on a single backend. This would make more sense imo.

That said, I don't know the origin of this request.

[flrgh]

Why would one use a path as the hash input? that doesn't make sense to me. Wouldn't it be better to enable using a path-parameter?

Say I have 20 paths defined, all starting like this; /myapp/[userid]/.... Now the cardinality of the hash is high because there are 20 individual paths, each getting their own hash.

If we'd use just the captured [userid] element, we'd ensure that all traffic related to a single user would end up on a single backend. This would make more sense imo.

Agree 100%

That said, I don't know the origin of this request.

Ah, you're one step ahead of me in my explanation 😉. This request comes from an enterprise customer who has explicitly named path as one of their use-cases.

[flrgh]

@Tieske I've added router URI capture support :)

The work for supporting path is already done and it has minimal impact (no new entity fields/migrations like query/URI capture), so I've left it in for now. It's probably not as useful as URI capture but IMO still has valid use cases.

[Tieske]

The work for supporting path is already done and it has minimal impact (no new entity fields/migrations like query/URI capture), so I've left it in for now. It's probably not as useful as URI capture but IMO still has valid use cases.

What usecase would they be useful for?

[flrgh]

The work for supporting path is already done and it has minimal impact (no new entity fields/migrations like query/URI capture), so I've left it in for now. It's probably not as useful as URI capture but IMO still has valid use cases.

What usecase would they be useful for?

It's the same use-case as uri_capture in my opinion: send repeat, homogeneous requests to the same upstream target in order to account for transient state and/or maximize cache hit ratio. If I'm a user with this goal, but I don't need the level of control that uri_capture would provide, I can just use path.

[Tieske]

still not convinced about the path option, I'm afraid it's bloat. Then again it's minimal and isolated in a specific corner of the code. So not that big a deal.

[flrgh]

rebased onto latest master + fixed merge conflicts

[flrgh]

Kong moderators: please hold off on merge for one more rebase+test cycle. I didn't think we'd be upgrading to OpenResty 1.21.4 in this release cycle, so I'm going to remove the version gate on this perf optimization and rebase+re-push. One moment...

EDIT: Code change is done--good to go after CI finishes.

[flrgh]

@aboudreault anything else I should address before merge, or is this good to go?

[aboudreault]

what's left to do will be in EE so you can merge