GeezerVison is showing github.com — Proposal for an additional instrumentation hook point by tinnou · Pull Request #4206 · graphql-java/graphql-java
Conversation
tinnou
commented
Jan 12, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
CONTEXT
In our infrastructure, we like to capture resolver latency along with error presence on a single metric. This unlocks alerting based on resolver error rate & resolver latency. We also leverage centralized resolver exception handling (via DataFetchingExceptionHandler) and implementations often contains logic to transforms certain classes of exceptions as client or server GraphQL errors. For example a data fetcher might raise a ValidationException which will be caught by the ExceptionHandler and mapped as a client GraphQL error (bad request error type).
Unfortunately right now, the existing instrumentation hooks (e.g instrumentDataFetcher or beginFieldFetching/beginFieldFetch) are insufficient because it's impossible to observe the post-exception-handling DataFetcherResult. So in the example above, instrumentations would only have access to the ValidationException and not the graphql client error because exception handling has not happened yet. Emitting a metric that tracks the rate of client/server errors as well as latency becomes complicated without replicating the exception handling inside instrumentations.
WHAT IS THIS PR PROPOSING?
This PR exposes an additional method on default void onExceptionHandled(DataFetcherResult<Object> dataFetcherResult) on FieldFetchingInstrumentationContext to observe the final DataFetcherResult after exception handling. This is part of the beginFieldFetching instrumentation method callback.
Breaking Changes
- This is an additive, API backwards-compatible extension. Existing instrumentation remains valid. Instrumentation implementations that want these new capabilities can implement the new interface.
- the
onCompleted method is now called after exception handling, which is a minor change in behavior from the current implementation.
I put this PR together as a discussion starting point (using the Cunningham's Law), there are definitive alternatives that I thought of and would love your input.
Alternatives considered:
- Extend the existing
InstrumentationContext API with new callbacks:
- Pros: single context type.
- Cons: would modify the general-purpose context contract and could break or confuse clients; less explicit about field-fetching semantics.
- Rejected to preserve separation of concerns and minimize API surface changes.
- Add separate callbacks on
Instrumentation (new methods) rather than a context:
- Pros: offers a purely additive solution
- Cons: less flexible for per-field context state and harder to reuse existing
InstrumentationContext lifecycle semantics. Adding extra hooks could make the execution feel crowded.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…g after exception handling
|
@tinnou - long time no see. Let me have a look at this and thanks once again for contributing
|
tinnou reacted with heart emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one - lets add this!
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Nice one - lets add this!
That's some serious speed! ⚡ ⚡ Thank you Brad!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
tinnou
added a commit
to tinnou/graphql-java
that referenced
this pull request
Jan 13, 2026
tinnou
added a commit
to tinnou/graphql-java
that referenced
this pull request
Jan 14, 2026
tinnou
added a commit
to tinnou/graphql-java
that referenced
this pull request
Jan 14, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.