feat(runtime): Add new Tracer API #706
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add ignore rules for Helix configuration files and introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant UGM as UnifiedGraphManager
participant T as Tracer
participant Exec as Subgraph Executor
UGM->>UGM: Initialize with optional tracer
UGM->>T: Call subgraphExecute with payload & callback
T->>Exec: Execute wrapped subgraph query
Exec-->>T: Return execution result
T-->>UGM: Return tracing information
sequenceDiagram
participant GW as Gateway Runtime
participant Plug as Gateway Plugin
participant GPE as getProxyExecutor
GW->>GW: Initialize tracer variable
GW->>Plug: Check for tracer property in plugins
Plug-->>GW: Provide tracer (if exists)
GW->>GW: Enforce single tracing plugin rule
GW->>GPE: Pass tracer to proxy executor
GPE-->>GW: Proceed with execution
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
EmrysMyrddin
force-pushed
the
feat/runtime/tracer
branch
from
a6a5e6b to
ce53311
Compare
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-tools/executor-common |
0.0.3-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/executor-graphql-ws |
2.0.3-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/executor-http |
1.2.8-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-tools/federation |
3.1.4-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/fusion-runtime |
0.11.2-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway |
1.10.4-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-json |
0.0.2-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/logger-winston |
1.0.1-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/hmac-upstream-signature |
1.2.21-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-opentelemetry |
1.3.44-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/plugin-prometheus |
1.3.32-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-hive/gateway-runtime |
1.4.17-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-common |
0.7.30-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http |
0.6.34-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-http-callback |
0.5.21-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
@graphql-mesh/transport-ws |
1.0.4-alpha-ce53311a0a4eb8c65196a0cc3f55e77ed591145d |
npm ↗︎ unpkg ↗︎ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks)package.json(1 hunks)packages/fusion-runtime/src/unifiedGraphManager.ts(4 hunks)packages/fusion-runtime/src/utils.ts(5 hunks)packages/runtime/src/createGatewayRuntime.ts(5 hunks)packages/runtime/src/getProxyExecutor.ts(3 hunks)packages/runtime/src/types.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`packages/**`: In this directory we keep all packages releva...
packages/**: In this directory we keep all packages relevant to the gateway.In most cases, when changes are made to the source code / dependencies in these directories, we also require to have a corresponding changeset file, created using
yarn changesetwith a full description on the changes with an example.
Also, when new features are added the packages, the changeset file should mentionminorsemver change. When a bug is fixed,patchneeds to be used. Themajorbump is used for a PR that has breaking changes.
packages/runtime/src/getProxyExecutor.tspackages/fusion-runtime/src/unifiedGraphManager.tspackages/fusion-runtime/src/utils.tspackages/runtime/src/types.tspackages/runtime/src/createGatewayRuntime.ts
`**`: For all PRs, we would like to verify that a Linear iss...
**: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding
consoleproject.
Use the global knowledge feature to search for PRs ingraphql-hive/consolerepository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
packages/runtime/src/getProxyExecutor.tspackage.jsonpackages/fusion-runtime/src/unifiedGraphManager.tspackages/fusion-runtime/src/utils.tspackages/runtime/src/types.tspackages/runtime/src/createGatewayRuntime.ts
🪛 Biome (1.9.4)
packages/fusion-runtime/src/utils.ts
[error] 261-261: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: E2E / Bun Docker on Ubuntu
- GitHub Check: E2E / Bun on Ubuntu
- GitHub Check: E2E / Node Binary on Windows
- GitHub Check: E2E / Node Binary on Ubuntu
- GitHub Check: E2E / Node Docker on Ubuntu
- GitHub Check: E2E / Node 23 on Ubuntu
- GitHub Check: E2E / Node 22 on Ubuntu
- GitHub Check: Leaks / Node v23
- GitHub Check: E2E / Node 20 on Ubuntu
- GitHub Check: Leaks / Node v22
- GitHub Check: Leaks / Node v20
- GitHub Check: E2E / Node 18 on Ubuntu
- GitHub Check: Leaks / Node v18
- GitHub Check: Benchmark / bun / 1000 items
- GitHub Check: Benchmark / bun / 100 items
- GitHub Check: Snapshot / snapshot
- GitHub Check: Benchmark / bun / 10 items
- GitHub Check: Benchmark / node / 1000 items
- GitHub Check: Bundle
- GitHub Check: Benchmark / node / 100 items
- GitHub Check: Benchmark / node / 10 items
🔇 Additional comments (7)
packages/runtime/src/createGatewayRuntime.ts (6)
169-169: LGTM: Tracer variable declaration.The declaration of the
tracervariable at the module level ensures it's accessible throughout the gateway runtime setup.
230-230: LGTM: Consistent tracer propagation.The tracer is consistently propagated to all relevant functions (
getProxyExecutor,getOnSubgraphExecute, andUnifiedGraphManager), ensuring tracing capabilities are available throughout the execution flow.Also applies to: 451-451, 679-679
800-805: LGTM: Single tracer plugin enforcement.The implementation correctly enforces that only one tracing plugin can be active at a time, preventing potential conflicts or inconsistencies in tracing behavior.
169-169: LGTM!The tracer variable is correctly declared with the appropriate type from GatewayPlugin.
230-230: LGTM!The tracer is consistently integrated at all relevant execution points (proxy executor, subgraph execution, and unified graph manager).
Also applies to: 451-451, 679-679
800-805: LGTM!The implementation correctly enforces the single tracer plugin constraint with clear error messaging.
package.json (1)
74-74:❓ Verification inconclusive
Verify the alpha version of graphql-yoga.
The specified version of
graphql-yogais an alpha version with a future date (2025). This could lead to stability issues and should be reviewed.Please verify:
- Why an alpha version is needed instead of a stable release
- The correctness of the version string (it contains a future date)
🌐 Web query:
What is the latest stable version of graphql-yoga?💡 Result:
The latest stable version of GraphQL Yoga is v5.12.0, released two days before February 21, 2025[7]. This information comes directly from its npm registry entry, which lists it as the most recent version available for installation[7].
Key details:
- Published 2 days ago relative to the current date (2025-02-21)[7]
- Available via npm using
npm i graphql-yoga[7]- Part of an active maintenance cycle with frequent updates - there were 279 other projects using this version at time of publication[7]
Earlier versions like v5.10.10 appear in dependency logs[1][2][3][4][5], but these represent previous patch releases rather than the current stable release[7].
Citations:
- 1: https://the-guild.dev/graphql/yoga-server/docs/migration/migration-from-yoga-v2
- 2: https://the-guild.dev/graphql/yoga-server/changelogs/plugins/graphql-sse
- 3: https://the-guild.dev/graphql/yoga-server/changelogs/graphql-yoga
- 4: https://the-guild.dev/graphql/yoga-server/changelogs/plugins/apollo-managed-federation
- 5: https://github.com/prisma/graphql-yoga/releases
- 6: https://the-guild.dev/graphql/yoga-server/changelogs/graphiql
- 7: https://www.npmjs.com/package/graphql-yoga
- 8: https://the-guild.dev/graphql/yoga-server/changelogs/logger
- 9: https://the-guild.dev/graphql/yoga-server
- 10: https://www.npmjs.com/package/graphql-yoga/v/1.18.3
Action Required: Validate the Use of the Alpha Release for graphql-yoga
The dependency in
package.jsonnow specifies an alpha release ofgraphql-yoga:"graphql-yoga": "5.13.0-alpha-20250221161011-2a41f6a3b66fd9850d356e6c07cf9392344bc1ed",Note that this alpha version includes a future date in its version string, while the latest stable release available on npm is v5.12.0 (released just before February 21, 2025). Please verify:
- Intentional Use: Confirm if using this alpha version is deliberate—are there experimental features or fixes in v5.13.0-alpha that are needed over the stable v5.12.0?
- Version String Accuracy: Double-check that the version string with a future date is correct and not an accidental error.
| @@ -1,5 +1,6 @@ | |||
| import type { | |||
| OnSubgraphExecuteHook, | |||
| Tracer, | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add JSDoc documentation for the tracer parameter.
The newly added tracer parameter would benefit from JSDoc documentation explaining its purpose and usage.
Add documentation like this:
+ /**
+ * @param tracer - Optional tracer for monitoring subgraph execution
+ */Also applies to: 17-17, 24-24, 49-49
| export type Tracer = { | ||
| subgraphExecute( | ||
| payload: { executionRequest: ExecutionRequest }, | ||
| wrapped: () => MaybePromise<void>, | ||
| ): MaybePromise<void>; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance the Tracer type definition.
The Tracer type could be more flexible and better documented.
Consider these improvements:
+/**
+ * Tracer interface for monitoring subgraph execution
+ */
export type Tracer = {
+ /**
+ * Traces a subgraph execution
+ * @param payload - Contains the execution request details
+ * @param wrapped - The function to execute and trace
+ * @returns A promise that resolves when tracing is complete
+ */
subgraphExecute(
payload: { executionRequest: ExecutionRequest },
wrapped: () => MaybePromise<void>,
): MaybePromise<void>;
+ /**
+ * Optional method to clean up tracing resources
+ */
+ dispose?(): MaybePromise<void>;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type Tracer = { | |
| subgraphExecute( | |
| payload: { executionRequest: ExecutionRequest }, | |
| wrapped: () => MaybePromise<void>, | |
| ): MaybePromise<void>; | |
| }; | |
| /** | |
| * Tracer interface for monitoring subgraph execution | |
| */ | |
| export type Tracer = { | |
| /** | |
| * Traces a subgraph execution | |
| * @param payload - Contains the execution request details | |
| * @param wrapped - The function to execute and trace | |
| * @returns A promise that resolves when tracing is complete | |
| */ | |
| subgraphExecute( | |
| payload: { executionRequest: ExecutionRequest }, | |
| wrapped: () => MaybePromise<void>, | |
| ): MaybePromise<void>; | |
| /** | |
| * Optional method to clean up tracing resources | |
| */ | |
| dispose?(): MaybePromise<void>; | |
| }; |
| export type Tracer<TContext extends Record<string, any>> = | ||
| YogaTracer<TContext> & GatewayRuntimeTracer; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Document the Tracer type alias.
The Tracer type alias would benefit from documentation explaining its purpose and composition.
Add documentation like this:
+/**
+ * Combined tracer type that implements both Yoga and Gateway runtime tracing
+ * capabilities.
+ */
export type Tracer<TContext extends Record<string, any>> =
YogaTracer<TContext> & GatewayRuntimeTracer;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type Tracer<TContext extends Record<string, any>> = | |
| YogaTracer<TContext> & GatewayRuntimeTracer; | |
| /** | |
| * Combined tracer type that implements both Yoga and Gateway runtime tracing | |
| * capabilities. | |
| */ | |
| export type Tracer<TContext extends Record<string, any>> = | |
| YogaTracer<TContext> & GatewayRuntimeTracer; |
| @@ -170,6 +172,7 @@ export function getOnSubgraphExecute({ | |||
| transports, | |||
| getDisposeReason, | |||
| batch = true, | |||
| tracer, | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add JSDoc documentation for the tracer parameter.
The tracer parameter in getOnSubgraphExecute would benefit from documentation.
Add documentation like this:
+/**
+ * @param tracer - Optional tracer for monitoring subgraph execution
+ */Also applies to: 185-185
| if (tracer?.subgraphExecute) { | ||
| const originalExecutor = executor; | ||
| executor = (executionRequest) => { | ||
| let result: MaybePromise<MaybeAsyncIterable<ExecutionResult<any, any>>>; | ||
| const traced$ = tracer.subgraphExecute({executionRequest}, () => { | ||
| result = originalExecutor(executionRequest); | ||
| return isPromise(result) ? result.then(() => undefined): undefined; | ||
| }) | ||
| return isPromise(traced$) ? traced$.then(() => result) : result!; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Refactor tracer integration for better readability.
The tracer integration logic is complex and could be clearer. Also, there's a non-null assertion that should be handled more safely.
Consider extracting the tracer logic and improving null safety:
- if (tracer?.subgraphExecute) {
- const originalExecutor = executor;
- executor = (executionRequest) => {
- let result: MaybePromise<MaybeAsyncIterable<ExecutionResult<any, any>>>;
- const traced$ = tracer.subgraphExecute({executionRequest}, () => {
- result = originalExecutor(executionRequest);
- return isPromise(result) ? result.then(() => undefined): undefined;
- })
- return isPromise(traced$) ? traced$.then(() => result) : result!;
- };
- }
+ executor = wrapExecutorWithTracer(executor, tracer);
+
+function wrapExecutorWithTracer(
+ executor: Executor,
+ tracer?: Tracer,
+): Executor {
+ if (!tracer?.subgraphExecute) {
+ return executor;
+ }
+
+ return (executionRequest) => {
+ let result: MaybePromise<MaybeAsyncIterable<ExecutionResult<any, any>>>;
+
+ const traced$ = tracer.subgraphExecute(
+ { executionRequest },
+ async () => {
+ result = executor(executionRequest);
+ if (isPromise(result)) {
+ await result;
+ }
+ }
+ );
+
+ return isPromise(traced$)
+ ? traced$.then(() => result)
+ : result;
+ };
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (tracer?.subgraphExecute) { | |
| const originalExecutor = executor; | |
| executor = (executionRequest) => { | |
| let result: MaybePromise<MaybeAsyncIterable<ExecutionResult<any, any>>>; | |
| const traced$ = tracer.subgraphExecute({executionRequest}, () => { | |
| result = originalExecutor(executionRequest); | |
| return isPromise(result) ? result.then(() => undefined): undefined; | |
| }) | |
| return isPromise(traced$) ? traced$.then(() => result) : result!; | |
| }; | |
| } | |
| executor = wrapExecutorWithTracer(executor, tracer); | |
| function wrapExecutorWithTracer( | |
| executor: Executor, | |
| tracer?: Tracer, | |
| ): Executor { | |
| if (!tracer?.subgraphExecute) { | |
| return executor; | |
| } | |
| return (executionRequest) => { | |
| let result: MaybePromise<MaybeAsyncIterable<ExecutionResult<any, any>>>; | |
| const traced$ = tracer.subgraphExecute( | |
| { executionRequest }, | |
| async () => { | |
| result = executor(executionRequest); | |
| if (isPromise(result)) { | |
| await result; | |
| } | |
| } | |
| ); | |
| return isPromise(traced$) | |
| ? traced$.then(() => result) | |
| : result; | |
| }; | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 261-261: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
| .helix/config.toml | ||
| .helix/config.toml | ||
| .helix/languages.toml |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove duplicate entry for .helix/config.toml.
The file contains a duplicate entry for .helix/config.toml which should be removed to maintain a clean .gitignore file.
Apply this diff to remove the duplicate entry:
.helix/config.toml
-.helix/config.toml
.helix/languages.toml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .helix/config.toml | |
| .helix/config.toml | |
| .helix/languages.toml | |
| .helix/config.toml | |
| .helix/languages.toml |
| @@ -166,6 +166,7 @@ export function createGatewayRuntime< | |||
| let unifiedGraphPlugin: GatewayPlugin; | |||
|
|
|||
| const readinessCheckEndpoint = config.readinessCheckEndpoint || '/readiness'; | |||
| let tracer: GatewayPlugin['tracer']; | |||
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add a changeset file for the new Tracer API.
Since this PR adds a new feature to a package in the packages directory, please create a changeset file using yarn changeset. The changeset should:
- Use
minorfor semver (new feature) - Include a description of the Tracer API
- Provide an example of usage
🏁 Script executed:
#!/bin/bash
# Check for changeset file
fd -e md -p '.changeset/'Length of output: 25
Action Required: Add a changeset file for New Tracer API
It appears that no changeset file exists in the repository, yet this PR introduces a new API which qualifies as a new feature. Please create a changeset file using yarn changeset with the following details:
- Semver update:
minor - Description: A brief description of the new Tracer API functionality.
- Usage Example: An example demonstrating how to integrate or use the new Tracer API.
This file is required for proper version management and release notes for packages in the repository (specifically in packages/runtime/src/createGatewayRuntime.ts at line 169).
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
package.json (1)
1-84: 🧹 Nitpick (assertive)Missing Linear Issue Link & Documentation Update.
The PR description does not reference a Linear issue (e.g., containing "GW-*"). Additionally, since this PR introduces a new Tracer API, it likely requires corresponding documentation updates in the
consoleproject. Please add the appropriate Linear issue reference and consider creating a follow-up issue to update the documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: For all PRs, we would like to verify that a Linear iss...
**: For all PRs, we would like to verify that a Linear issue is linked. Check if there's a mention for "GW-*" in the PR description, and the us know when it's missing.In most cases, when a PR adds a new functionality, we would like to make sure it has documentation in the corresponding
consoleproject.
Use the global knowledge feature to search for PRs ingraphql-hive/consolerepository that adds documentation. Suggest to create a follow up issue to add the missing documentation.
package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
| "graphql-yoga": "5.13.0-alpha-20250221161011-2a41f6a3b66fd9850d356e6c07cf9392344bc1ed", | ||
| "@envelop/core": "5.1.0-alpha-20250221160025-4035aee544c5d6343c568fa53cc56398e047b852", | ||
| "@envelop/types": "5.1.0-alpha-20250221160025-4035aee544c5d6343c568fa53cc56398e047b852", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Verify new devDependencies for Tracer API enhancements.
The newly added devDependencies "graphql-yoga", "@envelop/core", and "@envelop/types" are intended to support the new tracing functionality. Please verify that the alpha versions in use meet the project’s stability requirements and don’t introduce conflicts with other dependencies.
No description provided.