[backport 3.4]feat(router/atc): http segments matching and other improvements #12400

chronolaw · 2024-01-23T11:55:13Z

Summary

In 3.4 we have no request_aware_table, so I replace it with tb_new/tb_clear.

git cherry-pick 2457ef0 #11388
git cherry-pick 36647f2 #11430
git cherry-pick 8cd18bc #11411
git cherry-pick ba559ea #11449

git cherry-pick 59670a1 #11721
git cherry-pick 5f5e272 #11904
git cherry-pick 6ce1262 #11905
git cherry-pick 37bd9c2 #11914
git cherry-pick 12f45ad #11948
git cherry-pick c9fd6c1 #12127
git cherry-pick 8c2b5a4 #12258
git cherry-pick 86f0a6c #12261, fixing *.test
git cherry-pick f49abd6 #12180, fixing *.test https://github.com/Kong/kong/pull/12268/files#diff-50dcfdce82665f85d2027363af6956fc788e5b8f6c6b8de907e9bd82e8871501
git cherry-pick 11d7639 #12266
git cherry-pick 560fdfb #11950
git cherry-pick ce12f68 #12307, fixing *.test
git cherry-pick 9305665 #12283
git cherry-pick ef87932 #12378

Checklist

The Pull Request has tests
A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #[issue number]

nowNick

Looks good to me!

I have some questions that I was curious about:

Could you expand a little bit on the request-aware-table smaller fix? How does it impact / change the behaviour when you use tb_new/tb_clear compared to with / without request-aware-table? As far as I remember request-aware-table was only a test utility to detect if a corruption is happening. On normal flow it is not triggered - am I correct? 🤔
Does it make sense to cherry pick: 57df3bc if later on you revert those changes with: 4aad7d2 ?
Note: Super-tiny / nitpick:
In the PR description you point to: git cherry-pick f49abd69c70eb719b53b84db21a1756743c089a6 https://github.com/Kong/kong/pull/12180, fixing *.test which might sound like we want to backport test scheduler. I wonder if it would be more helpful to point to this: https://github.com/Kong/kong/pull/12268/files#diff-50dcfdce82665f85d2027363af6956fc788e5b8f6c6b8de907e9bd82e8871501
But it's just a PR description fix if anybody would like to dig it up in future.

chronolaw · 2024-02-28T01:02:56Z

Looks good to me!

I have some questions that I was curious about:

Could you expand a little bit on the request-aware-table smaller fix? How does it impact / change the behaviour when you use tb_new/tb_clear compared to with / without request-aware-table? As far as I remember request-aware-table was only a test utility to detect if a corruption is happening. On normal flow it is not triggered - am I correct? 🤔

Does it make sense to cherry pick: 57df3bc if later on you revert those changes with: 4aad7d2 ?

Note: Super-tiny / nitpick:
In the PR description you point to: git cherry-pick f49abd69c70eb719b53b84db21a1756743c089a6 https://github.com/Kong/kong/pull/12180, fixing *.test which might sound like we want to backport test scheduler. I wonder if it would be more helpful to point to this: https://github.com/Kong/kong/pull/12268/files#diff-50dcfdce82665f85d2027363af6956fc788e5b8f6c6b8de907e9bd82e8871501
But it's just a PR description fix if anybody would like to dig it up in future.

Yes, request-aware-table is only used for debugging, and introduce it into 3.4 will increase more complexity, so I replace it with tb_new/tb_clear.
I just follows the original PR order, sure, the final result is same.
I only picked one commit in feat(ci): dynamic test scheduler / balancer #12180, which revert some test code. Thank you for pointing it, I will add it in description.

chobits · 2024-02-28T09:35:18Z

focusing on these commits which are different between the master as chrono said in description

And some PRs, such as #12390 , have been also intergrated into release/3.4.x, but you do not mention them in your description.

chronolaw · 2024-02-28T09:58:58Z

The reviewers can refer to ticket KAG-3586, which recorded all the picked PRs, but I may miss some PRs.

chobits · 2024-03-01T03:17:36Z

TODO: I'll assist with running perf test on release/3.4.x after this PR is merged.

Summary Categorize atc fields to fields header_fields query_fields, then we can simplify the logic in select().

style(router): minor code style clean

…11721) resty.core.utils.str_replace_char() is a better way to replace - to _. In the future string.lua will gather more functions to simplify tools.utils.lua. See: #10443

#11904) This helps with generating easier to read expressions, and the code is more straightforward. However, we must fallback to the old style escaping if the value contains `"#` (very unlikely case). KAG-2952

