Conversation
…phQL execution tracing - Integrated `@pothos/plugin-tracing` and `@pothos/tracing-opentelemetry` into the GraphQL API. - Updated package dependencies in `package.json` and `pnpm-lock.yaml`. - Implemented tracing in the GraphQL execution flow and added active span handling in resolvers. - Enhanced configuration for tracing options based on environment variables. - Adjusted test timeout in `config.singleton.test.ts` for improved stability.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenTelemetry-based tracing across the ensapi GraphQL builder and resolvers, refines async span helper typings, adds Pothos tracing dependencies, adjusts a test timeout, renames a Yoga logger, and makes integration tests bail on first failure. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Yoga as Yoga (GraphQL Server)
participant Builder as Schema Builder
participant Tracer as OpenTelemetry Tracer
participant Resolver as Resolver
participant DB as Database / Dataloader
Client->>Yoga: send GraphQL request
activate Yoga
Yoga->>Builder: execute operation
activate Builder
Builder->>Tracer: start field span
activate Tracer
Builder->>Resolver: invoke resolver
activate Resolver
Resolver->>Tracer: start child span (e.g., connection / dataloader)
Resolver->>DB: execute DB query or dataloader.load
activate DB
DB-->>Resolver: return results
deactivate DB
Resolver->>Tracer: end child span (set attributes)
Resolver-->>Builder: return resolved value
deactivate Resolver
Builder->>Tracer: end field span
deactivate Tracer
Builder-->>Yoga: response
deactivate Builder
Yoga-->>Client: GraphQL response
deactivate Yoga
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds GraphQL tracing instrumentation to ENS API using Pothos tracing plugins and custom Yoga execution spans, with env-driven toggles.
Changes:
- Add
@pothos/plugin-tracingand@pothos/tracing-opentelemetrydependencies. - Enable Pothos field-level tracing (conditionally) and add a Yoga execution tracing plugin (conditionally).
- Add explicit span wrappers around
findDomainsresolver DB/query/dataloader steps; increase a test timeout for stability.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
apps/ensapi/package.json |
Adds Pothos tracing dependencies. |
pnpm-lock.yaml |
Locks new Pothos tracing packages and their peer deps. |
apps/ensapi/src/graphql-api/builder.ts |
Conditionally enables Pothos TracingPlugin + OTel wrapper. |
apps/ensapi/src/graphql-api/yoga.ts |
Adds Yoga execution-level tracing span with optional source capture. |
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts |
Wraps key resolver steps in spans for better visibility. |
apps/ensapi/src/lib/instrumentation/auto-span.ts |
Fixes async span helper generic typing; minor touched-area comment. |
apps/ensapi/src/config/config.singleton.test.ts |
Increases test timeout to reduce flakiness. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/ensapi/src/lib/instrumentation/auto-span.ts:21
- Spelling: "provded" -> "provided" in the comment (helps keep inline docs clear).
return tracer.startActiveSpan(name, async (span) => {
// add provded args to span attributes
for (const [key, value] of Object.entries(args)) {
span.setAttribute(key, value);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/graphql-api/yoga.ts`:
- Around line 19-33: The code currently allows capturing full GraphQL source
into spans when graphqlTracingIncludeSource is true, which risks leaking
sensitive literals in production; update the logic so AttributeNames.SOURCE is
only included when graphqlTracingIncludeSource is true AND the process is not
running in production (e.g., process.env.NODE_ENV !== "production" or an
equivalent guard), then use that guarded flag inside executionTracingPlugin
where graphqlTracer.startActiveSpan is called (references:
graphqlTracingIncludeSource, executionTracingPlugin,
graphqlTracer.startActiveSpan, SpanNames.EXECUTE, AttributeNames.SOURCE) so
source capture cannot be enabled in prod by env drift/misconfiguration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29d83590-0567-404d-839a-7560a038331b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/ensapi/package.jsonapps/ensapi/src/config/config.singleton.test.tsapps/ensapi/src/graphql-api/builder.tsapps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.tsapps/ensapi/src/graphql-api/yoga.tsapps/ensapi/src/lib/instrumentation/auto-span.ts
Greptile SummaryThis PR adds OpenTelemetry tracing to the ENSApi GraphQL layer: Pothos resolver-level spans via Key findings:
Confidence Score: 4/5Safe to merge after fixing the missing span status in the error handler; the main application path is unaffected since tracing is opt-in. One P1 defect: the
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Yoga as GraphQL Yoga
participant ExecPlugin as executionTracingPlugin
participant OTel as OTel Tracer
participant Pothos as Pothos Resolvers
participant Resolver as find-domains-resolver
participant DB as Database / Dataloader
Client->>Yoga: HTTP POST /graphql
Yoga->>ExecPlugin: onExecute (if ENSAPI_GRAPHQL_TRACING=1)
ExecPlugin->>OTel: startActiveSpan("graphql.execute")
ExecPlugin->>Pothos: executeFn(options)
Pothos->>OTel: createSpan per resolver (via TracingPlugin)
Pothos->>Resolver: resolveFindDomains()
Resolver->>OTel: startActiveSpan("find-domains.totalCount")
OTel-->>Resolver: span
Resolver->>DB: SELECT COUNT(*)
DB-->>Resolver: count
Resolver->>OTel: span.end()
Resolver->>OTel: startActiveSpan("find-domains.connection")
OTel-->>Resolver: span
Resolver->>DB: SELECT paginated
DB-->>Resolver: rows
Resolver->>OTel: span.end()
Resolver->>OTel: startActiveSpan("find-domains.dataloader-load")
OTel-->>Resolver: span
Resolver->>DB: dataloader.loadMany(ids)
DB-->>Resolver: Domain entities
Resolver->>OTel: span.end()
Resolver-->>Pothos: DomainWithOrderValue[]
Pothos-->>ExecPlugin: ExecutionResult
ExecPlugin->>OTel: span.end() (graphql.execute)
ExecPlugin-->>Yoga: ExecutionResult
Yoga-->>Client: HTTP 200
Reviews (1): Last reviewed commit: "feat(tracer): add initial GraphQL tracer..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensapi/src/omnigraph-api/schema/domain.ts (1)
79-97: 🧹 Nitpick | 🔵 TrivialInconsistent tracing:
DomainInterfaceRef.loadis not instrumented.The
ENSv1DomainRefandENSv2DomainRefloaders are wrapped withwithSpanAsync, butDomainInterfaceRef.loadperforms similar database queries without tracing. For consistent observability, consider wrapping this loader as well.♻️ Suggested fix
export const DomainInterfaceRef = builder.loadableInterfaceRef("Domain", { - load: async (ids: DomainId[]): Promise<(ENSv1Domain | ENSv2Domain)[]> => { - const [v1Domains, v2Domains] = await Promise.all([ - ensDb.query.v1Domain.findMany({ - where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv1DomainId - with: { label: true }, - }), - ensDb.query.v2Domain.findMany({ - where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv2DomainId - with: { label: true }, - }), - ]); - - return [...v1Domains, ...v2Domains]; - }, + load: (ids: DomainId[]): Promise<(ENSv1Domain | ENSv2Domain)[]> => + withSpanAsync(tracer, "Domain.load", { count: ids.length }, async () => { + const [v1Domains, v2Domains] = await Promise.all([ + ensDb.query.v1Domain.findMany({ + where: (t, { inArray }) => inArray(t.id, ids as any), + with: { label: true }, + }), + ensDb.query.v2Domain.findMany({ + where: (t, { inArray }) => inArray(t.id, ids as any), + with: { label: true }, + }), + ]); + + return [...v1Domains, ...v2Domains]; + }), toKey: getModelId,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/omnigraph-api/schema/domain.ts` around lines 79 - 97, The DomainInterfaceRef.load implementation lacks tracing; wrap its async load function with withSpanAsync (the same helper used for ENSv1DomainRef and ENSv2DomainRef) so the database queries (ensDb.query.v1Domain.findMany and ensDb.query.v2Domain.findMany) execute inside a span; give the span a clear name like "DomainInterfaceRef.load" and include relevant attributes (e.g., ids length or model ids via getModelId), ensure errors continue to propagate and preserve existing options (toKey: getModelId, cacheResolved, sort).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/omnigraph-api/builder.ts`:
- Around line 27-45: The span naming in createSpan's onSpan callback assumes
info.returnType has a .name and that parent.node.id exists; update onSpan to
defensively resolve the named type (use GraphQL's getNamedType on
info.returnType) to obtain typename only if present, and check that parent and
parent.node and parent.node.id are defined before using them; if either value is
missing fall back to a safe name (e.g., info.parentType.name or
`${info.parentType.name}.${info.fieldName}`) so no unsafe (info.returnType as
any).name or (parent as any).node.id assertions can throw at runtime.
In `@apps/ensapi/src/omnigraph-api/lib/get-domain-by-interpreted-name.ts`:
- Around line 74-82: The inner spans started via withActiveSpanAsync (tracer,
"v1_getDomainId", {}, ...) and the conditional v2 span ("v2_getDomainId", {},
...) lack useful attributes; update both calls to pass an attributes object
including the lookup name (e.g., { name }) so the span for v1_getDomainId and
v2_getDomainId contains the interpreted domain name being queried (affecting the
calls that invoke v1_getDomainIdByInterpretedName and
v2_getDomainIdByInterpretedName).
In `@apps/ensapi/src/omnigraph-api/yoga.ts`:
- Around line 43-48: The plugins array in yoga.ts currently leaves
maxTokensPlugin, maxDepthPlugin, and maxAliasesPlugin commented out; enable and
configure these plugins (maxTokensPlugin, maxDepthPlugin, maxAliasesPlugin) for
production to protect against malicious GraphQL queries by uncommenting or
wiring them into the plugins array and supplying appropriate configuration
values (e.g., maxOperationTokens, maxOperationDepth, maxOperationAliases) or
feature flags, and ensure any environment-based toggles or allowlists are
implemented so the plugins can be turned on in production while remaining off
during local/tracing runs.
---
Outside diff comments:
In `@apps/ensapi/src/omnigraph-api/schema/domain.ts`:
- Around line 79-97: The DomainInterfaceRef.load implementation lacks tracing;
wrap its async load function with withSpanAsync (the same helper used for
ENSv1DomainRef and ENSv2DomainRef) so the database queries
(ensDb.query.v1Domain.findMany and ensDb.query.v2Domain.findMany) execute inside
a span; give the span a clear name like "DomainInterfaceRef.load" and include
relevant attributes (e.g., ids length or model ids via getModelId), ensure
errors continue to propagate and preserve existing options (toKey: getModelId,
cacheResolved, sort).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b950a43d-a6b2-4cb5-b6ee-74d67e2ed3a0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
apps/ensadmin/src/app/@breadcrumbs/(apis)/api/omnigraph/page.tsxapps/ensapi/package.jsonapps/ensapi/src/omnigraph-api/builder.tsapps/ensapi/src/omnigraph-api/lib/find-domains/find-domains-resolver.tsapps/ensapi/src/omnigraph-api/lib/get-domain-by-interpreted-name.tsapps/ensapi/src/omnigraph-api/schema/domain.tsapps/ensapi/src/omnigraph-api/yoga.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
packages/integration-test-env/src/orchestrator.ts:344
- When running a pnpm script, additional CLI args are typically forwarded to the underlying command only after a
--delimiter. As written,--bail 1may be parsed by pnpm itself instead of being passed through to vitest, causing the orchestrator to fail. Consider invokingpnpm run test:integration -- --bail 1(or inserting"--"before--bail).
execaSync("pnpm", ["test:integration", "--bail", "1"], {
cwd: MONOREPO_ROOT,
stdio: "inherit",
env: {
ENSNODE_URL: `http://localhost:${ENSAPI_PORT}`,
},
});
apps/ensapi/src/lib/instrumentation/auto-span.ts:21
- Typo in comment: "provded" should be "provided".
// add provded args to span attributes
for (const [key, value] of Object.entries(args)) {
span.setAttribute(key, value);
apps/ensapi/src/lib/instrumentation/auto-span.ts:85
- Typo in comment: "provded" should be "provided".
// add provded args to span attributes
for (const [key, value] of Object.entries(args)) {
span.setAttribute(key, value);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ensapi/src/omnigraph-api/schema/domain.ts (1)
79-97: 🧹 Nitpick | 🔵 TrivialMissing tracing on
DomainInterfaceRef.loadfor consistency.
ENSv1DomainRefandENSv2DomainRefloaders are wrapped withwithSpanAsync, butDomainInterfaceRef.loadperforms similar database queries without tracing. This creates an observability gap when domains are loaded through the interface.♻️ Suggested fix to add tracing
export const DomainInterfaceRef = builder.loadableInterfaceRef("Domain", { load: async (ids: DomainId[]): Promise<(ENSv1Domain | ENSv2Domain)[]> => { - const [v1Domains, v2Domains] = await Promise.all([ - ensDb.query.v1Domain.findMany({ - where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv1DomainId - with: { label: true }, - }), - ensDb.query.v2Domain.findMany({ - where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv2DomainId - with: { label: true }, - }), - ]); + const [v1Domains, v2Domains] = await withSpanAsync( + tracer, + "Domain.load", + { count: ids.length }, + () => + Promise.all([ + ensDb.query.v1Domain.findMany({ + where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv1DomainId + with: { label: true }, + }), + ensDb.query.v2Domain.findMany({ + where: (t, { inArray }) => inArray(t.id, ids as any), // ignore downcast to ENSv2DomainId + with: { label: true }, + }), + ]), + ); return [...v1Domains, ...v2Domains]; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/omnigraph-api/schema/domain.ts` around lines 79 - 97, DomainInterfaceRef.load is performing DB queries without tracing; wrap the load implementation in the same withSpanAsync tracing helper used by ENSv1DomainRef and ENSv2DomainRef so the queries are instrumented. Specifically, change the load function passed to builder.loadableInterfaceRef("Domain") to call withSpanAsync (e.g., withSpanAsync("DomainInterfaceRef.load", async () => { ... })) around the Promise.all block, preserve returning [...v1Domains, ...v2Domains], keep toKey: getModelId, cacheResolved and sort settings, and ensure TypeScript types for the async callback match Promise<(ENSv1Domain|ENSv2Domain)[]> so tracing doesn’t alter the return type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/omnigraph-api/builder.ts`:
- Around line 44-49: The span naming uses getNamedType(info.returnType) without
guarding for undefined, which can produce "undefined(id)"; update the logic
around getNamedType(info.returnType) in the branch where info.parentType.name
endsWith("Edge") and info.fieldName === "node" to read the type name defensively
(e.g., const named = getNamedType(info.returnType); const typename = named?.name
?? "UnknownType") and then call span.updateName(`${typename}(${id})`) so a
sensible fallback is used when getNamedType returns undefined.
---
Outside diff comments:
In `@apps/ensapi/src/omnigraph-api/schema/domain.ts`:
- Around line 79-97: DomainInterfaceRef.load is performing DB queries without
tracing; wrap the load implementation in the same withSpanAsync tracing helper
used by ENSv1DomainRef and ENSv2DomainRef so the queries are instrumented.
Specifically, change the load function passed to
builder.loadableInterfaceRef("Domain") to call withSpanAsync (e.g.,
withSpanAsync("DomainInterfaceRef.load", async () => { ... })) around the
Promise.all block, preserve returning [...v1Domains, ...v2Domains], keep toKey:
getModelId, cacheResolved and sort settings, and ensure TypeScript types for the
async callback match Promise<(ENSv1Domain|ENSv2Domain)[]> so tracing doesn’t
alter the return type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b8eb342-60ff-4e2e-bcfe-2b90959ea980
📒 Files selected for processing (3)
apps/ensapi/src/omnigraph-api/builder.tsapps/ensapi/src/omnigraph-api/schema/domain.tspackages/integration-test-env/src/orchestrator.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/ensapi/src/lib/instrumentation/auto-span.ts:21
- The comment says "add provded args" (typo). Consider correcting to "add provided args" (appears in multiple helpers in this file).
// add provded args to span attributes
for (const [key, value] of Object.entries(args)) {
span.setAttribute(key, value);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lite PR
Summary
graphql.executespan).find-domains.*manual spans forCOUNT, paginatedSELECT, and dataloader load.Why
Testing
test:integrationand inspect tracesNotes for Reviewer (Optional)
apps/ensapi/src/graphql-api/lib/find-domains/find-domains-resolver.ts.OTEL_EXPORTER_OTLP_ENDPOINT: set to enable exporting spans (otherwise spans are not exported).OTEL_DEBUG: set to enable OTel diagnostic loggingPre-Review Checklist (Blocking)