fix: federation query planning issues #355

YassinEldeeb · 2024-01-23T13:02:48Z

github-actions · 2024-01-23T13:03:16Z

🚨 Rust Panic Audit: 210 Potential Panic Points Detected 🚨

Crate: `federation_query_planner`

📊 Total Usages: 90

🔎 expect usages: 12
🔢 array_index usages: 8
🎁 unwrap usages: 68
🚨 panic usages: 2

Crate: `engine`

📊 Total Usages: 25

🎁 unwrap usages: 24
🔎 expect usages: 1

Crate: `trusted_documents`

📊 Total Usages: 15

🎁 unwrap usages: 15

Crate: `jwt_auth`

📊 Total Usages: 11

🔢 array_index usages: 1
🎁 unwrap usages: 10

Crate: `common`

📊 Total Usages: 11

🎁 unwrap usages: 10
🔢 array_index usages: 1

Crate: `telemetry`

📊 Total Usages: 9

🔢 array_index usages: 5
🎁 unwrap usages: 4

Crate: `http_get`

📊 Total Usages: 8

🎁 unwrap usages: 8

Crate: `cloudflare_worker`

📊 Total Usages: 8

🚨 panic usages: 1
🎁 unwrap usages: 5
🔎 expect usages: 2

Crate: `tracing`

📊 Total Usages: 6

🎁 unwrap usages: 5
🔎 expect usages: 1

Crate: `vrl`

📊 Total Usages: 6

🔢 array_index usages: 2
🎁 unwrap usages: 4

Crate: `disable_introspection`

📊 Total Usages: 5

🎁 unwrap usages: 5

Crate: `conductor`

📊 Total Usages: 5

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

Crate: `cors`

📊 Total Usages: 4

🎁 unwrap usages: 4

Crate: `config`

📊 Total Usages: 3

🚨 panic usages: 1
🎁 unwrap usages: 2

Crate: `graphql_validation`

📊 Total Usages: 2

🎁 unwrap usages: 2

Crate: `match_content_type`

📊 Total Usages: 1

🎁 unwrap usages: 1

Crate: `graphiql`

📊 Total Usages: 1

🎁 unwrap usages: 1

📌 Expected Annotations

Crate: `common`

📊 Total Expected Usages: 1

expand details

Reason: "we're parsing a statically defined constant, we know it works ;)"

Code: .unwrap()
Location: ./libs/common/src/graphql.rs:31

Crate: `config`

📊 Total Expected Usages: 9

expand details

Reason: "statically defined regex pattern, we know it works ;)"

Code: .unwrap();
Location: ./libs/config/src/interpolate.rs:18

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

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

Reason: "👇"

Code: let raw_contents = read_to_string(file_path)
Location: ./libs/config/src/lib.rs:815

Reason: "👇"

Code: panic!("Failed to interpolate config file, please resolve the above errors");
Location: ./libs/config/src/lib.rs:847

Reason: "👇"

Code: parse_config_from_json(&config_string).expect("Failed to parse JSON config file")
Location: ./libs/config/src/lib.rs:854

Reason: "👇"

Code: parse_config_from_yaml(&config_string).expect("Failed to parse YAML config file")
Location: ./libs/config/src/lib.rs:858

Reason: "👇"

Code: _ => panic!("Unsupported config file extension"),
Location: ./libs/config/src/lib.rs:875

Reason: "👇"

Code: None => panic!("Config file has no extension"),
Location: ./libs/config/src/lib.rs:878

Crate: `conductor`

📊 Total Expected Usages: 2

expand details

Reason: "we need to exit the process, if the logger can't be correctly set."

Code: let _guard = tracing::subscriber::set_default(subscriber);
Location: ./bin/conductor/src/lib.rs:37

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:76

Crate: `cloudflare_worker`

📊 Total Expected Usages: 4

expand details

Reason: "it panics only if the header name is not valid, and we know it is."

Code: .unwrap()
Location: ./bin/cloudflare_worker/src/http_tracing.rs:20

Reason: "it panics only if the URL source is not valid, and it's already validated before."

Code: let url = req.url().unwrap();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:23

Reason: "it only panics if we are not running in a CF context, should be safe."

Code: let cf_info = req.cf().unwrap();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:27

Reason: "unwraps only in special cases where "data:text" is used."

Code: let http_host = url.host().unwrap().to_string();
Location: ./bin/cloudflare_worker/src/http_tracing.rs:36

Crate: `vrl`

📊 Total Expected Usages: 2

expand details

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:130

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:147

Crate: `napi`

📊 Total Expected Usages: 1

expand details

Reason: "we need this"

Code: panic!("Exited process!")
Location: ./libs/napi/src/lib.rs:18

Crate: `jwt_auth`

📊 Total Expected Usages: 1

expand details

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:49

Crate: `engine`

📊 Total Expected Usages: 2

expand details

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:162

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:194

