Merge pull request rmosolgo#4853 from porter77/master

Add support for transaction names in Sentry tracing

Author: rmosolgo

lib/graphql/tracing/sentry_trace.rb

@@ -5,14 +5,31 @@ module Tracing
     module SentryTrace
       include PlatformTrace
 
+      # @param set_transaction_name [Boolean] If true, the GraphQL operation name will be used as the transaction name.
+      #   This is not advised if you run more than one query per HTTP request, for example, with `graphql-client` or multiplexing.
+      #   It can also be specified per-query with `context[:set_sentry_transaction_name]`.
+      def initialize(set_transaction_name: false, **_rest)
+        @set_transaction_name = set_transaction_name
+        super
+      end
+      
+      def execute_query(**data)
+        set_this_txn_name = data[:query].context[:set_sentry_transaction_name]
+        if set_this_txn_name == true || (set_this_txn_name.nil? && @set_transaction_name)
+          Sentry.configure_scope do |scope|
+            scope.set_transaction_name(transaction_name(data[:query]))
+          end
+        end
+        instrument_execution("graphql.execute", "execute_query", data) { super }
+      end
+
       {
         "lex" => "graphql.lex",
         "parse" => "graphql.parse",
         "validate" => "graphql.validate",
         "analyze_query" => "graphql.analyze",
         "analyze_multiplex" => "graphql.analyze_multiplex",
         "execute_multiplex" => "graphql.execute_multiplex",
-        "execute_query" => "graphql.execute",
         "execute_query_lazy" => "graphql.execute"
       }.each do |trace_method, platform_key|
         module_eval <<-RUBY, __FILE__, __LINE__

spec/graphql/tracing/sentry_trace_spec.rb

@@ -1,9 +1,8 @@
 # frozen_string_literal: true
-
 require "spec_helper"
 
 describe GraphQL::Tracing::SentryTrace do
-  class SentryTraceTestSchema < GraphQL::Schema
+  module SentryTraceTest
     class Thing < GraphQL::Schema::Object
       field :str, String
       def str; "blah"; end
@@ -20,31 +19,38 @@ def int
       def thing; :thing; end
     end
 
-    query(Query)
+    class SchemaWithoutTransactionName < GraphQL::Schema
+      query(Query)
 
-    module OtherTrace
-      def execute_query(query:)
-        query.context[:other_trace_ran] = true
-        super
+      module OtherTrace
+        def execute_query(query:)
+          query.context[:other_trace_ran] = true
+          super
+        end
       end
+      trace_with OtherTrace
+      trace_with GraphQL::Tracing::SentryTrace
+    end
+
+    class SchemaWithTransactionName < GraphQL::Schema
+      query(Query)
+      trace_with(GraphQL::Tracing::SentryTrace, set_transaction_name: true)
     end
-    trace_with OtherTrace
-    trace_with GraphQL::Tracing::SentryTrace
   end
 
   before do
     Sentry.clear_all
   end
 
   it "works with other trace modules" do
-    res = SentryTraceTestSchema.execute("{ int }")
+    res = SentryTraceTest::SchemaWithoutTransactionName.execute("{ int }")
     assert res.context[:other_trace_ran]
   end
 
   describe "When Sentry is not configured" do
     it "does not initialize any spans" do
       Sentry.stub(:initialized?, false) do
-        SentryTraceTestSchema.execute("{ int thing { str } }")
+        SentryTraceTest::SchemaWithoutTransactionName.execute("{ int thing { str } }")
         assert_equal [], Sentry::SPAN_DATA
         assert_equal [], Sentry::SPAN_DESCRIPTIONS
         assert_equal [], Sentry::SPAN_OPS
@@ -55,7 +61,7 @@ def execute_query(query:)
   describe "When Sentry.with_child_span returns nil" do
     it "does not initialize any spans" do
       Sentry.stub(:with_child_span, nil) do
-        SentryTraceTestSchema.execute("{ int thing { str } }")
+        SentryTraceTest::SchemaWithoutTransactionName.execute("{ int thing { str } }")
         assert_equal [], Sentry::SPAN_DATA
         assert_equal [], Sentry::SPAN_DESCRIPTIONS
         assert_equal [], Sentry::SPAN_OPS
@@ -64,7 +70,7 @@ def execute_query(query:)
   end
 
   it "sets the expected spans" do
