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

DataFetcher observations have incorrect hierarchy #676

Closed
bclozel opened this issue Apr 24, 2023 · 2 comments
Closed

DataFetcher observations have incorrect hierarchy #676

bclozel opened this issue Apr 24, 2023 · 2 comments
Assignees
Labels
in: core Issues related to config and core support type: bug A general bug
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Apr 24, 2023

When recording data fetcher observations, the GraphQlObservationInstrumentation organizes the parent/child relationship between observations by setting the current observation under a well-known key in the global GraphQL context. In some cases, the order of execution and the scheduling of operations does not reflect the actual operation hierarchy as defined by the GraphQL ExecutionStepInfo.

This results in traces where data fetching operations are set with incorrect parent/child relationships. We should instead use the ExecutionStepInfo to ensure that the correct observation information is used.

Thanks @tkmax83 for providing a sample application.

@bclozel bclozel added type: bug A general bug in: core Issues related to config and core support labels Apr 24, 2023
@bclozel bclozel added this to the 1.1.4 milestone Apr 24, 2023
@bclozel bclozel self-assigned this Apr 24, 2023
@bclozel
Copy link
Member Author

bclozel commented May 10, 2023

Reopening as the fix is incomplete.
For DataFetcher observations, we cannot use the global GraphQLContext as it shared for the entire GraphQL request (and no thread safe). The fix above relies on an externally maintained map tracking observations and their execution step info. This fix is incomplete, as the DataFetcher observation will not be set as current anywhere, and DataFetcher implementations will not see it as current in the ThreadLocal or reactive context.

In this case, we should probably use the graphql.schema.DataFetchingEnvironment#getLocalContext to provide context values that are local to field fetching operations. For this fix to be complete, we need the ContextDataFetcherDecorator to capture values from both the global and local GraphQLContext, see #688.

@bclozel
Copy link
Member Author

bclozel commented May 11, 2023

Reopening as the instrumentation breaks the data fetchers that return CompletableFuture. See #692.

@bclozel bclozel reopened this May 11, 2023
bclozel added a commit that referenced this issue Feb 13, 2024
Prior to this commit, gh-676 fixed the local and global context
inheritance in the `ContextDataFetcherDecorator` with a workaround.
This commit undoes this workaround and uses the new Context Propagation
API for this, effectively raising the requirement for version 1.0.3 and
removing all deprecated APIs in the meantime.

Closes gh-688
bclozel added a commit to bclozel/spring-graphql that referenced this issue Feb 14, 2024
Prior to this commit, spring-projectsgh-676 fixed the local and global context
inheritance in the `ContextDataFetcherDecorator` with a workaround.
This commit undoes this workaround and uses the new Context Propagation
API for this, effectively raising the requirement for version 1.0.3 and
removing all deprecated APIs in the meantime.

Closes spring-projectsgh-688
bclozel added a commit that referenced this issue Feb 14, 2024
Prior to this commit, gh-676 fixed the local and global context
inheritance in the `ContextDataFetcherDecorator` with a workaround.
This commit undoes this workaround and uses the new Context Propagation
API for this, effectively raising the requirement for version 1.0.3 and
removing all deprecated APIs in the meantime.

Closes gh-688
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues related to config and core support type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant