Skip to content
This repository was archived by the owner on Dec 19, 2023. It is now read-only.

MetricsInstrumentation does not implement updated API of TracingInstrumentation provided by graphql-java #950

Closed
bsara opened this issue Jun 7, 2023 · 0 comments · Fixed by #951
Labels

Comments

@bsara
Copy link

bsara commented Jun 7, 2023

Describe the bug
TracingInstrumentation#instrumentExecutionResult is being executed twice per query and MetricsInstrumentation#instrumentExecutionResult is never called because it overrides a deprecated method that is no longer called by graphql.execution.instrumentation.ChainedInstrumentation. This results in tracing metrics always being included in the extensions of results even when graphql.servlet.tracing-enabled is set to metrics-only and actuator metrics are never recorded.

To Reproduce
Steps to reproduce the behavior:

  1. In a graphql spring boot project, set graphql.servlet.tracing-enabled to metrics-only and graphql.servlet.actuator-metrics to true.
  2. Run server and submit a query of any complexity/simplicity.
  3. Notice that extensions is populated with tracing information.
  4. Submit a request to the server's JMX metrics endpoint.
  5. Notice that no graphql metrics are found.

Expected behavior
When graphql.servlet.tracing-enabled is set to metrics-only and graphql.servlet.actuator-metrics is set to true. The response should exclude tracing from extensions and graphql metrics should be present in the response of a request to obtain the server's JMX metrics.

Desktop (please complete the following information):

  • OS: macOS Ventura 13.4 (M1 processor)
  • Browser N/A
  • Version 14.1.0
  • Client: Insomnia 2023.2.2

Additional context
The API seems to have been deprecated and using a different method for some time now. Perhaps I have something wrong with my implementation that is making me see this issue and not others?

Here is the MetricsInstrumentation method in question: https://github.com/graphql-java-kickstart/graphql-spring-boot/blob/master/graphql-spring-boot-autoconfigure/src/main/java/graphql/kickstart/autoconfigure/web/servlet/metrics/MetricsInstrumentation.java#L40-L41

Here is the graphql-java method that MetricsInstrumentation is actually overriding: https://github.com/graphql-java/graphql-java/blob/v20.3/src/main/java/graphql/execution/instrumentation/SimplePerformantInstrumentation.java#L190-L192

Here is the graphql-java method that should be overridden: https://github.com/graphql-java/graphql-java/blob/v20.3/src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java#L80-L88

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
1 participant