From 2586b759c66ad1892bd8c6a75bd6cb9dc0e090fb Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Fri, 22 Dec 2023 15:30:17 -0600 Subject: [PATCH] chore: Add Tests for GraphQL C Parser (#773) Graphql 2.2 introduces an optional C implementation of the Parser as well as optimizations to the pure Ruby parser that result in changes to what is captured during instrumentation. Users of the pure Ruby implementation will no longer see graphql.lex spans emitted as a part of their traces and will instead need to switch to using the C Parser in order to explicitly see time spent in the lexer. This PR introduces updates to the test suite to add coverage for the C Parser and changes to the Ruby implementation. There are no underlying changes in the instrumentation itself. --- instrumentation/graphql/Appraisals | 35 +++++++++++++++---- .../graphql/tracers/graphql_trace_test.rb | 4 ++- .../graphql/tracers/graphql_tracer_test.rb | 3 +- instrumentation/graphql/test/test_helper.rb | 9 +++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/instrumentation/graphql/Appraisals b/instrumentation/graphql/Appraisals index d478f591b..05298c160 100644 --- a/instrumentation/graphql/Appraisals +++ b/instrumentation/graphql/Appraisals @@ -4,18 +4,39 @@ # # SPDX-License-Identifier: Apache-2.0 -appraise 'graphql-1.13' do +# Max compatible version of 1.x +appraise 'graphql-1.x' do gem 'graphql', '~> 1.13' end -appraise 'graphql-2.0.17' do - gem 'graphql', '2.0.17' -end - +# A bug was introduced in 2.0.18 that was fixed in 2.0.19 appraise 'graphql-2.0.18' do gem 'graphql', '2.0.18' end -appraise 'graphql-2.x' do - gem 'graphql', '>= 2.0.18', '< 3.0.0' +# Max compatible version of 2.0.x +appraise 'graphql-2.0' do + gem 'graphql', '~> 2.0.27' +end + +# Max compatible version of 2.1.x +appraise 'graphql-2.1' do + gem 'graphql', '~> 2.1.8' +end + +appraise 'graphql-c_parser-2.2.x' do + gem 'graphql', '~> 2.2.1' + gem 'graphql-c_parser', '~> 1.0.7' +end + +appraise 'graphql-2.2.x' do + gem 'graphql', '~> 2.2.1', '< 3.0.0' +end + +appraise 'graphql-c_parser-latest' do + gem 'graphql-c_parser' +end + +appraise 'graphql-latest' do + gem 'graphql' end diff --git a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb index 59de00473..4ae7fb15e 100644 --- a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb +++ b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb @@ -54,6 +54,8 @@ 'graphql.execute_multiplex' ] + expected_spans.delete('graphql.lex') unless trace_lex_supported? + expected_result = { 'simpleField' => 'Hello.', 'resolvedField' => { 'originalValue' => 'testing=1', 'uppercasedValue' => 'TESTING=1' } @@ -103,7 +105,7 @@ it 'traces the provided schemas' do SomeOtherGraphQLAppSchema.execute('query SimpleQuery{ __typename }') - _(spans.size).must_equal(8) + _(spans.select { |s| s.name.start_with?('graphql.') }).wont_be(:empty?) end it 'does not trace all schemas' do diff --git a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb index 35e5e4b0d..0813c22c0 100644 --- a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb +++ b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb @@ -53,6 +53,7 @@ 'graphql.execute_query_lazy', 'graphql.execute_multiplex' ] + expected_spans.delete('graphql.lex') unless trace_lex_supported? expected_result = { 'simpleField' => 'Hello.', @@ -102,7 +103,7 @@ it 'traces the provided schemas' do SomeOtherGraphQLAppSchema.execute('query SimpleQuery{ __typename }') - _(spans.size).must_equal(8) + _(spans.select { |s| s.name.start_with?('graphql.') }).wont_be(:empty?) end it 'does not trace all schemas' do diff --git a/instrumentation/graphql/test/test_helper.rb b/instrumentation/graphql/test/test_helper.rb index 75f349b28..b25005d16 100644 --- a/instrumentation/graphql/test/test_helper.rb +++ b/instrumentation/graphql/test/test_helper.rb @@ -125,3 +125,12 @@ def uses_platform_interfaces? def gem_version Gem::Version.new(GraphQL::VERSION) end + +# When tracing, is the parser expected to call `lex` before `parse` +def trace_lex_supported? + return @trace_lex_supported if defined?(@trace_lex_supported) + + # In GraphQL 2.2, the default parser was changed such that `lex` is no longer called + @trace_lex_supported = Gem::Requirement.new('< 2.2').satisfied_by?(Gem::Version.new(GraphQL::VERSION)) \ + || (defined?(GraphQL::CParser) == 'constant') +end