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

Deprecate Schema.tracer (use .trace_with instead) #4878

Merged
merged 3 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/tracing/platform_tracing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 28 additions & 3 deletions spec/graphql/language/parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,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
Expand Down
21 changes: 1 addition & 20 deletions spec/graphql/schema_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion spec/graphql/tracing/legacy_trace_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion spec/graphql/tracing/notifications_tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/graphql/tracing/trace_modes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down