…pressions` flavor (#11905) `traditional_compatible` flavor remains case insensitive to stay compatible with `traditional` flavor. This change allow `expressions` route authors to pick whether they want case sensitive or insensitive matches. KAG-2905 --------- Co-authored-by: Datong Sun <[email protected]>

…ssions router schema (#11914) KAG-2961 --------- Co-authored-by: Datong Sun <[email protected]>

…outer (#12127) Cache key and context generation are closely related on field present inside configured expressions. It is advantageous to unify the logic for generating them to: 1. Improve cache hit rate, so that only fields referenced inside expressions participates in cache key generation. This is particularly important since we plan on adding more match fields into expressions in the future 2. Improve performance, allows field value to be cached and reused between cache key and context generation 3. Reduced code duplication KAG-3032

… new context (#12258) To avoid frequent memory allocation/deallocations. KAG-3448

…12261) KAG-3032

Fix `03-http-log/01-log_spec.lua` Fix `13-cors/01-access_spec.lua` Fix `spec/03-plugins/03-http-log/01-log_spec.lua`

…expression routes (#11950) These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes. KAG-2963 KAG-3032

…ase of cache hits (#12307) `upstream_uri` is dependent on request path, and should not be automatically cached inside the route cache due to potential for path matching not being configured inside routes. KAG-3505

…12283) KAG-3351

KAG-3583

@ADD-SP

Full changelog: https://github.com/Kong/atc-router/releases/tag/v1.5.1 This primarily contains the small Bazel change which reduced artifact size by 50%. Thanks @ADD-SP for the work.

This field represents how much segments the request path contains. For example, "/a/b/c/" contains 3 segments, "/a" contains 1 and "/" contains 0 segment. It is useful for implementing segment based routing logic such as these used in OpenAPI spec. KAG-3604

… field (#12406) KAG-3604 --------- Co-authored-by: Datong Sun <[email protected]>

…g segments from it (#12407) KAG-3351

…ot" operator support in ATC (#12405) KAG-3605 --------- Co-authored-by: Datong Sun <[email protected]>

… folder (#12419) KAG-3605

* perf(router): use static functions for callbacks Signed-off-by: Aapo Talvensaari <[email protected]> * tuning some code * style clean * style clean * style clean --------- Signed-off-by: Aapo Talvensaari <[email protected]> Co-authored-by: chronolaw <[email protected]>

chobits · 2024-03-05T02:58:58Z

running perf test for this PR's image kong/kong:90a783b8388db4e1cc290c34c9d5617a6cacc841-ubuntu (here) compared to 3.4.3.4

pull-request-size bot added the size/XXL label Jan 23, 2024

github-actions bot added core/pdk core/db schema-change-noteworthy core/wasm Everything relevant to [proxy-]wasm labels Jan 23, 2024

github-actions bot assigned chronolaw Jan 23, 2024

chronolaw added the backport label Jan 23, 2024

chronolaw force-pushed the cherry/http_segments_matching_34 branch 2 times, most recently from ecf540e to f848e65 Compare January 24, 2024 01:35

chronolaw changed the title ~~[backport]feat(router/atc): http segments matching and other improvements~~ [backport 3.4]feat(router/atc): http segments matching and other improvements Jan 24, 2024

chronolaw marked this pull request as ready for review January 27, 2024 01:19

chronolaw force-pushed the cherry/http_segments_matching_34 branch from e0a4e6a to f87f0e0 Compare February 26, 2024 02:26

kikito requested review from StarlightIbuki and nowNick February 27, 2024 09:05

nowNick approved these changes Feb 27, 2024

View reviewed changes

nowNick mentioned this pull request Feb 27, 2024

[backport 3.5]feat(router/atc): http segments matching and other improvements #12397

Merged

3 tasks

chobits approved these changes Feb 28, 2024

View reviewed changes

chronolaw and others added 9 commits March 4, 2024 10:37

style(router): move some http only functions

7271a93

style(router): simplify logic of getting http params (#11430)

1a6259a

refactor(router): categorize router fields to simplify logic (#11411)

ff4e6ac

Summary Categorize atc fields to fields header_fields query_fields, then we can simplify the logic in select().

style(router): minor code style clean (#11449)

38e9995

style(router): minor code style clean

perf(router): use resty.core.utils.str_replace_char() for dashes (#…

063c87d

…11721) resty.core.utils.str_replace_char() is a better way to replace - to _. In the future string.lua will gather more functions to simplify tools.utils.lua. See: #10443

refactor(router): use ATC raw string literal in expressions generation (

bd563c1

#11904) This helps with generating easier to read expressions, and the code is more straightforward. However, we must fallback to the old style escaping if the value contains `"#` (very unlikely case). KAG-2952

