Merge pull request #4878 from rmosolgo/deprecate-tracer

Deprecate Schema.tracer (use .trace_with instead)

Author: rmosolgo

lib/graphql/schema.rb

@@ -1129,7 +1129,11 @@ def default_directives
         }.freeze
       end
 
-      def tracer(new_tracer)
+      def tracer(new_tracer, silence_deprecation_warning: false)
+        if !silence_deprecation_warning
+          warn("`Schema.tracer(#{new_tracer.inspect})` is deprecated; use module-based `trace_with` instead. See: https://graphql-ruby.org/queries/tracing.html")
+          warn "  #{caller(1, 1).first}"
+        end
         default_trace = trace_class_for(:default, build: true)
         if default_trace.nil? || !(default_trace < GraphQL::Tracing::CallLegacyTracers)
           trace_with(GraphQL::Tracing::CallLegacyTracers)

lib/graphql/tracing/platform_tracing.rb

@@ -86,7 +86,7 @@ def self.use(schema_defn, options = {})
           else
             warn("`use(#{self.name})` and `Tracing::PlatformTracing` are deprecated. Use a `trace_with(...)` module instead. More info: https://graphql-ruby.org/queries/tracing.html. Please open an issue on the GraphQL-Ruby repo if you want to discuss further!")
             tracer = self.new(**options)
-            schema_defn.tracer(tracer)
+          schema_defn.tracer(tracer, silence_deprecation_warning: true)
           end
         end
       end

spec/graphql/language/parser_spec.rb

@@ -413,14 +413,39 @@
     end
   end
 
+  module ParserTrace
+    TRACES = []
+    def parse(query_string:)
+      TRACES << (trace = { key: "parse", query_string: query_string })
+      result = super
+      trace[:result] = result
+      result
+    end
+
+    def lex(query_string:)
+      TRACES << (trace = { key: "lex", query_string: query_string })
+      result = super
+      trace[:result] = result
+      result
+    end
+
+    def self.clear
+      TRACES.clear
+    end
+
+    def self.traces
+      TRACES
+    end
+  end
+
   it "serves traces" do
-    TestTracing.clear
+    ParserTrace.clear
     schema = Class.new(GraphQL::Schema) do
-      tracer(TestTracing)
+      trace_with(ParserTrace)
     end
     query = GraphQL::Query.new(schema, "{ t: __typename }")
     subject.parse("{ t: __typename }", trace: query.current_trace)
-    traces = TestTracing.traces
+    traces = ParserTrace.traces
     expected_traces = if USING_C_PARSER
       2
     else

spec/graphql/schema_spec.rb

@@ -44,7 +44,6 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions
         cursor_encoder Object.new
         context_class Class.new
         directives [DummyFeature1]
-        tracer GraphQL::Tracing::DataDogTracing
         extra_types ExtraType
         query_analyzer Object.new
         multiplex_analyzer Object.new
@@ -69,7 +68,6 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions
       assert_equal base_schema.orphan_types, schema.orphan_types
       assert_equal base_schema.context_class, schema.context_class
       assert_equal base_schema.directives, schema.directives
-      assert_equal base_schema.tracers, schema.tracers
       assert_equal base_schema.query_analyzers, schema.query_analyzers
       assert_equal base_schema.multiplex_analyzers, schema.multiplex_analyzers
       assert_equal base_schema.disable_introspection_entry_points?, schema.disable_introspection_entry_points?
@@ -123,7 +121,6 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions
       multiplex_analyzer = Object.new
       schema.multiplex_analyzer(multiplex_analyzer)
       schema.rescue_from(GraphQL::ExecutionError)
-      schema.tracer(GraphQL::Tracing::NewRelicTracing)
 
       assert_equal query, schema.query
       assert_equal mutation, schema.mutation
@@ -142,10 +139,6 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions
       assert_equal base_schema.query_analyzers + [query_analyzer], schema.query_analyzers
       assert_equal base_schema.multiplex_analyzers + [multiplex_analyzer], schema.multiplex_analyzers
       assert_equal [GraphQL::Backtrace, GraphQL::Subscriptions::ActionCableSubscriptions, CustomSubscriptions], schema.plugins.map(&:first)
