diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-18 16:16:36 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-11-18 16:16:36 +0300 |
commit | 311b0269b4eb9839fa63f80c8d7a58f32b8138a0 (patch) | |
tree | 07e7870bca8aed6d61fdcc810731c50d2c40af47 /lib/gitlab/graphql | |
parent | 27909cef6c4170ed9205afa7426b8d3de47cbb0c (diff) |
Add latest changes from gitlab-org/gitlab@14-5-stable-eev14.5.0-rc42
Diffstat (limited to 'lib/gitlab/graphql')
-rw-r--r-- | lib/gitlab/graphql/known_operations.rb | 45 | ||||
-rw-r--r-- | lib/gitlab/graphql/loaders/full_path_model_loader.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/graphql/query_analyzers/logger_analyzer.rb | 50 | ||||
-rw-r--r-- | lib/gitlab/graphql/tracers/application_context_tracer.rb | 40 | ||||
-rw-r--r-- | lib/gitlab/graphql/tracers/logger_tracer.rb | 58 | ||||
-rw-r--r-- | lib/gitlab/graphql/tracers/metrics_tracer.rb | 48 | ||||
-rw-r--r-- | lib/gitlab/graphql/tracers/timer_tracer.rb | 31 | ||||
-rw-r--r-- | lib/gitlab/graphql/variables.rb | 7 |
9 files changed, 259 insertions, 27 deletions
diff --git a/lib/gitlab/graphql/known_operations.rb b/lib/gitlab/graphql/known_operations.rb new file mode 100644 index 00000000000..ead52935945 --- /dev/null +++ b/lib/gitlab/graphql/known_operations.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + class KnownOperations + Operation = Struct.new(:name) do + def to_caller_id + "graphql:#{name}" + end + + def query_urgency + # We'll be able to actually correlate query_urgency with https://gitlab.com/gitlab-org/gitlab/-/issues/345141 + ::Gitlab::EndpointAttributes::DEFAULT_URGENCY + end + end + + ANONYMOUS = Operation.new("anonymous").freeze + UNKNOWN = Operation.new("unknown").freeze + + def self.default + @default ||= self.new(Gitlab::Webpack::GraphqlKnownOperations.load) + end + + def initialize(operation_names) + @operation_hash = operation_names + .map { |name| Operation.new(name).freeze } + .concat([ANONYMOUS, UNKNOWN]) + .index_by(&:name) + end + + # Returns the known operation from the given ::GraphQL::Query object + def from_query(query) + operation_name = query.selected_operation_name + + return ANONYMOUS unless operation_name + + @operation_hash[operation_name] || UNKNOWN + end + + def operations + @operation_hash.values + end + end + end +end diff --git a/lib/gitlab/graphql/loaders/full_path_model_loader.rb b/lib/gitlab/graphql/loaders/full_path_model_loader.rb index 7f9013c6e4c..2ea3fa71d5e 100644 --- a/lib/gitlab/graphql/loaders/full_path_model_loader.rb +++ b/lib/gitlab/graphql/loaders/full_path_model_loader.rb @@ -16,8 +16,11 @@ module Gitlab def find BatchLoader::GraphQL.for(full_path).batch(key: model_class) do |full_paths, loader, args| + scope = args[:key] + # this logic cannot be placed in the NamespaceResolver due to N+1 + scope = scope.without_project_namespaces if scope == Namespace # `with_route` avoids an N+1 calculating full_path - args[:key].where_full_path_in(full_paths).with_route.each do |model_instance| + scope.where_full_path_in(full_paths).with_route.each do |model_instance| loader.call(model_instance.full_path.downcase, model_instance) end end diff --git a/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb b/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb index 5a9d21e7469..15f95edd318 100644 --- a/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb +++ b/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb @@ -76,7 +76,7 @@ module Gitlab def items original_items = super - return original_items if Gitlab::Pagination::Keyset::Order.keyset_aware?(original_items) || Feature.disabled?(:new_graphql_keyset_pagination) + return original_items if Feature.disabled?(:new_graphql_keyset_pagination, default_enabled: :yaml) || Gitlab::Pagination::Keyset::Order.keyset_aware?(original_items) strong_memoize(:generic_keyset_pagination_items) do rebuilt_items_with_keyset_order, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(original_items) diff --git a/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb b/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb index b8d2f5b0f29..207324e73bd 100644 --- a/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb +++ b/lib/gitlab/graphql/query_analyzers/logger_analyzer.rb @@ -10,15 +10,10 @@ module Gitlab ALL_ANALYZERS = [COMPLEXITY_ANALYZER, DEPTH_ANALYZER, FIELD_USAGE_ANALYZER].freeze def initial_value(query) - variables = process_variables(query.provided_variables) - default_initial_values(query).merge({ - operation_name: query.operation_name, - query_string: query.query_string, - variables: variables - }) - rescue StandardError => e - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) - default_initial_values(query) + { + time_started: Gitlab::Metrics::System.monotonic_time, + query: query + } end def call(memo, *) @@ -28,25 +23,42 @@ module Gitlab def final_value(memo) return if memo.nil? - complexity, depth, field_usages = GraphQL::Analysis.analyze_query(memo[:query], ALL_ANALYZERS) + query = memo[:query] + complexity, depth, field_usages = GraphQL::Analysis.analyze_query(query, ALL_ANALYZERS) memo[:depth] = depth memo[:complexity] = complexity # This duration is not the execution time of the # query but the execution time of the analyzer. - memo[:duration_s] = duration(memo[:time_started]).round(1) + memo[:duration_s] = duration(memo[:time_started]) memo[:used_fields] = field_usages.first memo[:used_deprecated_fields] = field_usages.second - RequestStore.store[:graphql_logs] ||= [] - RequestStore.store[:graphql_logs] << memo - GraphqlLogger.info(memo.except!(:time_started, :query)) + push_to_request_store(memo) + + # This gl_analysis is included in the tracer log + query.context[:gl_analysis] = memo.except!(:time_started, :query) rescue StandardError => e Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) end private + def push_to_request_store(memo) + query = memo[:query] + + # TODO: This RequestStore management is used to handle setting request wide metadata + # to improve preexisting logging. We should handle this either with ApplicationContext + # or in a separate tracer. + # https://gitlab.com/gitlab-org/gitlab/-/issues/343802 + + RequestStore.store[:graphql_logs] ||= [] + RequestStore.store[:graphql_logs] << memo.except(:time_started, :duration_s, :query).merge({ + variables: process_variables(query.provided_variables), + operation_name: query.operation_name + }) + end + def process_variables(variables) filtered_variables = filter_sensitive_variables(variables) @@ -66,16 +78,6 @@ module Gitlab def duration(time_started) Gitlab::Metrics::System.monotonic_time - time_started end - - def default_initial_values(query) - { - time_started: Gitlab::Metrics::System.monotonic_time, - query_string: nil, - query: query, - variables: nil, - duration_s: nil - } - end end end end diff --git a/lib/gitlab/graphql/tracers/application_context_tracer.rb b/lib/gitlab/graphql/tracers/application_context_tracer.rb new file mode 100644 index 00000000000..4193c46e321 --- /dev/null +++ b/lib/gitlab/graphql/tracers/application_context_tracer.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Tracers + # This graphql-ruby tracer sets up `ApplicationContext` for certain operations. + class ApplicationContextTracer + def self.use(schema) + schema.tracer(self.new) + end + + # See docs on expected interface for trace + # https://graphql-ruby.org/api-doc/1.12.17/GraphQL/Tracing + def trace(key, data) + case key + when "execute_query" + operation = known_operation(data) + + ::Gitlab::ApplicationContext.with_context(caller_id: operation.to_caller_id) do + yield + end + else + yield + end + end + + private + + def known_operation(data) + # The library guarantees that we should have :query for execute_query, but we're being defensive here + query = data.fetch(:query, nil) + + return ::Gitlab::Graphql::KnownOperations.UNKNOWN unless query + + ::Gitlab::Graphql::KnownOperations.default.from_query(query) + end + end + end + end +end diff --git a/lib/gitlab/graphql/tracers/logger_tracer.rb b/lib/gitlab/graphql/tracers/logger_tracer.rb new file mode 100644 index 00000000000..c7ba56824db --- /dev/null +++ b/lib/gitlab/graphql/tracers/logger_tracer.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Tracers + # This tracer writes logs for certain trace events. + # It reads duration metadata written by TimerTracer. + class LoggerTracer + def self.use(schema) + schema.tracer(self.new) + end + + def trace(key, data) + result = yield + + case key + when "execute_query" + log_execute_query(**data) + end + + result + end + + private + + def log_execute_query(query: nil, duration_s: 0) + # execute_query should always have :query, but we're just being defensive + return unless query + + analysis_info = query.context[:gl_analysis]&.transform_keys { |key| "query_analysis.#{key}" } + info = { + trace_type: 'execute_query', + query_fingerprint: query.fingerprint, + duration_s: duration_s, + operation_name: query.operation_name, + operation_fingerprint: query.operation_fingerprint, + is_mutation: query.mutation?, + variables: clean_variables(query.provided_variables), + query_string: query.query_string + } + + info.merge!(::Gitlab::ApplicationContext.current) + info.merge!(analysis_info) if analysis_info + + ::Gitlab::GraphqlLogger.info(info) + end + + def clean_variables(variables) + filtered = ActiveSupport::ParameterFilter + .new(::Rails.application.config.filter_parameters) + .filter(variables) + + filtered&.to_s + end + end + end + end +end diff --git a/lib/gitlab/graphql/tracers/metrics_tracer.rb b/lib/gitlab/graphql/tracers/metrics_tracer.rb new file mode 100644 index 00000000000..9fc001c0a6d --- /dev/null +++ b/lib/gitlab/graphql/tracers/metrics_tracer.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Tracers + class MetricsTracer + def self.use(schema) + schema.tracer(self.new) + end + + # See https://graphql-ruby.org/api-doc/1.12.16/GraphQL/Tracing for full list of events + def trace(key, data) + result = yield + + case key + when "execute_query" + increment_query_sli(data) + end + + result + end + + private + + def increment_query_sli(data) + duration_s = data.fetch(:duration_s, nil) + query = data.fetch(:query, nil) + + # We're just being defensive here... + # duration_s comes from TimerTracer and we should be pretty much guaranteed it exists + return unless duration_s && query + + operation = ::Gitlab::Graphql::KnownOperations.default.from_query(query) + query_urgency = operation.query_urgency + + Gitlab::Metrics::RailsSlis.graphql_query_apdex.increment( + labels: { + endpoint_id: ::Gitlab::ApplicationContext.current_context_attribute(:caller_id), + feature_category: ::Gitlab::ApplicationContext.current_context_attribute(:feature_category), + query_urgency: query_urgency.name + }, + success: duration_s <= query_urgency.duration + ) + end + end + end + end +end diff --git a/lib/gitlab/graphql/tracers/timer_tracer.rb b/lib/gitlab/graphql/tracers/timer_tracer.rb new file mode 100644 index 00000000000..326620a22bc --- /dev/null +++ b/lib/gitlab/graphql/tracers/timer_tracer.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module Tracers + # This graphql-ruby tracer records duration for trace events and merges + # the duration into the trace event's metadata. This way, separate tracers + # can all use the same duration information. + # + # NOTE: TimerTracer should be applied last **after** other tracers, so + # that it runs first (similar to function composition) + class TimerTracer + def self.use(schema) + schema.tracer(self.new) + end + + def trace(key, data) + start_time = Gitlab::Metrics::System.monotonic_time + + result = yield + + duration_s = Gitlab::Metrics::System.monotonic_time - start_time + + data[:duration_s] = duration_s + + result + end + end + end + end +end diff --git a/lib/gitlab/graphql/variables.rb b/lib/gitlab/graphql/variables.rb index e17ca56d022..102a269dd5b 100644 --- a/lib/gitlab/graphql/variables.rb +++ b/lib/gitlab/graphql/variables.rb @@ -24,8 +24,13 @@ module Gitlab else {} end - when Hash, ActionController::Parameters + when Hash ambiguous_param + when ActionController::Parameters + # We can and have to trust the "Parameters" because `graphql-ruby` handles this hash safely + # Also, `graphql-ruby` uses hash-specific methods, for example `size`: + # https://sourcegraph.com/github.com/rmosolgo/graphql-ruby@61232b03412df6685406fc46c414e11d3f447817/-/blob/lib/graphql/query.rb?L304 + ambiguous_param.to_unsafe_h when nil {} else |