Merge pull request #73 from rmosolgo/yield-to-super

Fix calls to other traces

lib/graphql/metrics/trace.rb

@@ -15,58 +15,57 @@ def initialize(**_rest)
       # multiplexing multiple queries.
 
       # wraps everything below this line; only run once
-      def execute_multiplex(multiplex:, &block)
-        return yield if @skip_tracing
-        capture_multiplex_start_time(&block)
-
+      def execute_multiplex(multiplex:)
+        return super if @skip_tracing
+        capture_multiplex_start_time { super }
       end
 
       # may not trigger if the query is passed in pre-parsed
-      def lex(query_string:, &block)
-        return yield if @skip_tracing
-        capture_lexing_time(&block)
+      def lex(query_string:)
+        return super if @skip_tracing
+        capture_lexing_time { super }
       end
 
       # may not trigger if the query is passed in pre-parsed
-      def parse(query_string:, &block)
-        return yield if @skip_tracing
-        capture_parsing_time(&block)
+      def parse(query_string:)
+        return super if @skip_tracing
+        capture_parsing_time { super }
       end
 
-      def validate(query:, validate:, &block)
-        return yield if @skip_tracing
-        capture_validation_time(query.context, &block)
+      def validate(query:, validate:)
+        return super if @skip_tracing
+        capture_validation_time(query.context) { super }
       end
 
       # wraps all `analyze_query`s; only run once
-      def analyze_multiplex(multiplex:, &block)
-        return yield if @skip_tracing
+      def analyze_multiplex(multiplex:)
+        return super if @skip_tracing
         # Ensures that we reset potentially long-lived PreContext objects between multiplexs. We reset at this point
         # since all parsing and validation will be done by this point, and a GraphQL::Query::Context will exist.
         pre_context.reset
-        yield
+        super
       end
 
-      def analyze_query(query:, &block)
-        return yield if @skip_tracing
-        capture_analysis_time(query.context, &block)
+      def analyze_query(query:)
+        return super if @skip_tracing
+        capture_analysis_time(query.context) { super }
       end
 
-      def execute_query(query:, &block)
-        return yield if @skip_tracing
-        capture_query_start_time(query.context, &block)
+      def execute_query(query:)
+        return super if @skip_tracing
+        capture_query_start_time(query.context) { super }
       end
 
-      def execute_field(field:, query:, ast_node:, arguments:, object:, &block)
-        return yield if @skip_tracing || query.context[SKIP_FIELD_AND_ARGUMENT_METRICS]
-        return yield unless GraphQL::Metrics.timings_capture_enabled?(query.context)
-        trace_field(GraphQL::Metrics::INLINE_FIELD_TIMINGS, query, &block)
+      def execute_field(field:, query:, ast_node:, arguments:, object:)
+        return super if @skip_tracing || query.context[SKIP_FIELD_AND_ARGUMENT_METRICS]
+        return super unless GraphQL::Metrics.timings_capture_enabled?(query.context)
+        trace_field(GraphQL::Metrics::INLINE_FIELD_TIMINGS, query) { super }
       end
 
-      def execute_field_lazy(field:, query:, ast_node:, arguments:, object:, &block)
-        return yield if @skip_tracing || query.context[SKIP_FIELD_AND_ARGUMENT_METRICS]
-        return yield unless GraphQL::Metrics.timings_capture_enabled?(query.context)
-        trace_field(GraphQL::Metrics::LAZY_FIELD_TIMINGS, query, &block)
+      def execute_field_lazy(field:, query:, ast_node:, arguments:, object:)
+        return super if @skip_tracing || query.context[SKIP_FIELD_AND_ARGUMENT_METRICS]
+        return super unless GraphQL::Metrics.timings_capture_enabled?(query.context)
+        trace_field(GraphQL::Metrics::LAZY_FIELD_TIMINGS, query) { super }
       end
 
       private

test/unit/graphql/metrics/trace_integration_test.rb

@@ -76,6 +76,13 @@ def store_metrics(context_key, metrics)
           end
         end
 
+        module TestTrace
+          def execute_multiplex(multiplex:)
+            multiplex.context[:test_trace_was_executed] = true
+            super
+          end
+        end
+
         class SchemaWithFullMetrics < GraphQL::Schema
           query QueryRoot
           mutation MutationRoot
@@ -85,6 +92,7 @@ class SchemaWithFullMetrics < GraphQL::Schema
 
           instrument :query, GraphQL::Metrics::Instrumentation.new
           query_analyzer SimpleAnalyzer
+          trace_with TestTrace
           trace_with GraphQL::Metrics::Trace
 
           def self.parse_error(err, _context)
@@ -93,6 +101,19 @@ def self.parse_error(err, _context)
           end
         end
 
+        test "it doesn't short-circuit other traces" do
+          context = {}
+          query = GraphQL::Query.new(
+            SchemaWithFullMetrics,
+            kitchen_sink_query_document,
+            variables: { 'postId': '1', 'titleUpcase': true },
+            operation_name: 'PostDetails',
+            context: context
+          )
+          result = query.result.to_h
+          assert query.multiplex.context[:test_trace_was_executed]
+        end
+
         test 'extracts metrics from queries, as well as their fields and arguments (when using Query#result)' do
           context = {}
           query = GraphQL::Query.new(