Commit 07b76c54 authored by Mehmet Emin INAC's avatar Mehmet Emin INAC Committed by Alex Kalderimis

Log all GraphQL operation names

Changes the log analyzer to log all operation names in multiplex queries.
parent 25903c13
...@@ -146,8 +146,7 @@ class GraphqlController < ApplicationController ...@@ -146,8 +146,7 @@ class GraphqlController < ApplicationController
end end
def logs def logs
RequestStore.store[:graphql_logs].to_h RequestStore.store[:graphql_logs].to_a
.except(:duration_s, :query_string) .map { |log| log.except(:duration_s, :query_string) }
.merge(operation_name: params[:operationName])
end end
end end
...@@ -12,6 +12,7 @@ module Gitlab ...@@ -12,6 +12,7 @@ module Gitlab
def initial_value(query) def initial_value(query)
variables = process_variables(query.provided_variables) variables = process_variables(query.provided_variables)
default_initial_values(query).merge({ default_initial_values(query).merge({
operation_name: query.operation_name,
query_string: query.query_string, query_string: query.query_string,
variables: variables variables: variables
}) })
...@@ -20,8 +21,8 @@ module Gitlab ...@@ -20,8 +21,8 @@ module Gitlab
default_initial_values(query) default_initial_values(query)
end end
def call(memo, visit_type, irep_node) def call(memo, *)
RequestStore.store[:graphql_logs] = memo memo
end end
def final_value(memo) def final_value(memo)
...@@ -37,6 +38,8 @@ module Gitlab ...@@ -37,6 +38,8 @@ module Gitlab
memo[:used_fields] = field_usages.first memo[:used_fields] = field_usages.first
memo[:used_deprecated_fields] = field_usages.second memo[:used_deprecated_fields] = field_usages.second
RequestStore.store[:graphql_logs] ||= []
RequestStore.store[:graphql_logs] << memo
GraphqlLogger.info(memo.except!(:time_started, :query)) GraphqlLogger.info(memo.except!(:time_started, :query))
rescue => e rescue => e
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
......
...@@ -175,22 +175,44 @@ RSpec.describe GraphqlController do ...@@ -175,22 +175,44 @@ RSpec.describe GraphqlController do
end end
describe '#append_info_to_payload' do describe '#append_info_to_payload' do
let(:graphql_query) { graphql_query_for('project', { 'fullPath' => 'foo' }, %w(id name)) } let(:query_1) { { query: graphql_query_for('project', { 'fullPath' => 'foo' }, %w(id name), 'getProject_1') } }
let(:mock_store) { { graphql_logs: { foo: :bar } } } let(:query_2) { { query: graphql_query_for('project', { 'fullPath' => 'bar' }, %w(id), 'getProject_2') } }
let(:graphql_queries) { [query_1, query_2] }
let(:log_payload) { {} } let(:log_payload) { {} }
let(:expected_logs) do
[
{
operation_name: 'getProject_1',
complexity: 3,
depth: 2,
used_deprecated_fields: [],
used_fields: ['Project.id', 'Project.name', 'Query.project'],
variables: '{}'
},
{
operation_name: 'getProject_2',
complexity: 2,
depth: 2,
used_deprecated_fields: [],
used_fields: ['Project.id', 'Query.project'],
variables: '{}'
}
]
end
before do before do
allow(RequestStore).to receive(:store).and_return(mock_store) RequestStore.clear!
allow(controller).to receive(:append_info_to_payload).and_wrap_original do |method, *| allow(controller).to receive(:append_info_to_payload).and_wrap_original do |method, *|
method.call(log_payload) method.call(log_payload)
end end
end end
it 'appends metadata for logging' do it 'appends metadata for logging' do
post :execute, params: { query: graphql_query, operationName: 'Foo' } post :execute, params: { _json: graphql_queries }
expect(controller).to have_received(:append_info_to_payload) expect(controller).to have_received(:append_info_to_payload)
expect(log_payload.dig(:metadata, :graphql)).to eq({ operation_name: 'Foo', foo: :bar }) expect(log_payload.dig(:metadata, :graphql)).to match_array(expected_logs)
end end
end end
end end
...@@ -3,43 +3,46 @@ ...@@ -3,43 +3,46 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
subject { described_class.new } let(:initial_value) { analyzer.initial_value(query) }
let(:analyzer) { described_class.new }
describe '#initial_value' do let(:query) { GraphQL::Query.new(GitlabSchema, document: document, context: {}, variables: { body: "some note" }) }
it 'filters out sensitive variables' do let(:document) do
doc = GraphQL.parse <<-GRAPHQL GraphQL.parse <<-GRAPHQL
mutation createNote($body: String!) { mutation createNote($body: String!) {
createNote(input: {noteableId: "1", body: $body}) { createNote(input: {noteableId: "1", body: $body}) {
note { note {
id id
}
} }
} }
GRAPHQL }
GRAPHQL
end
query = GraphQL::Query.new(GitlabSchema, document: doc, context: {}, variables: { body: "some note" }) describe 'variables' do
subject { initial_value.fetch(:variables) }
expect(subject.initial_value(query)[:variables]).to eq('{:body=>"[FILTERED]"}') it { is_expected.to eq('{:body=>"[FILTERED]"}') }
end
end end
describe '#final_value' do describe '#final_value' do
let(:monotonic_time_before) { 42 } let(:monotonic_time_before) { 42 }
let(:monotonic_time_after) { 500 } let(:monotonic_time_after) { 500 }
let(:monotonic_time_duration) { monotonic_time_after - monotonic_time_before } let(:monotonic_time_duration) { monotonic_time_after - monotonic_time_before }
let(:memo) { initial_value }
subject(:final_value) { analyzer.final_value(memo) }
before do
RequestStore.store[:graphql_logs] = nil
it 'returns a duration in seconds' do
allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2, [[], []]]) allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2, [[], []]])
allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
allow(Gitlab::GraphqlLogger).to receive(:info) allow(Gitlab::GraphqlLogger).to receive(:info)
end
expected_duration = monotonic_time_duration it 'inserts duration in seconds to memo and sets request store' do
memo = subject.initial_value(spy('query')) expect { final_value }.to change { memo[:duration_s] }.to(monotonic_time_duration)
.and change { RequestStore.store[:graphql_logs] }.to([memo])
subject.final_value(memo)
expect(memo).to have_key(:duration_s)
expect(memo[:duration_s]).to eq(expected_duration)
end end
end end
end end
...@@ -191,6 +191,7 @@ RSpec.describe 'GitlabSchema configurations' do ...@@ -191,6 +191,7 @@ RSpec.describe 'GitlabSchema configurations' do
complexity: 181, complexity: 181,
depth: 13, depth: 13,
duration_s: 7, duration_s: 7,
operation_name: 'IntrospectionQuery',
used_fields: an_instance_of(Array), used_fields: an_instance_of(Array),
used_deprecated_fields: an_instance_of(Array) used_deprecated_fields: an_instance_of(Array)
} }
......
...@@ -13,6 +13,7 @@ RSpec.describe 'GraphQL' do ...@@ -13,6 +13,7 @@ RSpec.describe 'GraphQL' do
query_string: query, query_string: query,
variables: variables.to_s, variables: variables.to_s,
duration_s: anything, duration_s: anything,
operation_name: nil,
depth: 1, depth: 1,
complexity: 1, complexity: 1,
used_fields: ['Query.echo'], used_fields: ['Query.echo'],
......
...@@ -222,9 +222,12 @@ module GraphqlHelpers ...@@ -222,9 +222,12 @@ module GraphqlHelpers
lazy_vals.is_a?(Array) ? lazy_vals.map { |val| sync(val) } : sync(lazy_vals) lazy_vals.is_a?(Array) ? lazy_vals.map { |val| sync(val) } : sync(lazy_vals)
end end
def graphql_query_for(name, args = {}, selection = nil) def graphql_query_for(name, args = {}, selection = nil, operation_name = nil)
type = GitlabSchema.types['Query'].fields[GraphqlHelpers.fieldnamerize(name)]&.type type = GitlabSchema.types['Query'].fields[GraphqlHelpers.fieldnamerize(name)]&.type
wrap_query(query_graphql_field(name, args, selection, type)) query = wrap_query(query_graphql_field(name, args, selection, type))
query = "query #{operation_name}#{query}" if operation_name
query
end end
def wrap_query(query) def wrap_query(query)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment