Skip to content

Commit a9eed77

Browse files
authored
fix(tracing): Properly handle tracing fields that resolve an array of lazy values (#87)
* test: Add failing test * fix(tracing): Properly handle arrays of lazy values * refactor: Move EntityTypeResolutionExtension.after_resolve behavior directly into the _entities resolver * fix: Remove broken require; * style: Fix linter errors * test: Add more test cases * 🎨 Cleanup * Clarify todo * style: Fix linter error
1 parent 75f0265 commit a9eed77

File tree

6 files changed

+306
-97
lines changed

6 files changed

+306
-97
lines changed

.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ node_modules/
77

88
# built CircleCI configs
99
.circleci/processed-config.yml
10+
11+
.DS_Store

lib/apollo-federation/entities_field.rb

+8-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
require 'graphql'
44
require 'apollo-federation/any'
5-
require 'apollo-federation/entity_type_resolution_extension'
65

76
module ApolloFederation
87
module EntitiesField
@@ -23,7 +22,6 @@ def define_entities_field(possible_entities)
2322

2423
field(:_entities, [entity_type, null: true], null: false) do
2524
argument :representations, [Any], required: true
26-
extension(EntityTypeResolutionExtension)
2725
end
2826
end
2927
end
@@ -47,7 +45,14 @@ def _entities(representations:)
4745
result = reference
4846
end
4947

50-
[type, result]
48+
context.schema.after_lazy(result) do |resolved_value|
49+
# TODO: This isn't 100% correct: if (for some reason) 2 different resolve_reference calls
50+
# return the same object, it might not have the right type
51+
# Right now, apollo-federation just adds a __typename property to the result,
52+
# but I don't really like the idea of modifying the resolved object
53+
context[resolved_value] = type
54+
resolved_value
55+
end
5156
end
5257
end
5358
end

lib/apollo-federation/entity_type_resolution_extension.rb

-16
This file was deleted.

lib/apollo-federation/tracing.rb

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ def attach_trace_to_result(result)
2727
return result unless result.context[:tracing_enabled]
2828

2929
trace = result.context.namespace(KEY)
30+
# TODO: If there are errors during query validation, that could also cause a missing
31+
# start_time
3032
raise NotInstalledError unless trace[:start_time]
3133

3234
result['errors']&.each do |error|

lib/apollo-federation/tracing/tracer.rb

+14-4
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,27 @@ def self.execute_field_lazy(data, &block)
159159

160160
end_time_nanos = Process.clock_gettime(Process::CLOCK_MONOTONIC, :nanosecond)
161161

162-
# interpreter runtime
162+
# legacy runtime
163163
if data.include?(:context)
164-
context = data.fetch(:context)
165164
path = context.path
166-
else # legacy runtime
167-
context = data.fetch(:query).context
165+
field = context.field
166+
else # interpreter runtime
168167
path = data.fetch(:path)
168+
field = data.fetch(:field)
169169
end
170170

171171
trace = context.namespace(ApolloFederation::Tracing::KEY)
172172

173+
# When a field is resolved with an array of lazy values, the interpreter fires an
174+
# `execute_field` for the resolution of the field and then a `execute_field_lazy` event for
175+
# each lazy value in the array. Since the path here will contain an index (indicating which
176+
# lazy value we're executing: e.g. ['arrayOfLazies', 0]), we won't have a node for the path.
177+
# We only care about the end of the parent field (e.g. ['arrayOfLazies']), so we get the
178+
# node for that path. What ends up happening is we update the end_time for the parent node
179+
# for each of the lazy values. The last one that's executed becomes the final end time.
180+
if field.type.list? && path.last.is_a?(Integer)
181+
path = path[0...-1]
182+
end
173183
node = trace[:node_map].node_for_path(path)
174184
node.end_time = end_time_nanos - trace[:start_time_nanos]
175185

0 commit comments

Comments
 (0)