Merge pull request #5277 from rmosolgo/nr-fix-dataloaded-skipped-scalars

NewRelicTrace: fix dataloaded, skipped scalar fields

Author: rmosolgo

lib/graphql/tracing/new_relic_trace.rb

@@ -158,8 +158,10 @@ def dataloader_fiber_yield(source)
       def dataloader_fiber_resume(source)
         prev_segment = Fiber[:graphql_nr_previous_segment]
         Fiber[:graphql_nr_previous_segment] = nil
-        seg_partial_name = prev_segment.name.sub(/^.*(GraphQL.*)$/, '\1')
-        start_segment(partial_name: seg_partial_name, category: :web)
+        if prev_segment
+          seg_partial_name = prev_segment.name.sub(/^.*(GraphQL.*)$/, '\1')
+          start_segment(partial_name: seg_partial_name, category: :web)
+        end
         super
       end
 

spec/graphql/tracing/new_relic_trace_spec.rb

@@ -3,15 +3,15 @@
 
 describe GraphQL::Tracing::NewRelicTrace do
   module NewRelicTraceTest
-    class NestedSource < GraphQL::Dataloader::Source
+    class EchoSource < GraphQL::Dataloader::Source
       def fetch(keys)
         keys
       end
     end
 
     class OtherSource < GraphQL::Dataloader::Source
       def fetch(keys)
-        dataloader.with(NestedSource).load_all(keys)
+        dataloader.with(EchoSource).load_all(keys)
       end
     end
     class Thing < GraphQL::Schema::Object
@@ -31,7 +31,11 @@ class Other < GraphQL::Schema::Object
       def self.authorized?(obj, ctx)
         ctx[:lazy] ? -> { true } : true
       end
-      field :name, String, fallback_value: "other"
+      field :name, String
+
+      def name
+        dataload(EchoSource, "name")
+      end
     end
 
     class Query < GraphQL::Schema::Object
@@ -83,6 +87,7 @@ class SchemaWithScalarTrace < GraphQL::Schema
     class SchemaWithoutAuthorizedOrResolveType < GraphQL::Schema
       query(Query)
       trace_with(GraphQL::Tracing::NewRelicTrace, set_transaction_name: true, trace_authorized: false, trace_resolve_type: false, trace_scalars: true)
+      use GraphQL::Dataloader
     end
   end
 
@@ -157,15 +162,17 @@ class SchemaWithoutAuthorizedOrResolveType < GraphQL::Schema
         "GraphQL/Source/NewRelicTraceTest::OtherSource",
         "FINISH GraphQL/Source/NewRelicTraceTest::OtherSource",
           # Nested source:
-          "GraphQL/Source/NewRelicTraceTest::NestedSource",
-          "FINISH GraphQL/Source/NewRelicTraceTest::NestedSource",
+          "GraphQL/Source/NewRelicTraceTest::EchoSource",
+          "FINISH GraphQL/Source/NewRelicTraceTest::EchoSource",
         "GraphQL/Source/NewRelicTraceTest::OtherSource",
         "FINISH GraphQL/Source/NewRelicTraceTest::OtherSource",
         # And back to the field:
         "GraphQL/Query/other",
         "FINISH GraphQL/Query/other",
         "GraphQL/Authorized/Other",
         "FINISH GraphQL/Authorized/Other",
+          "GraphQL/Source/NewRelicTraceTest::EchoSource",
+          "FINISH GraphQL/Source/NewRelicTraceTest::EchoSource",
       "FINISH GraphQL/execute"
     ]
     assert_equal expected_steps, NewRelic::EXECUTION_SCOPES
@@ -191,6 +198,12 @@ class SchemaWithoutAuthorizedOrResolveType < GraphQL::Schema
       # "FINISH GraphQL/Authorized/Other",
       "GraphQL/Other/name",
       "FINISH GraphQL/Other/name",
+
+      "GraphQL/Source/NewRelicTraceTest::EchoSource",
+      "FINISH GraphQL/Source/NewRelicTraceTest::EchoSource",
+      "GraphQL/Other/name",
+      "FINISH GraphQL/Other/name",
+
       "FINISH GraphQL/execute"
     ]
     assert_equal expected_steps, NewRelic::EXECUTION_SCOPES
@@ -232,6 +245,8 @@ class SchemaWithoutAuthorizedOrResolveType < GraphQL::Schema
       "FINISH GraphQL/Authorized/Other",
       "GraphQL/Authorized/Other",
       "FINISH GraphQL/Authorized/Other",
+      "GraphQL/Source/NewRelicTraceTest::EchoSource",
+      "FINISH GraphQL/Source/NewRelicTraceTest::EchoSource",
 
       "FINISH GraphQL/execute",
     ]