perf(router): adjust the sequence of fields generated inside a traditional compatible route expression

Author: windmgc

changelog/unreleased/kong/perf-router-trad-compat-expression.yml

@@ -0,0 +1,3 @@
+message: "Improved performance of the traditional compatible router by optimizing the expression generation."
+type: performance
+scope: Core

kong/router/transform.lua

@@ -331,6 +331,15 @@ local function path_val_transform(op, p)
 end
 
 
+-- Generates an ATC router expression for traditional compat router flavor
+-- match priority for http is as follows:
+-- (sni if net.protocol == https) -> net.src.ip -> net.src.port
+--   -> net.dst.ip -> net.dst.port -> http.path -> http.headers
+--   -> http.host -> http.method
+--
+-- match priority for stream is as follows:
+-- (sni if net.protocol == tls) -> net.src.ip -> net.src.port
+--   -> net.dst.ip -> net.dst.port
 local function get_expression(route)
   -- we prefer the field 'expression', reject others
   if not is_null(route.expression) then
@@ -377,11 +386,39 @@ local function get_expression(route)
 
   -- http expression, protocol = http/https/grpc/grpcs
 
-  gen = gen_for_field("http.method", OP_EQUAL, route.methods)
+  gen = gen_for_field("http.path", path_op_transform, route.paths, path_val_transform)
   if gen then
     expression_append(expr_buf, LOGICAL_AND, gen)
   end
 
+  local headers = route.headers
+  if not is_empty_field(headers) then
+    headers_buf:reset()
+
+    for h, v in pairs(headers) do
+      single_header_buf:reset():put("(")
+
+      for i, value in ipairs(v) do
+        local name = "any(lower(http.headers." .. replace_dashes_lower(h) .. "))"
+        local op = OP_EQUAL
+
+        -- value starts with "~*"
+        if byte(value, 1) == TILDE and byte(value, 2) == ASTERISK then
+          value = value:sub(3)
+          op = OP_REGEX
+        end
+
+        expression_append(single_header_buf, LOGICAL_OR,
+                          name .. " " .. op .. " " .. escape_str(value:lower()), i)
+      end
+
+      expression_append(headers_buf, LOGICAL_AND,
+                        single_header_buf:put(")"):get())
+    end
+
+    expression_append(expr_buf, LOGICAL_AND, headers_buf:get())
+  end
+
   local hosts = route.hosts
   if not is_empty_field(hosts) then
     hosts_buf:reset():put("(")
@@ -412,39 +449,11 @@ local function get_expression(route)
     expression_append(expr_buf, LOGICAL_AND, hosts_buf:put(")"):get())
   end
 
-  gen = gen_for_field("http.path", path_op_transform, route.paths, path_val_transform)
+  gen = gen_for_field("http.method", OP_EQUAL, route.methods)
   if gen then
     expression_append(expr_buf, LOGICAL_AND, gen)
   end
 
-  local headers = route.headers
-  if not is_empty_field(headers) then
-    headers_buf:reset()
-
-    for h, v in pairs(headers) do
-      single_header_buf:reset():put("(")
-
-      for i, value in ipairs(v) do
-        local name = "any(lower(http.headers." .. replace_dashes_lower(h) .. "))"
-        local op = OP_EQUAL
-
-        -- value starts with "~*"
-        if byte(value, 1) == TILDE and byte(value, 2) == ASTERISK then
-          value = value:sub(3)
-          op = OP_REGEX
-        end
-
-        expression_append(single_header_buf, LOGICAL_OR,
-                          name .. " " .. op .. " " .. escape_str(value:lower()), i)
-      end
-
-      expression_append(headers_buf, LOGICAL_AND,
-                        single_header_buf:put(")"):get())
-    end
-
-    expression_append(expr_buf, LOGICAL_AND, headers_buf:get())
-  end
-
   local str = expr_buf:get()
 
   -- returns a local variable instead of using a tail call

spec/01-unit/08-router_spec.lua

@@ -2257,14 +2257,14 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
             it("empty hosts", function()
               use_case[1].route.hosts = v
 
-              assert.equal(get_expression(use_case[1].route), [[(http.method == r#"GET"#) && (http.path ^= r#"/foo"#)]])
+              assert.equal(get_expression(use_case[1].route), [[(http.path ^= r#"/foo"#) && (http.method == r#"GET"#)]])
               assert(new_router(use_case))
             end)
 
             it("empty headers", function()
               use_case[1].route.headers = v
 
-              assert.equal(get_expression(use_case[1].route), [[(http.method == r#"GET"#) && (http.path ^= r#"/foo"#)]])
+              assert.equal(get_expression(use_case[1].route), [[(http.path ^= r#"/foo"#) && (http.method == r#"GET"#)]])
               assert(new_router(use_case))
             end)
 
@@ -2278,7 +2278,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
             it("empty snis", function()
               use_case[1].route.snis = v
 
-              assert.equal(get_expression(use_case[1].route), [[(http.method == r#"GET"#) && (http.path ^= r#"/foo"#)]])
+              assert.equal(get_expression(use_case[1].route), [[(http.path ^= r#"/foo"#) && (http.method == r#"GET"#)]])
               assert(new_router(use_case))
             end)
           end
@@ -2302,15 +2302,15 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
           it("path has '\"'", function()
             use_case[1].route.paths = { [[~/\"/*$]], }
 
-            assert.equal([[(http.method == r#"GET"#) && (http.path ~ r#"^/\"/*$"#)]],
+            assert.equal([[(http.path ~ r#"^/\"/*$"#) && (http.method == r#"GET"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)
 
           it("path has '\"#'", function()
             use_case[1].route.paths = { [[~/\"#/*$]], }
 
-            assert.equal([[(http.method == r#"GET"#) && (http.path ~ "^/\\\"#/*$")]],
+            assert.equal([[(http.path ~ "^/\\\"#/*$") && (http.method == r#"GET"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)
@@ -2334,15 +2334,15 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
           it("regex path has double '\\'", function()
             use_case[1].route.paths = { [[~/\\/*$]], }
 
-            assert.equal([[(http.method == r#"GET"#) && (http.path ~ r#"^/\\/*$"#)]],
+            assert.equal([[(http.path ~ r#"^/\\/*$"#) && (http.method == r#"GET"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)
 
           it("regex path has '\\d'", function()
             use_case[1].route.paths = { [[~/\d+]], }
 
-            assert.equal([[(http.method == r#"GET"#) && (http.path ~ r#"^/\d+"#)]],
+            assert.equal([[(http.path ~ r#"^/\d+"#) && (http.method == r#"GET"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)
@@ -2366,7 +2366,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
             use_case[1].route.methods = { "GET" }
             use_case[1].route.headers = { test = { "~*Quote" }, }
 
-            assert.equal([[(http.method == r#"GET"#) && (any(lower(http.headers.test)) ~ r#"quote"#)]],
+            assert.equal([[(any(lower(http.headers.test)) ~ r#"quote"#) && (http.method == r#"GET"#)]],
                          get_expression(use_case[1].route))
             assert(new_router(use_case))
           end)