diff --git a/app/controllers/glql/base_controller.rb b/app/controllers/glql/base_controller.rb index 3f031860220a14f1f1338e20613f4b9513561dff..ed4566e4183a3a64e853f041462ab5f9f5443a0b 100644 --- a/app/controllers/glql/base_controller.rb +++ b/app/controllers/glql/base_controller.rb @@ -31,6 +31,15 @@ def execute private + def logs + super.map do |log| + log.merge( + glql_referer: request.headers["Referer"], + glql_query_sha: query_sha + ) + end + end + def check_rate_limit return unless Gitlab::ApplicationRateLimiter.peek(:glql, scope: query_sha) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index fe13a3dfd999ef91f60ba83987b386dcb744cf4f..d10fad48918916eba8e5d6af12f86cc31626375a 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -330,13 +330,7 @@ def append_info_to_payload(payload) end def logs - graphql_logs = RequestStore.store[:graphql_logs] - - Array(graphql_logs).map do |log| - next log unless log[:operation_name] == 'GLQL' - - log.merge(glql_referer: request.headers["Referer"]) - end + RequestStore.store[:graphql_logs].to_a end def execute_introspection_query diff --git a/spec/controllers/glql/base_controller_spec.rb b/spec/controllers/glql/base_controller_spec.rb index 236520066695007b770fbd3f2fa97a4315313ade..0141c7964c36082a7b1124caf5976229a29389b0 100644 --- a/spec/controllers/glql/base_controller_spec.rb +++ b/spec/controllers/glql/base_controller_spec.rb @@ -3,11 +3,11 @@ require 'spec_helper' RSpec.describe Glql::BaseController, feature_category: :integrations do + let(:query) { 'query GLQL { __typename }' } + let(:query_sha) { Digest::SHA256.hexdigest(query) } + describe 'POST #execute' do let(:user) { create(:user, last_activity_on: 2.days.ago.to_date) } - let(:query) { 'query GLQL { __typename }' } - let(:operation_name) { 'GLQL' } - let(:query_sha) { Digest::SHA256.hexdigest(query) } before do sign_in(user) @@ -102,6 +102,42 @@ end end + describe '#append_info_to_payload' do + let(:log_payload) { {} } + let(:expected_logs) do + [ + { + operation_name: 'GLQL', + complexity: 1, + depth: 1, + used_deprecated_arguments: [], + used_deprecated_fields: [], + used_fields: ['Query.__typename'], + variables: '{}', + glql_referer: 'path', + glql_query_sha: query_sha + } + ] + end + + before do + RequestStore.clear! + + request.headers['Referer'] = 'path' + + allow(controller).to receive(:append_info_to_payload).and_wrap_original do |method, *| + method.call(log_payload) + end + end + + it 'appends glql-related metadata for logging' do + execute_request + + expect(controller).to have_received(:append_info_to_payload) + expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_logs) + end + end + describe 'rescue_from' do let(:error_message) do 'Query execution is locked due to repeated failures.' @@ -121,7 +157,7 @@ end def execute_request - post :execute, params: { query: query, operationName: operation_name } + post :execute, params: { query: query, operationName: 'GLQL' } end def current_rate_limit_value(sha) diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index 973a4fae376fc84001af221cbe32b4f301d78332..7cbcf4f49438e21ae3ff126867a6cdd3ca513811 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -880,36 +880,12 @@ end end - context 'when source is not glql' do - it 'appends metadata for logging' do - post :execute, params: { _json: graphql_queries } - - expect(controller).to have_received(:append_info_to_payload) - expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_logs) - expect(log_payload.dig(:metadata, :referer)).to be_nil - end - end - - context 'when source is glql' do - let(:query_1) { { query: graphql_query_for('project', { 'fullPath' => 'foo' }, %w[id name], 'GLQL') } } - let(:query_2) { { query: graphql_query_for('project', { 'fullPath' => 'bar' }, %w[id], 'GLQL') } } - - let(:expected_glql_logs) do - expected_logs.map do |q| - q.merge(glql_referer: 'path', operation_name: "GLQL") - end - end - - before do - request.headers['Referer'] = 'path' - end - - it 'appends glql-related metadata for logging' do - post :execute, params: { _json: graphql_queries } + it 'appends metadata for logging' do + post :execute, params: { _json: graphql_queries } - expect(controller).to have_received(:append_info_to_payload) - expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_glql_logs) - end + expect(controller).to have_received(:append_info_to_payload) + expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_logs) + expect(log_payload.dig(:metadata, :referer)).to be_nil end it 'appends the exception in case of errors' do