- Notifications
You must be signed in to change notification settings - Fork 324
MetricsInstrumentation does not implement updated API of TracingInstrumentation provided by graphql-java #950
Description
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:
- In a graphql spring boot project, set
graphql.servlet.tracing-enabledtometrics-onlyandgraphql.servlet.actuator-metricstotrue. - Run server and submit a query of any complexity/simplicity.
- Notice that
extensionsis populated withtracinginformation. - Submit a request to the server's JMX metrics endpoint.
- 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