Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: federation support + query planner #73

Merged
merged 22 commits into from
Jan 11, 2024

Conversation

YassinEldeeb
Copy link
Member

@YassinEldeeb YassinEldeeb commented Aug 17, 2023

Closes #7

@YassinEldeeb YassinEldeeb changed the title v2: query planner better complete impl take2: query planner better complete impl Sep 11, 2023
@YassinEldeeb YassinEldeeb marked this pull request as ready for review November 6, 2023 03:30
@YassinEldeeb YassinEldeeb force-pushed the query-planner-better-complete-impl branch from 7e3675b to 3d73984 Compare November 19, 2023 17:14
Copy link

github-actions bot commented Nov 19, 2023

✅ Benchmark Results

     data_received..................: 13 MB   221 kB/s
     data_sent......................: 22 MB   363 kB/s
     http_req_blocked...............: min=1.2µs    avg=3.11µs   med=2.45µs   max=1.18ms  p(95)=3.44µs  p(99)=6.21µs 
     http_req_connecting............: min=0s       avg=378ns    med=0s       max=1.15ms  p(95)=0s      p(99)=0s     
     http_req_duration..............: min=775.41µs avg=1.01ms   med=916.51µs max=48.77ms p(95)=1.31ms  p(99)=1.92ms 
       { expected_response:true }...: min=775.41µs avg=1.01ms   med=916.51µs max=48.77ms p(95)=1.31ms  p(99)=1.92ms 
     ✓ { scenario:rps_1000 }........: min=775.41µs avg=1.01ms   med=916.51µs max=48.77ms p(95)=1.31ms  p(99)=1.92ms 
     http_req_failed................: 0.00%   ✓ 0          ✗ 60001
     ✓ { scenario:rps_1000 }........: 0.00%   ✓ 0          ✗ 60001
     http_req_receiving.............: min=10.11µs  avg=30.27µs  med=28.22µs  max=6.39ms  p(95)=47.27µs p(99)=60.93µs
     http_req_sending...............: min=7.4µs    avg=15.96µs  med=13.79µs  max=2.45ms  p(95)=29.08µs p(99)=38.06µs
     http_req_tls_handshaking.......: min=0s       avg=0s       med=0s       max=0s      p(95)=0s      p(99)=0s     
     http_req_waiting...............: min=738.69µs avg=969.75µs med=869.82µs max=48.72ms p(95)=1.26ms  p(99)=1.84ms 
     http_reqs......................: 60001   999.996818/s
     ✓ { scenario:rps_1000 }........: 60001   999.996818/s
     iteration_duration.............: min=857.69µs avg=1.11ms   med=1.01ms   max=49.01ms p(95)=1.41ms  p(99)=2.09ms 
     iterations.....................: 60001   999.996818/s
     ✓ { scenario:rps_1000 }........: 60001   999.996818/s
     valid_graphql_response.........: 100.00% ✓ 60001      ✗ 0    
     ✓ { scenario:rps_1000 }........: 100.00% ✓ 60001      ✗ 0    
     valid_http_code................: 100.00% ✓ 60001      ✗ 0    
     ✓ { scenario:rps_1000 }........: 100.00% ✓ 60001      ✗ 0    
     vus............................: 2       min=0        max=3  
     vus_max........................: 200     min=200      max=200

@YassinEldeeb YassinEldeeb changed the title take2: query planner better complete impl feat: federation support + query planner Nov 19, 2023
libs/config/src/lib.rs Outdated Show resolved Hide resolved
libs/engine/Cargo.toml Outdated Show resolved Hide resolved
libs/config/src/lib.rs Outdated Show resolved Hide resolved
@YassinEldeeb
Copy link
Member Author

YassinEldeeb commented Nov 19, 2023

there's so much to cleanup in the query planner code, a lot of unused utilities, and not a very well structured code.
will spend tomorrow on that specifically.

really appreciate the initial review! ❤️

@dotansimha dotansimha force-pushed the query-planner-better-complete-impl branch 2 times, most recently from cfe0f79 to ebfa2ad Compare December 11, 2023 07:26
@the-guild-org the-guild-org deleted a comment from github-actions bot Dec 11, 2023
Copy link

github-actions bot commented Dec 11, 2023

💻 Website Preview

The latest changes are available as preview in: https://4b371d51.conductor-t2.pages.dev

Copy link

github-actions bot commented Dec 11, 2023

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

Docker Bake metadata
{
"conductor": {
  "containerimage.config.digest": "sha256:e4f6c9a913eb5b624d2b467e699fb9e7b3c65785fa0acf5b0dcada9ba6249e8d",
  "containerimage.descriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "digest": "sha256:f213ec2e2e3d0866a437215f13c65b96a2d9e503b102b1fcd77799765d357648",
    "size": 902,
    "platform": {
      "architecture": "amd64",
      "os": "linux"
    }
  },
  "containerimage.digest": "sha256:f213ec2e2e3d0866a437215f13c65b96a2d9e503b102b1fcd77799765d357648",
  "image.name": "ghcr.io/the-guild-org/conductor-t2/conductor:f65472a47e4454b21869341db9e4143b9e7d4e1a"
}
}

