Shared utilities to handle VRL expressions & some other improvements #540

ardatan · 2025-10-31T10:17:10Z

Removed pool_idle_timeout_seconds from traffic_shaping, instead use pool_idle_timeout with duration format.

traffic_shaping:
-  pool_idle_timeout_seconds: 30
+  pool_idle_timeout: 30s

Related documentation changes PR -> graphql-hive/console#7207

Created some helper functions to deduplicate the code to compile and resolve VRL expressions

gemini-code-assist

Code Review

This pull request introduces a new Expression type to centralize VRL expression handling, which is a great improvement for maintainability. The new type encapsulates VRL compilation and execution, removing duplicated logic from various parts of the executor. My review focuses on performance improvements for this new type, in line with the repository's style guide.

lib/router-config/src/primitives/expression.rs

github-actions · 2025-10-31T10:22:57Z

✅ `k6-benchmark` results

     ✓ response code was 200
     ✓ no graphql errors
     ✓ valid response structure

     █ setup

     checks.........................: 100.00% ✓ 207978      ✗ 0    
     data_received..................: 6.1 GB  202 MB/s
     data_sent......................: 81 MB   2.7 MB/s
     http_req_blocked...............: avg=3.88µs   min=661ns   med=1.86µs  max=4.02ms   p(90)=2.7µs   p(95)=3.14µs  
     http_req_connecting............: avg=1.11µs   min=0s      med=0s      max=3.02ms   p(90)=0s      p(95)=0s      
     http_req_duration..............: avg=21.16ms  min=2.38ms  med=20.28ms max=134.87ms p(90)=28.65ms p(95)=31.94ms 
       { expected_response:true }...: avg=21.16ms  min=2.38ms  med=20.28ms max=134.87ms p(90)=28.65ms p(95)=31.94ms 
     http_req_failed................: 0.00%   ✓ 0           ✗ 69346
     http_req_receiving.............: avg=151.39µs min=26.01µs med=41.7µs  max=95.1ms   p(90)=94.67µs p(95)=412.14µs
     http_req_sending...............: avg=23.71µs  min=5.49µs  med=10.85µs max=23.02ms  p(90)=16.69µs p(95)=27.97µs 
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s      max=0s       p(90)=0s      p(95)=0s      
     http_req_waiting...............: avg=20.98ms  min=2.32ms  med=20.15ms max=103.61ms p(90)=28.39ms p(95)=31.62ms 
     http_reqs......................: 69346   2306.327786/s
     iteration_duration.............: avg=21.63ms  min=6.77ms  med=20.63ms max=255.92ms p(90)=29.1ms  p(95)=32.43ms 
     iterations.....................: 69326   2305.662621/s
     vus............................: 50      min=50        max=50 
     vus_max........................: 50      min=50        max=50

github-actions · 2025-10-31T10:25:12Z

🐋 This PR was built and pushed to the following Docker images:

Image Names: ghcr.io/graphql-hive/router

Platforms: linux/amd64,linux/arm64

Image Tags: ghcr.io/graphql-hive/router:pr-540 ghcr.io/graphql-hive/router:sha-a7217d0

Docker metadata

{
"buildx.build.ref": "builder-0c044f54-0e2b-44ec-bfea-e82a686617ee/builder-0c044f54-0e2b-44ec-bfea-e82a686617ee0/y48xbq6zugd2ihjm2u5orc7zd",
"containerimage.descriptor": {
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "digest": "sha256:bddf9468aac1eed8ed60e9aafe2961dd07f839d788b19fcc1f45309bcf028c53",
  "size": 1609
},
"containerimage.digest": "sha256:bddf9468aac1eed8ed60e9aafe2961dd07f839d788b19fcc1f45309bcf028c53",
"image.name": "ghcr.io/graphql-hive/router:pr-540,ghcr.io/graphql-hive/router:sha-a7217d0"
}

kamilkisiela · 2025-10-31T11:49:25Z

lib/router-config/src/primitives/expression.rs

+static VRL_FUNCTIONS: Lazy<Vec<Box<dyn Function>>> = Lazy::new(vrl_build_functions);
+static VRL_TIMEZONE: Lazy<VrlTimeZone> = Lazy::new(VrlTimeZone::default);


