Skip to content

Commit acd1455

Browse files
committed
fix: GraphQL validate events
Fix the logic for adding `graphql.validation.error` events on `validate` spans. The `errors` variable would always end up being a serialized `"[]"` empty array which wasn't nil and an empty `graphql.validation.error` event would be added.
1 parent b76e752 commit acd1455

File tree

3 files changed

+34
-21
lines changed

3 files changed

+34
-21
lines changed

instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_trace.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,13 @@ def parse(query_string:, &block)
3636
def validate(query:, validate:, &block)
3737
tracer.in_span('graphql.validate') do |span|
3838
super.tap do |response|
39-
errors = response[:errors]&.compact&.map(&:to_h)&.to_json
40-
unless errors.nil?
39+
errors = response[:errors]&.compact&.map(&:to_h) || []
40+
41+
unless errors.empty?
4142
span.add_event(
4243
'graphql.validation.error',
4344
attributes: {
44-
'message' => errors
45+
'message' => errors.to_json
4546
}
4647
)
4748
end

instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,15 @@ def platform_trace(platform_key, key, data) # rubocop:disable Metrics/Cyclomatic
2929

3030
tracer.in_span(platform_key, attributes: attributes_for(key, data)) do |span|
3131
yield.tap do |response|
32-
errors = response[:errors]&.compact&.map(&:to_h)&.to_json if key == 'validate'
33-
unless errors.nil?
32+
next unless key == 'validate'
33+
34+
errors = response[:errors]&.compact&.map(&:to_h) || []
35+
36+
unless errors.empty?
3437
span.add_event(
3538
'graphql.validation.error',
3639
attributes: {
37-
'message' => errors
40+
'message' => errors.to_json
3841
}
3942
)
4043
end

instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracing_test.rb

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -252,21 +252,30 @@
252252
end
253253
end
254254

255-
it 'traces validate with events' do
256-
SomeGraphQLAppSchema.execute(
257-
<<-GRAPHQL
258-
{
259-
nonExistentField
260-
}
261-
GRAPHQL
262-
)
263-
span = spans.find { |s| s.name == 'graphql.validate' }
264-
event = span.events.find { |e| e.name == 'graphql.validation.error' }
265-
# rubocop:disable Layout/LineLength
266-
_(event.attributes['message']).must_equal(
267-
"[{\"message\":\"Field 'nonExistentField' doesn't exist on type 'Query'\",\"locations\":[{\"line\":2,\"column\":13}],\"path\":[\"query\",\"nonExistentField\"],\"extensions\":{\"code\":\"undefinedField\",\"typeName\":\"Query\",\"fieldName\":\"nonExistentField\"}}]"
268-
)
269-
# rubocop:enable Layout/LineLength
255+
describe 'validate spans' do
256+
it 'adds events for validation errors' do
257+
SomeGraphQLAppSchema.execute(
258+
<<-GRAPHQL
259+
{
260+
nonExistentField
261+
}
262+
GRAPHQL
263+
)
264+
span = spans.find { |s| s.name == 'graphql.validate' }
265+
event = span.events.find { |e| e.name == 'graphql.validation.error' }
266+
# rubocop:disable Layout/LineLength
267+
_(event.attributes['message']).must_equal(
268+
"[{\"message\":\"Field 'nonExistentField' doesn't exist on type 'Query'\",\"locations\":[{\"line\":2,\"column\":15}],\"path\":[\"query\",\"nonExistentField\"],\"extensions\":{\"code\":\"undefinedField\",\"typeName\":\"Query\",\"fieldName\":\"nonExistentField\"}}]"
269+
)
270+
# rubocop:enable Layout/LineLength
271+
end
272+
273+
it 'does not add events for valid documents' do
274+
SomeGraphQLAppSchema.execute('{ vehicle { __typename } }')
275+
276+
span = spans.find { |s| s.name == 'graphql.validate' }
277+
_(span.events).must_be_nil
278+
end
270279
end
271280
end
272281

0 commit comments

Comments
 (0)