From b354b730d5dcc0393efa848b9e0349a0cef68f94 Mon Sep 17 00:00:00 2001 From: Karming Date: Fri, 3 May 2024 16:24:51 -0400 Subject: [PATCH] fix: update GraphQL span name to follow semantic conventions The semantic convention for GraphQL spans is specified here: https://opentelemetry.io/docs/specs/semconv/graphql/graphql-spans/. This will also resolve https://github.com/open-telemetry/opentelemetry-ruby-contrib/issues/560. --- instrumentation/graphql/Gemfile | 1 + .../graphql/tracers/graphql_trace.rb | 10 +++++++--- .../graphql/tracers/graphql_tracer.rb | 14 +++++++++++++- .../graphql/tracers/graphql_trace_test.rb | 6 +++--- .../graphql/tracers/graphql_tracer_test.rb | 6 +++--- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/instrumentation/graphql/Gemfile b/instrumentation/graphql/Gemfile index 5b7d54b645..069a7e9340 100644 --- a/instrumentation/graphql/Gemfile +++ b/instrumentation/graphql/Gemfile @@ -10,5 +10,6 @@ gemspec group :test do gem 'opentelemetry-instrumentation-base', path: '../base' + gem 'graphql' gem 'pry-byebug' end diff --git a/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_trace.rb b/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_trace.rb index 70c9666235..500869574d 100644 --- a/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_trace.rb +++ b/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_trace.rb @@ -98,11 +98,15 @@ def analyze_query(query:, &block) def execute_query(query:, &block) attributes = {} - attributes['graphql.operation.name'] = query.selected_operation_name if query.selected_operation_name - attributes['graphql.operation.type'] = query.selected_operation.operation_type + operation_type = query.selected_operation.operation_type + operation_name = query.selected_operation_name + + attributes['graphql.operation.name'] = operation_name if operation_name + attributes['graphql.operation.type'] = operation_type attributes['graphql.document'] = query.query_string - tracer.in_span('graphql.execute_query', attributes: attributes, &block) + span_name = operation_name ? "#{operation_type} #{operation_name}" : operation_type + tracer.in_span(span_name, attributes: attributes, &block) end def execute_query_lazy(query:, multiplex:, &block) diff --git a/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb b/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb index 0d4c3b0ad1..0171e3ebdf 100644 --- a/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb +++ b/instrumentation/graphql/lib/opentelemetry/instrumentation/graphql/tracers/graphql_tracer.rb @@ -29,7 +29,7 @@ class GraphQLTracer < ::GraphQL::Tracing::PlatformTracing def platform_trace(platform_key, key, data) return yield if platform_key.nil? - tracer.in_span(platform_key, attributes: attributes_for(key, data)) do |span| + tracer.in_span(span_name(platform_key, data), attributes: attributes_for(key, data)) do |span| yield.tap do |response| next unless key == 'validate' @@ -143,6 +143,18 @@ def attr_cache cache_h.compare_by_identity cache_h end + + def span_name(key, data) + case key + when @platform_keys['execute_query'] + operation_type = data[:query].selected_operation.operation_type + operation_name = data[:query].selected_operation_name + + operation_name ? "#{operation_type} #{operation_name}" : operation_type + else + key + end + end end end 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 22e194036f..c2fa38c742 100644 --- a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb +++ b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_trace_test.rb @@ -49,7 +49,7 @@ 'graphql.validate', 'graphql.analyze_query', 'graphql.analyze_multiplex', - 'graphql.execute_query', + 'query', 'graphql.execute_query_lazy', 'graphql.execute_multiplex' ] @@ -76,7 +76,7 @@ SomeGraphQLAppSchema.execute('query SimpleQuery{ simpleField }') - span = spans.find { |s| s.name == 'graphql.execute_query' } + span = spans.find { |s| s.name == 'query SimpleQuery' } _(span).wont_be_nil _(span.attributes.to_h).must_equal(expected_attributes) end @@ -89,7 +89,7 @@ SomeGraphQLAppSchema.execute('{ simpleField }') - span = spans.find { |s| s.name == 'graphql.execute_query' } + span = spans.find { |s| s.name == 'query' } _(span).wont_be_nil _(span.attributes.to_h).must_equal(expected_attributes) end 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 61908a79d2..27cd59b9c6 100644 --- a/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb +++ b/instrumentation/graphql/test/instrumentation/graphql/tracers/graphql_tracer_test.rb @@ -49,7 +49,7 @@ 'graphql.validate', 'graphql.analyze_query', 'graphql.analyze_multiplex', - 'graphql.execute_query', + 'query', 'graphql.execute_query_lazy', 'graphql.execute_multiplex' ] @@ -75,7 +75,7 @@ SomeGraphQLAppSchema.execute('query SimpleQuery{ simpleField }') - span = spans.find { |s| s.name == 'graphql.execute_query' } + span = spans.find { |s| s.name == 'query SimpleQuery' } _(span).wont_be_nil _(span.attributes.to_h).must_equal(expected_attributes) end @@ -89,7 +89,7 @@ SomeGraphQLAppSchema.execute('{ simpleField }') - span = spans.find { |s| s.name == 'graphql.execute_query' } + span = spans.find { |s| s.name == 'query' } _(span).wont_be_nil _(span.attributes.to_h).must_equal(expected_attributes) end