github-actions · 2024-01-23T13:10:14Z

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

Docker Bake metadata

{
"conductor": {
  "buildx.build.provenance": {
    "buildType": "https://mobyproject.org/buildkit@v1",
    "materials": [
      {
        "uri": "pkg:docker/[email protected]?platform=linux%2Famd64",
        "digest": {
          "sha256": "1aadfee8d292f64b045adb830f8a58bfacc15789ae5f489a0fedcd517a862cb9"
        }
      },
      {
        "uri": "pkg:docker/[email protected]?platform=linux%2Famd64",
        "digest": {
          "sha256": "83101f6985c93e1e6501b3375de188ee3d2cbb89968bcc91611591f9f447bd42"
        }
      }
    ],
    "invocation": {
      "configSource": {
        "entryPoint": "Dockerfile"
      },
      "parameters": {
        "frontend": "dockerfile.v0",
        "args": {
          "label:org.opencontainers.image.authors": "The Guild <[email protected]>",
          "label:org.opencontainers.image.description": "Conductor is a robust GraphQL Gateway.",
          "label:org.opencontainers.image.docs": "https://the-guild.dev/graphql/gateway",
          "label:org.opencontainers.image.licenses": "MIT",
          "label:org.opencontainers.image.revision": "c80a1c9fb9014bee93dc1326e42c8e0be95ab212",
          "label:org.opencontainers.image.source": "https://github.com/the-guild-org/conductor",
          "label:org.opencontainers.image.title": "Conductor",
          "label:org.opencontainers.image.url": "https://the-guild.dev/graphql/gateway",
          "label:org.opencontainers.image.vendor": "The Guild",
          "label:org.opencontainers.image.version": ""
        },
        "locals": [
          {
            "name": "context"
          },
          {
            "name": "dockerfile"
          }
        ]
      },
      "environment": {
        "platform": "linux/amd64"
      }
    }
  },
  "buildx.build.ref": "builder-b49469fe-372a-47b5-b469-4a30628c08c2/builder-b49469fe-372a-47b5-b469-4a30628c08c20/xm0rho4o83bzmfdacy5vp0vqz",
  "containerimage.config.digest": "sha256:93a8583aa0785b2145774de0c0038bc380a5694585b13acb39ec70993e24ec98",
  "containerimage.descriptor": {
    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
    "digest": "sha256:964cae76f47b863d2f9072dbc2508a3758c5efb0a54b06f9eb526ba43fa64e35",
    "size": 902,
    "platform": {
      "architecture": "amd64",
      "os": "linux"
    }
  },
  "containerimage.digest": "sha256:964cae76f47b863d2f9072dbc2508a3758c5efb0a54b06f9eb526ba43fa64e35",
  "image.name": "ghcr.io/the-guild-org/conductor/conductor:c80a1c9fb9014bee93dc1326e42c8e0be95ab212"
}
}

github-actions · 2024-01-23T13:14:08Z

❌ Benchmark Failed

Performance regression detected: it seems like your Pull Request adds some extra latency to Conductor request hot path.

If the performance regression is expected, please increase the failing threshold.

     data_received..............: 1.3 kB  14 B/s
     data_sent..................: 158 kB  1.8 kB/s
     dropped_iterations.........: 59564   661.802068/s
     http_req_blocked...........: min=83.53µs avg=202.44µs med=152.56µs max=2.8ms    p(95)=436.66µs p(99)=1.69ms  
     http_req_connecting........: min=58.49µs avg=151.69µs med=99.28µs  max=2.75ms   p(95)=387.8µs  p(99)=1.66ms  
     http_req_duration..........: min=1.01s   avg=35.15s   med=41.08s   max=1m0s     p(95)=1m0s     p(99)=1m0s    
     ✗ { scenario:rps_1000 }....: min=1.01s   avg=35.15s   med=41.08s   max=1m0s     p(95)=1m0s     p(99)=1m0s    
     http_req_failed............: 100.00% ✓ 400        ✗ 0    
     ✗ { scenario:rps_1000 }....: 100.00% ✓ 400        ✗ 0    
     http_req_receiving.........: min=0s      avg=1.11µs   med=0s       max=47.73µs  p(95)=0s       p(99)=42.59µs 
     http_req_sending...........: min=10.76µs avg=30.42µs  med=25.91µs  max=187.82µs p(95)=59.44µs  p(99)=133.79µs
     http_req_tls_handshaking...: min=0s      avg=0s       med=0s       max=0s       p(95)=0s       p(99)=0s      
     http_req_waiting...........: min=1.01s   avg=35.15s   med=41.08s   max=1m0s     p(95)=1m0s     p(99)=1m0s    
     http_reqs..................: 400     4.444309/s
     ✗ { scenario:rps_1000 }....: 400     4.444309/s
     iteration_duration.........: min=1.01s   avg=35.15s   med=41.08s   max=1m0s     p(95)=1m0s     p(99)=1m0s    
     iterations.................: 400     4.444309/s
     ✗ { scenario:rps_1000 }....: 400     4.444309/s
     valid_graphql_response.....: 0.00%   ✓ 0          ✗ 0    
     ✓ { scenario:rps_1000 }....: 0.00%   ✓ 0          ✗ 0    
     valid_http_code............: 0.00%   ✓ 0          ✗ 0    
     ✓ { scenario:rps_1000 }....: 0.00%   ✓ 0          ✗ 0    
     vus........................: 36      min=36       max=200
     vus_max....................: 200     min=200      max=200