fix(router): http and stream subsystems no longer share the expre…

dc727d4

…ssions router schema (#11914) KAG-2961 --------- Co-authored-by: Datong Sun <[email protected]>

refactor(router): simplify the functions of cache calculation (#11948)

807228e

chronolaw and others added 17 commits March 4, 2024 10:37

perf(router): reuse ATC context in router match instead of creating a…

d04991f

… new context (#12258) To avoid frequent memory allocation/deallocations. KAG-3448

fix(router): add missing preserve_host logic in stream subsystem (#…

bed2a9e

…12261) KAG-3032

tests(plugins): fix previous master test failures

99318ec

Fix `03-http-log/01-log_spec.lua` Fix `13-cors/01-access_spec.lua` Fix `spec/03-plugins/03-http-log/01-log_spec.lua`

docs(changelog): fix a typo in #11258 (#12266)

846bca7

feat(router/atc): support net.src.* and net.dst.* fields in HTTP …

ae0b3a6

…expression routes (#11950) These fields are available in stream (L4) routes, making them also available in HTTP to reduce discrepancy between HTTP and stream routes. KAG-2963 KAG-3032

feat(router): support segment based matching in expressions flavor (#…

040d690

…12283) KAG-3351

perf(router/atc): lazy generate and cache field visit functions (#12378)

8d3cf25

KAG-3583

refactor(router): replace request aware table with tb_new/tb_clear

a4ec01d

chore(requirements): bump atc-router to v1.5.1 (#12390)

d462ece

Full changelog: https://github.com/Kong/atc-router/releases/tag/v1.5.1 This primarily contains the small Bazel change which reduced artifact size by 50%. Thanks @ADD-SP for the work.

docs(changelog): add changelog entry for the http.path.segments.len…

694b245

… field (#12406) KAG-3604 --------- Co-authored-by: Datong Sun <[email protected]>

tests(router/atc): ensure request path is normalized before extractin…

2488031

…g segments from it (#12407) KAG-3351

chore(deps): bump atc-router to v1.6.0 and add changelog entry for "n…

61a5e96

…ot" operator support in ATC (#12405) KAG-3605 --------- Co-authored-by: Datong Sun <[email protected]>

docs(changelog): move "expressions_not_operator" entry to the correct…

1c15109

… folder (#12419) KAG-3605

chronolaw force-pushed the cherry/http_segments_matching_34 branch from f87f0e0 to 1543f9f Compare March 4, 2024 02:37

ADD-SP approved these changes Mar 7, 2024

View reviewed changes

ADD-SP merged commit 74f7933 into release/3.4.x Mar 7, 2024
24 checks passed

ADD-SP deleted the cherry/http_segments_matching_34 branch March 7, 2024 08:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[backport 3.4]feat(router/atc): http segments matching and other improvements #12400

[backport 3.4]feat(router/atc): http segments matching and other improvements #12400

chronolaw commented Jan 23, 2024 •

edited

Loading

nowNick left a comment

chronolaw commented Feb 28, 2024 •

edited

Loading

chobits commented Feb 28, 2024 •

edited

Loading

chronolaw commented Feb 28, 2024 •

edited

Loading

chobits commented Mar 1, 2024 •

edited

Loading

chobits commented Mar 5, 2024

[backport 3.4]feat(router/atc): http segments matching and other improvements #12400

[backport 3.4]feat(router/atc): http segments matching and other improvements #12400

Conversation

chronolaw commented Jan 23, 2024 • edited Loading

Summary

Checklist

Issue reference

nowNick left a comment

Choose a reason for hiding this comment

chronolaw commented Feb 28, 2024 • edited Loading

chobits commented Feb 28, 2024 • edited Loading

chronolaw commented Feb 28, 2024 • edited Loading

chobits commented Mar 1, 2024 • edited Loading

chobits commented Mar 5, 2024

chronolaw commented Jan 23, 2024 •

edited

Loading

chronolaw commented Feb 28, 2024 •

edited

Loading

chobits commented Feb 28, 2024 •

edited

Loading

chronolaw commented Feb 28, 2024 •

edited

Loading

chobits commented Mar 1, 2024 •

edited

Loading