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

fix(prom): graphql-ws integration #3651

Merged
merged 2 commits into from
Jan 15, 2025
Merged

fix(prom): graphql-ws integration #3651

merged 2 commits into from
Jan 15, 2025

Conversation

ardatan
Copy link
Collaborator

@ardatan ardatan commented Jan 14, 2025

Fixes #3374
Fixes graphql-hive/gateway#450

Skip collecting Yoga/HTTP specific metrics in case of WS or any other usage of the Envelop instance of Yoga

Copy link

changeset-bot bot commented Jan 14, 2025

🦋 Changeset detected

Latest commit: d9a0bfe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-yoga/plugin-prometheus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

Apollo Federation Subgraph Compatibility Results

Federation 1 Support Federation 2 Support
_service🟢
@key (single)🟢
@key (multi)🟢
@key (composite)🟢
repeatable @key🟢
@requires🟢
@provides🟢
federated tracing🟢
@link🟢
@shareable🟢
@tag🟢
@override🟢
@inaccessible🟢
@composeDirective🟢
@interfaceObject🟢

Learn more:

Copy link
Contributor

💻 Website Preview

The latest changes are available as preview in: https://4d1d0191.graphql-yoga.pages.dev

Copy link
Contributor

github-actions bot commented Jan 14, 2025

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}
     ✓ no_errors{mode:uws}
     ✓ expected_result{mode:uws}

     checks.......................................: 100.00% ✓ 518370      ✗ 0     
     data_received................................: 2.1 GB  14 MB/s
     data_sent....................................: 104 MB  695 kB/s
     http_req_blocked.............................: avg=1.59µs   min=1.05µs   med=1.38µs   max=305.88µs p(90)=2.1µs    p(95)=2.3µs   
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=164.94µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=358.99µs min=212.91µs med=326.68µs max=17.75ms  p(90)=466.53µs p(95)=489.16µs
       { expected_response:true }.................: avg=358.99µs min=212.91µs med=326.68µs max=17.75ms  p(90)=466.53µs p(95)=489.16µs
     ✓ { mode:graphql-jit }.......................: avg=286.3µs  min=212.91µs med=269.09µs max=15.43ms  p(90)=300.66µs p(95)=318.04µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=491.85µs min=405.56µs med=466.87µs max=5.86ms   p(90)=507.1µs  p(95)=547.96µs
     ✓ { mode:graphql-response-cache }............: avg=342.32µs min=267.6µs  med=324.98µs max=6.88ms   p(90)=354.19µs p(95)=365.13µs
     ✓ { mode:graphql }...........................: avg=370.01µs min=279.94µs med=340.08µs max=17.75ms  p(90)=394.8µs  p(95)=466.34µs
     ✓ { mode:uws }...............................: avg=341.44µs min=266.67µs med=322.7µs  max=6.56ms   p(90)=356.87µs p(95)=380µs   
     http_req_failed..............................: 0.00%   ✓ 0           ✗ 259185
     http_req_receiving...........................: avg=33.63µs  min=16.33µs  med=33.24µs  max=2.94ms   p(90)=39.76µs  p(95)=42.64µs 
     http_req_sending.............................: avg=8.88µs   min=6.03µs   med=7.85µs   max=5.93ms   p(90)=11.3µs   p(95)=12.46µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=316.47µs min=182.45µs med=285.19µs max=17.57ms  p(90)=423.62µs p(95)=444.38µs
     http_reqs....................................: 259185  1727.879501/s
     iteration_duration...........................: avg=573.65µs min=389.99µs med=537.12µs max=18.57ms  p(90)=684.39µs p(95)=711.64µs
     iterations...................................: 259185  1727.879501/s
     vus..........................................: 1       min=1         max=1   
     vus_max......................................: 2       min=2         max=2   

};
onParse({ context }) {
// If only it is Yoga, we calculate HTTP request time
if (context.request) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand why we don't want to also track subscriptions ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subscriptions are not the case here. Yoga subscriptions(SSE) are still tracked.
The issue is when GraphQL WS uses the Envelop instance of Yoga, this plugin fails because GraphQL WS doesn't have request object in the context.
It is ok to skip this because GraphQL WS is not HTTP at all.
This doesn't skip tracking entirely, it just skips the metric starts on onRequest and ends in onResponse hooks. Those hooks are not triggered by GraphQL WS at all.

@ardatan ardatan merged commit 690294a into main Jan 15, 2025
24 checks passed
@ardatan ardatan deleted the prom-graphql-ws branch January 15, 2025 14:12
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.

Prometheus with sub websocket @graphql-yoga/plugin-prometheus breaks subscriptions over graphql-ws
3 participants