dotansimha · 2024-03-17T10:50:28Z

I think it needs a rebase? 👀

YassinEldeeb · 2024-03-17T10:55:18Z

yes! on it today!

refactor restructure default to Query in parent type fixes cleaning up execution depend on depends on refactor fix: execution layer memory issues regarding responses make FieldNode an arc rwlock fixes fix errors with supergraph parsing and source rules accurate paths introspection issues fixes fix $representation argument generation refine merging polishing fixes fixes fix rebase issues resolve lifetimes issues lifetimes fixes lifetimes fixes add custom lock loggers fix logging lock update lock usage

YassinEldeeb force-pushed the fix-response-merging branch 2 times, most recently from 7d9963f to c4eb21f Compare January 30, 2024 02:08

YassinEldeeb changed the title ~~fix: rewrite response merging~~ fix: federation query planning Jan 30, 2024

YassinEldeeb changed the title ~~fix: federation query planning~~ fix: federation query planning issues Jan 30, 2024

YassinEldeeb marked this pull request as ready for review March 17, 2024 10:44

dotansimha mentioned this pull request Mar 17, 2024

fix(deps): update async-graphql to 7.0.3 #455

Closed

1 task

YassinEldeeb force-pushed the fix-response-merging branch from ddcc400 to 936c185 Compare March 26, 2024 11:47

dotansimha force-pushed the fix-response-merging branch from 936c185 to 6d26241 Compare March 26, 2024 12:14

YassinEldeeb force-pushed the fix-response-merging branch from 6d26241 to a11928c Compare April 3, 2024 07:16

dotansimha force-pushed the fix-response-merging branch from 61e64ca to e7e30e4 Compare April 8, 2024 13:23

YassinEldeeb added 9 commits April 10, 2024 10:06

cleanup

2d4ee4f

format plan.json

6d2fab8

response merging try1

f6c270c

remove logs

bc0ece2

fix introspection

696a88e

plan.json

f354281

fix type unwraping

b6e7c24

cleanup

1d8b93c

cleanup

c80a1c9

dotansimha closed this Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: federation query planning issues #355

fix: federation query planning issues #355

YassinEldeeb commented Jan 23, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024 •

edited

Loading

dotansimha commented Mar 17, 2024

YassinEldeeb commented Mar 17, 2024

fix: federation query planning issues #355

fix: federation query planning issues #355

Conversation

YassinEldeeb commented Jan 23, 2024 • edited Loading

github-actions bot commented Jan 23, 2024 • edited Loading

🚨 Rust Panic Audit: 210 Potential Panic Points Detected 🚨

Crate: federation_query_planner

Crate: engine

Crate: trusted_documents

Crate: jwt_auth

Crate: common

Crate: telemetry

Crate: http_get

Crate: cloudflare_worker

Crate: tracing

Crate: vrl

Crate: disable_introspection

Crate: conductor

Crate: cors

Crate: config

Crate: graphql_validation

Crate: match_content_type

Crate: graphiql

📌 Expected Annotations

Crate: common

Crate: config

Crate: conductor

Crate: cloudflare_worker

Crate: vrl

Crate: napi

Crate: jwt_auth

Crate: engine

github-actions bot commented Jan 23, 2024 • edited Loading

github-actions bot commented Jan 23, 2024 • edited Loading

❌ Benchmark Failed

dotansimha commented Mar 17, 2024

YassinEldeeb commented Mar 17, 2024

YassinEldeeb commented Jan 23, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024 •

edited

Loading

Crate: `federation_query_planner`

Crate: `engine`

Crate: `trusted_documents`

Crate: `jwt_auth`

Crate: `common`

Crate: `telemetry`

Crate: `http_get`

Crate: `cloudflare_worker`

Crate: `tracing`

Crate: `vrl`

Crate: `disable_introspection`

Crate: `conductor`

Crate: `cors`

Crate: `config`

Crate: `graphql_validation`

Crate: `match_content_type`

Crate: `graphiql`

Crate: `common`

Crate: `config`

Crate: `conductor`

Crate: `cloudflare_worker`

Crate: `vrl`

Crate: `napi`

Crate: `jwt_auth`

Crate: `engine`

github-actions bot commented Jan 23, 2024 •

edited

Loading

github-actions bot commented Jan 23, 2024 •

edited

Loading