@dotansimha dotansimha force-pushed the query-planner-better-complete-impl branch 11 times, most recently from 77f6436 to d0a79fe Compare December 11, 2023 09:51
@YassinEldeeb YassinEldeeb force-pushed the query-planner-better-complete-impl branch from 5f481b0 to c6ab534 Compare January 9, 2024 14:00
Copy link

github-actions bot commented Jan 9, 2024

🚨 Rust Panic Audit: 59 Potential Panic Points Detected 🚨

Crate: federation_query_planner

📊 Total Usages: 50

  • 🎁 unwrap usages: 29
  • 🚨 panic usages: 3
  • 🔢 array_index usages: 10
  • 🔎 expect usages: 8

Crate: engine

📊 Total Usages: 9

  • 🔎 expect usages: 1
  • 🚨 panic usages: 3
  • 🎁 unwrap usages: 5

📌 Expected Annotations

Crate: config

📊 Total Expected Usages: 9

expand details
  1. Reason: "statically defined regex pattern, we know it works ;)"
  • Code: .unwrap();
  • Location: ./libs/config/src/interpolate.rs:18
  1. Reason: "👇"
  • Code: let raw_contents = read_to_string(file_path).expect("Failed to read config file");
  • Location: ./libs/config/src/lib.rs:415
  1. Reason: "👇"
  • Code: panic!("Failed to interpolate config file, please resolve the above errors");
  • Location: ./libs/config/src/lib.rs:445
  1. Reason: "👇"
  • Code: parse_config_from_json(&config_string).expect("Failed to parse JSON config file")
  • Location: ./libs/config/src/lib.rs:452
  1. Reason: "👇"
  • Code: parse_config_from_yaml(&config_string).expect("Failed to parse YAML config file")
  • Location: ./libs/config/src/lib.rs:456
  1. Reason: "👇"
  • Code: _ => panic!("Unsupported config file extension"),
  • Location: ./libs/config/src/lib.rs:473
  1. Reason: "👇"
  • Code: None => panic!("Config file has no extension"),
  • Location: ./libs/config/src/lib.rs:476
  1. Reason: "part of development docgen CLI"
  • Code: .expect("Failed to serialize json schema for config file!");
  • Location: ./libs/config/src/generate-json-schema.rs:50
  1. Reason: "part of development docgen CLI"
  • Code: .expect("Failed to write the json schema to the file system!");
  • Location: ./libs/config/src/generate-json-schema.rs:54

Crate: common

📊 Total Expected Usages: 1

expand details
  1. Reason: "we're parsing a statically defined constant, we know it works ;)"
  • Code: .unwrap()
  • Location: ./libs/common/src/graphql.rs:22

Crate: engine

📊 Total Expected Usages: 3

expand details
  1. Reason: "if we are unable to construct the endpoints and attach them onto the gateway's http server, we have to exit"
  • Code: Err(e) => panic!("failed to construct endpoint: {:?}", e),
  • Location: ./libs/engine/src/gateway.rs:113
  1. Reason: "we can safely index here, it's inside a test with constant defined fixtures."
  • Code: ConductorGateway::execute(request, &gw.routes[0].route_data).await
  • Location: ./libs/engine/src/gateway.rs:144
  1. Reason: "without a fetcher, there's no executor, without an executor, there's no gateway."
  • Code: panic!(
  • Location: ./libs/engine/src/source/graphql_source.rs:28

Crate: napi

📊 Total Expected Usages: 1

expand details
  1. Reason: "we need this"
  • Code: panic!("Exited process!")
  • Location: ./libs/napi/src/lib.rs:18

Crate: vrl

📊 Total Expected Usages: 2

expand details
  1. Reason: "if the provided VRL code in the config file can't compile, we have to exit."
  • Code: panic!("failed to compile vrl program");
  • Location: ./plugins/vrl/src/plugin.rs:122
  1. Reason: "states is a non-user provided variable"
  • Code: .expect("can't merge states when states is an empty vector!")
  • Location: ./plugins/vrl/src/plugin.rs:139

Crate: jwt_auth

📊 Total Expected Usages: 1

expand details
  1. Reason: "if initiating an http client fails, then we have to exit."
  • Code: let client = wasm_polyfills::create_http_client().build().unwrap();
  • Location: ./plugins/jwt_auth/src/jwks_provider.rs:47

Crate: disable_introspection

📊 Total Expected Usages: 1

expand details
  1. Reason: "we need to exit the process if our provided VRL condition has incorrect syntax."
  • Code: panic!("failed to compile vrl program for disable_introspection plugin");
  • Location: ./plugins/disable_introspection/src/plugin.rs:33

Crate: conductor

📊 Total Expected Usages: 3

expand details
  1. Reason: "we need to exit the process, if the logger can't be correctly set."
  • Code: tracing::subscriber::set_global_default(subscriber).expect("failed to set up the logger");
  • Location: ./bin/conductor/src/lib.rs:30
  1. Reason: "we need to exit, if the provided log level in the configuration file is incompaitable."
  • Code: .expect("Failed to modify the log level");
  • Location: ./bin/conductor/src/lib.rs:45
  1. Reason: "we need to exit the process, if the provided configuration file is incorrect."
  • Code: panic!("Failed to initialize gateway: {:?}", e);
  • Location: ./bin/conductor/src/lib.rs:80

@YassinEldeeb YassinEldeeb merged commit 4dedc4e into master Jan 11, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Supergraph (Apollo) / Fed
2 participants