Let's not do it this way, where we centralize the function building.
Yeah, right now we pass all functions that vrl provides, but if we ever want to add one made by us or filter some, for certain expressions, we would have to opt out and, we won't be able to do it without basically reverting your changes

I just didn't want to copy/paste the same logic again like we did 3 different places before. How do you think I should do that instead?

You can create a function and reuse it. If we ever want to opt-out, we either slightly refactor it or duplicate.

if you move this logic to the config crate then we’re not able to do those small adjustments in future

Looks better now?

Yes, it's much better now. Thanks

If we ever want to use different set of function, we can now do:

compile_expression(expression, Some(fns))

and add pre-built functions anywhere in code, where it makes sense.

kamilkisiela · 2025-11-03T10:13:34Z

lib/router-config/src/override_labels.rs

-        /// An expression that must evaluate to a boolean. If true, the label will be applied.
-        expression: String,
-    },
+    Expression { expression: String },


original comment is missing

Reverted 👍

kamilkisiela · 2025-11-03T10:21:06Z

lib/executor/Cargo.toml

 indexmap = "2.10.0"
 bumpalo = "3.19.0"
+once_cell = "1.21.3"
+schemars = "1.0.4"


you sure we need schemars in executor?

Removed 👍

Removed `pool_idle_timeout_seconds` from `traffic_shaping` and replaced it with `pool_idle_timeout` using duration format.

This comment was marked as outdated.

Sign in to view

gemini-code-assist bot reviewed Oct 31, 2025

View reviewed changes

lib/router-config/src/primitives/expression.rs Outdated Show resolved Hide resolved

lib/router-config/src/primitives/expression.rs Outdated Show resolved Hide resolved

kamilkisiela reviewed Oct 31, 2025

View reviewed changes

ardatan changed the title ~~New Expression type to handle VRL in one place~~ Shared utilities to handle VRL expressions Oct 31, 2025

kamilkisiela reviewed Nov 3, 2025

View reviewed changes

ardatan force-pushed the primitive-expression branch from 2644a55 to 128b10a Compare November 3, 2025 10:40

ardatan and others added 7 commits November 3, 2025 06:30

New Expression type to handle VRL in one place

b45e012

Fix

65b7617

Readme

c9ca484

Format

dae1e06

Revert

cb63733

Remove extra dep and apply comments

e1111dc

Auto generate changeset

766bd0e

ardatan enabled auto-merge (squash) November 3, 2025 11:30

ardatan force-pushed the primitive-expression branch from 128b10a to 766bd0e Compare November 3, 2025 11:30

ardatan added 2 commits November 3, 2025 15:12

Duration

6b9ee81

Fix config'

47f8297

ardatan changed the title ~~Shared utilities to handle VRL expressions~~ Shared utilities to handle VRL expressions & some other improvements Nov 3, 2025

ardatan mentioned this pull request Nov 3, 2025

docs(router): new pool_idle_timeout graphql-hive/console#7207

Open

ardatan requested a review from kamilkisiela November 3, 2025 12:51

Update traffic shaping configuration for VRL expressions

7ebe2d9

Removed `pool_idle_timeout_seconds` from `traffic_shaping` and replaced it with `pool_idle_timeout` using duration format.

ardatan requested a review from dotansimha November 3, 2025 12:55

dotansimha force-pushed the main branch 4 times, most recently from 4cffb36 to c709bec Compare November 4, 2025 11:27

		static VRL_FUNCTIONS: Lazy<Vec<Box<dyn Function>>> = Lazy::new(vrl_build_functions);
		static VRL_TIMEZONE: Lazy<VrlTimeZone> = Lazy::new(VrlTimeZone::default);

Shared utilities to handle VRL expressions & some other improvements #540

Are you sure you want to change the base?

Shared utilities to handle VRL expressions & some other improvements #540

Conversation

ardatan commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ k6-benchmark results

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kamilkisiela Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ardatan commented Oct 31, 2025 •

edited

Loading

github-actions bot commented Oct 31, 2025 •

edited

Loading

✅ `k6-benchmark` results

github-actions bot commented Oct 31, 2025 •

edited

Loading

kamilkisiela Oct 31, 2025 •

edited

Loading