-    SentryTraceTestSchema.execute("{ int thing { str } }")
+    SentryTraceTest::SchemaWithoutTransactionName.execute("{ int thing { str } }")
     expected_span_ops = [
       "graphql.execute_multiplex",
       "graphql.analyze_multiplex",
@@ -83,13 +89,13 @@ def execute_query(query:)
   end
 
   it "sets span descriptions for an anonymous query" do
-    SentryTraceTestSchema.execute("{ int }")
+    SentryTraceTest::SchemaWithoutTransactionName.execute("{ int }")
 
     assert_equal ["query", "query"], Sentry::SPAN_DESCRIPTIONS
   end
 
   it "sets span data for an anonymous query" do
-    SentryTraceTestSchema.execute("{ int }")
+    SentryTraceTest::SchemaWithoutTransactionName.execute("{ int }")
     expected_span_data = [
       ["graphql.document", "{ int }"],
       ["graphql.operation.type", "query"]
@@ -99,13 +105,13 @@ def execute_query(query:)
   end
 
   it "sets span descriptions for a named query" do
-    SentryTraceTestSchema.execute("query Ab { int }")
+    SentryTraceTest::SchemaWithoutTransactionName.execute("query Ab { int }")
 
     assert_equal ["query Ab", "query Ab"], Sentry::SPAN_DESCRIPTIONS
   end
 
   it "sets span data for a named query" do
-    SentryTraceTestSchema.execute("query Ab { int }")
+    SentryTraceTest::SchemaWithoutTransactionName.execute("query Ab { int }")
     expected_span_data = [
       ["graphql.document", "query Ab { int }"],
       ["graphql.operation.name", "Ab"],
@@ -114,4 +120,41 @@ def execute_query(query:)
 
     assert_equal expected_span_data.sort, Sentry::SPAN_DATA.sort
   end
+
+  it "can leave the transaction name in place" do
+    SentryTraceTest::SchemaWithoutTransactionName.execute "query X { int }"
+    assert_equal [], Sentry::TRANSACTION_NAMES
+  end
+
+  it "can override the transaction name" do
+    SentryTraceTest::SchemaWithTransactionName.execute "query X { int }"
+    assert_equal ["GraphQL/query.X"], Sentry::TRANSACTION_NAMES
+  end
+
+  it "can override the transaction name per query" do
+    # Override with `false`
+    SentryTraceTest::SchemaWithTransactionName.execute "{ int }", context: { set_sentry_transaction_name: false }
+    assert_equal [], Sentry::TRANSACTION_NAMES
+    # Override with `true`
+    SentryTraceTest::SchemaWithoutTransactionName.execute "{ int }", context: { set_sentry_transaction_name: true }
+    assert_equal ["GraphQL/query.anonymous"], Sentry::TRANSACTION_NAMES
+  end
+
+  it "falls back to a :tracing_fallback_transaction_name when provided" do
+    SentryTraceTest::SchemaWithTransactionName.execute("{ int }", context: { tracing_fallback_transaction_name: "Abcd" })
+    assert_equal ["GraphQL/query.Abcd"], Sentry::TRANSACTION_NAMES
+  end
+
+  it "does not use the :tracing_fallback_transaction_name if an operation name is present" do
+    SentryTraceTest::SchemaWithTransactionName.execute(
+      "query Ab { int }",
+      context: { tracing_fallback_transaction_name: "Cd" }
+    )
+    assert_equal ["GraphQL/query.Ab"], Sentry::TRANSACTION_NAMES
+  end
+
+  it "does not require a :tracing_fallback_transaction_name even if an operation name is not present" do
+    SentryTraceTest::SchemaWithTransactionName.execute("{ int }")
+    assert_equal ["GraphQL/query.anonymous"], Sentry::TRANSACTION_NAMES
+  end
 end

spec/support/sentry.rb

@@ -9,6 +9,7 @@ module Sentry
   SPAN_OPS = []
   SPAN_DATA = []
   SPAN_DESCRIPTIONS = []
+  TRANSACTION_NAMES = []
 
   def self.initialized?
     true
@@ -23,10 +24,15 @@ def self.with_child_span(**args, &block)
     yield DummySpan.new
   end
 
+  def self.configure_scope(&block)
+    yield DummyScope.new
+  end
+
   def self.clear_all
     SPAN_DATA.clear
     SPAN_DESCRIPTIONS.clear
     SPAN_OPS.clear
+    TRANSACTION_NAMES.clear
   end
 
   class DummySpan
@@ -42,4 +48,10 @@ def finish
       # no-op
     end
   end
+
+  class DummyScope
+    def set_transaction_name(name)
+      TRANSACTION_NAMES << name
+    end
+  end
 end