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

Request observation has no parent observation #675

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

Request observation has no parent observation #675

bclozel opened this issue Apr 24, 2023 · 3 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

With recent versions of Spring for GraphQL and Micrometer, it seems that a regression has been introduced where GraphQL request observations have no parent observation set, whereas the server http observation would be expected.

We initially introduced the GraphQL request observation and its ExecutionRequestObservationContext as RequestReplyReceiverContext. This type of context is meant to be used in cases we need to propagate incoming tracing information (at the transport level) to the observation being created. This is a required step for observation correlation across network boundaries. In the case of the HTTP transport, our implementation would rely on a custom PropagationWebGraphQlInterceptor that extracts tracing information from request headers and copy them to the GraphQL context. As a fallback, would rely on the fact that any current observation would be set as such in the GraphQL context. While this approach worked in the past, it seems that recent changes/fixes broke it.

After reviewing the request instrumentation, I came to the following conclusion:

  • the GraphQL request execution is transport agnostic and should not deal with propagation; this means that the observation context should be a simple Observation.Context
  • some transports might not support tracing information on a per execution input basis. For example, the WebSocket transport does not support sending headers for each message
  • it is the responsibility of the underlying transport to be instrumented; if it is, the current observation should be set as such in the GraphQL context so that it can be used as parent by the main observation. If the underlying transport is not instrumented, tracing propagation will not apply and the request observation will have no parent.

As a result, we should update ExecutionRequestObservationContext and deprecate PropagationWebGraphQlInterceptor as they should not be used anymore as of today.

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
@rsercano
Copy link

Looking forward to 3.0.7 release since this affects us too.

@bclozel
Copy link
Member Author

bclozel commented May 20, 2023

It is out already: https://spring.io/blog/2023/05/18/spring-boot-3-0-7-available-now

@rsercano
Copy link

Nice thx!

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

2 participants