feat(opentelemetry): add schema coordinate to error events #1636
Conversation
Summary of ChangesHello @EmrysMyrddin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the observability of GraphQL operations by integrating detailed schema coordinate information into OpenTelemetry error events. It also refines the handling of Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the schemaCoordinate to OpenTelemetry error events, which is a great enhancement for error tracking. The changes look good overall. I've added a few comments regarding documentation clarity, potential improvements for code maintainability, and a small safety improvement. The new abortSignalAll utility is a nice addition for memory-safe signal handling, though its documentation and related tests have some inconsistencies.
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-hive/gateway |
2.1.14-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/nestjs |
2.0.19-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-aws-sigv4 |
2.0.14-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-deduplicate-request |
2.0.6-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/plugin-opentelemetry |
1.0.16-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-prometheus |
2.0.17-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-runtime |
2.3.2-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-testing |
2.0.2-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http |
1.0.10-alpha-3cb754de986123ebad30cdea4fd47417d8fff5cd |
npm ↗︎ unpkg ↗︎ |
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Binary for
|
🚀 Snapshot Release (Node Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared |
🚀 Snapshot Release (Bun Docker Image)The latest changes of this PR are available as image on GitHub Container Registry (based on the declared |
🚀 Snapshot Release (Binary for
|
EmrysMyrddin
force-pushed
the
feat/error-schema-coord
branch
3 times, most recently
from
1395885 to
e9883a0
Compare
|
@n1ru4l It should be ready to test and experiment on your end 😃 |
EmrysMyrddin
force-pushed
the
feat/error-schema-coord
branch
from
e9883a0 to
7cab6c7
Compare
EmrysMyrddin
force-pushed
the
feat/error-schema-coord
branch
from
7cab6c7 to
3cb754d
Compare
2 participants
Context
Today, errors as reported as classic OTEL Exceptions. Following the work of the GraphQL-OTEL working group (graphql/otel-wg#50) on GraphQL error reporting, Exceptions are not the right model fit.
Exception are for uncaught runtime errors. GraphQL Errors in the other hand are user facing business errors, with much more metadata than the traditional
message,typeandstacktrace.Related to GW-501
Solution
This PR migrates GraphQL errors to OTEL Event. This is following upcomging OTEL standard for GraphQL, and allows to add more metadata to it.
Errors are also attached to the operation root span, for easier filtering in dashboards.
Normal JS Error are still recorded as OTEL Exceptions, only actual
GraphQLErrorfound in the result, or returned byvalidationandparsingphases will be reported as Events.All metadata available in
GraphQLerror are reported as attribute of the event:messagepathlocationscode: If it exists, the commonly usederror.extensions.codeallowing to group errors of the same kind.coordinate: a new extension added by this PR allowing to identify the faulty resolver.For the last attribute, the plugin have to override the execution function to enable a flag that adds schema coordinate to errors. This feature is only available when using our own fork of the graphql-js executor (which is the case by default).
Side Quest
By modifying the batch executor, I also noted that the signal is just ignored when merging multiple execution requests.
I've fixed that by adding a new util to merge a list of signal into one that gets aborted only once all signals are aborted or GCed.
Questions
This is changing how errors are reported in a few ways, which can break user dashboard or alerts depending on how they have written filters:
exceptionbutgraphql.errorgraphql.operationspan instead of the specific phase span (graphql.execute,graphql.validateandgraphql.parse)TODO