-      assert_equal [GraphQL::Tracing::DataDogTracing], base_schema.tracers
-      assert_includes base_schema.new_trace.class.ancestors, GraphQL::Tracing::CallLegacyTracers
-      assert_equal [GraphQL::Tracing::DataDogTracing, GraphQL::Tracing::NewRelicTracing], schema.tracers
-      assert_includes schema.new_trace.class.ancestors, GraphQL::Tracing::CallLegacyTracers
       assert_equal custom_query_class, schema.query_class
       assert_equal [ExtraType, extra_type_2], schema.extra_types
       assert_instance_of CustomSubscriptions, schema.subscriptions
@@ -217,16 +210,7 @@ def self.reset_calls
     end
   end
 
-  describe "`use` works with plugins that attach instrumentation, tracers, query analyzers" do
-    class NoOpTracer
-      def trace(_key, data)
-        if (query = data[:query])
-          query.context[:no_op_tracer_ran] = true
-        end
-        yield
-      end
-    end
-
+  describe "`use` works with plugins that attach instrumentation, trace modules, query analyzers" do
     module NoOpTrace
       def execute_query(query:)
         query.context[:no_op_trace_ran_before_query] = true
@@ -254,7 +238,6 @@ def result
     module PluginWithInstrumentationTracingAndAnalyzer
       def self.use(schema_defn)
         schema_defn.trace_with(NoOpTrace)
-        schema_defn.tracer NoOpTracer.new
         schema_defn.query_analyzer NoOpAnalyzer
       end
     end
@@ -281,7 +264,6 @@ def foobar; 1337; end
 
         assert_equal true, query.context[:no_op_trace_ran_before_query]
         assert_equal true, query.context[:no_op_trace_ran_after_query]
-        assert_equal true, query.context[:no_op_tracer_ran]
         assert_equal true, query.context[:no_op_analyzer_ran_initialize]
         assert_equal true, query.context[:no_op_analyzer_ran_on_leave_field]
         assert_equal true, query.context[:no_op_analyzer_ran_result]
@@ -308,7 +290,6 @@ def foobar; 1337; end
 
         assert_equal true, query.context[:no_op_trace_ran_before_query]
         assert_equal true, query.context[:no_op_trace_ran_after_query]
-        assert_equal true, query.context[:no_op_tracer_ran]
         assert_equal true, query.context[:no_op_analyzer_ran_initialize]
         assert_equal true, query.context[:no_op_analyzer_ran_on_leave_field]
         assert_equal true, query.context[:no_op_analyzer_ran_result]

spec/graphql/tracing/legacy_trace_spec.rb

@@ -22,7 +22,7 @@ def int
 
     parent_schema = Class.new(GraphQL::Schema) do
       query(query_type)
-      tracer(custom_tracer)
+      tracer(custom_tracer, silence_deprecation_warning: true)
     end
 
     child_schema = Class.new(parent_schema)

spec/graphql/tracing/notifications_tracing_spec.rb

@@ -40,7 +40,7 @@ def trigger_fake_notifications_tracer(schema)
     end
 
     tracer = GraphQL::Tracing::NotificationsTracing.new(engine)
-    schema.tracer(tracer)
+    schema.tracer(tracer, silence_deprecation_warning: true)
     schema.execute "query X { int }"
 
     dispatched_events

spec/graphql/tracing/trace_modes_spec.rb

@@ -137,7 +137,7 @@ def execute_query(query:)
       tracer_class = Class.new
 
       # add a legacy tracer
-      GraphQL::Schema.tracer(tracer_class)
+      GraphQL::Schema.tracer(tracer_class, silence_deprecation_warning: true)
       # A newly created child class gets the right setup:
       new_child_class = Class.new(GraphQL::Schema)
       assert_includes new_child_class.trace_class_for(:default).ancestors, GraphQL::Tracing::CallLegacyTracers