diff options
| -rw-r--r-- | changelogs/unreleased/33338-notify-user-if-they-have-over-1000-epics-in-roadmap.yml | 5 | ||||
| -rw-r--r-- | changelogs/unreleased/sh-flatten-json-exceptions.yml | 5 | ||||
| -rw-r--r-- | config/initializers/lograge.rb | 11 | ||||
| -rw-r--r-- | doc/administration/logs.md | 26 | ||||
| -rw-r--r-- | lib/gitlab/exception_log_formatter.rb | 20 | ||||
| -rw-r--r-- | lib/gitlab/grape_logging/loggers/exception_logger.rb | 14 | ||||
| -rw-r--r-- | locale/gitlab.pot | 3 | ||||
| -rw-r--r-- | spec/initializers/lograge_spec.rb | 6 | ||||
| -rw-r--r-- | spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb | 8 |
9 files changed, 55 insertions, 43 deletions
diff --git a/changelogs/unreleased/33338-notify-user-if-they-have-over-1000-epics-in-roadmap.yml b/changelogs/unreleased/33338-notify-user-if-they-have-over-1000-epics-in-roadmap.yml new file mode 100644 index 00000000000..ad3033305b8 --- /dev/null +++ b/changelogs/unreleased/33338-notify-user-if-they-have-over-1000-epics-in-roadmap.yml @@ -0,0 +1,5 @@ +--- +title: Notify user when over 1000 epics in roadmap +merge_request: 19419 +author: +type: added diff --git a/changelogs/unreleased/sh-flatten-json-exceptions.yml b/changelogs/unreleased/sh-flatten-json-exceptions.yml new file mode 100644 index 00000000000..a312748d491 --- /dev/null +++ b/changelogs/unreleased/sh-flatten-json-exceptions.yml @@ -0,0 +1,5 @@ +--- +title: Flatten exception details in API and controller logs +merge_request: 20434 +author: +type: changed diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index 769ef2af0e7..a8207862739 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -44,16 +44,7 @@ unless Sidekiq.server? # https://github.com/roidrage/lograge#logging-errors--exceptions exception = event.payload[:exception_object] - if exception - payload[:exception] = { - class: exception.class.name, - message: exception.message - } - - if exception.backtrace - payload[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(exception.backtrace) - end - end + ::Gitlab::ExceptionLogFormatter.format!(exception, payload) payload end diff --git a/doc/administration/logs.md b/doc/administration/logs.md index aa10cdd220c..e1910b0b3f3 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -67,20 +67,18 @@ NOTE: **Note:** Starting with GitLab 12.5, if an error occurs, an "queue_duration": 274.35, "correlation_id": "KjDVUhNvvV3", "cpu_s": 2.837645135999999, - "exception": { - "class": "NameError", - "message": "undefined local variable or method `adsf' for #<Admin::DashboardController:0x00007ff3c9648588>", - "backtrace": [ - "app/controllers/admin/dashboard_controller.rb:11:in `index'", - "ee/app/controllers/ee/admin/dashboard_controller.rb:14:in `index'", - "ee/lib/gitlab/ip_address_state.rb:10:in `with'", - "ee/app/controllers/ee/application_controller.rb:43:in `set_current_ip_address'", - "lib/gitlab/session.rb:11:in `with_session'", - "app/controllers/application_controller.rb:450:in `set_session_storage'", - "app/controllers/application_controller.rb:444:in `set_locale'", - "ee/lib/gitlab/jira/middleware.rb:19:in `call'" - ] - } + "exception.class": "NameError", + "exception.message": "undefined local variable or method `adsf' for #<Admin::DashboardController:0x00007ff3c9648588>", + "exception.backtrace": [ + "app/controllers/admin/dashboard_controller.rb:11:in `index'", + "ee/app/controllers/ee/admin/dashboard_controller.rb:14:in `index'", + "ee/lib/gitlab/ip_address_state.rb:10:in `with'", + "ee/app/controllers/ee/application_controller.rb:43:in `set_current_ip_address'", + "lib/gitlab/session.rb:11:in `with_session'", + "app/controllers/application_controller.rb:450:in `set_session_storage'", + "app/controllers/application_controller.rb:444:in `set_locale'", + "ee/lib/gitlab/jira/middleware.rb:19:in `call'" + ] } ``` diff --git a/lib/gitlab/exception_log_formatter.rb b/lib/gitlab/exception_log_formatter.rb new file mode 100644 index 00000000000..e0de0219294 --- /dev/null +++ b/lib/gitlab/exception_log_formatter.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Gitlab + module ExceptionLogFormatter + def self.format!(exception, payload) + return unless exception + + # Elasticsearch/Fluentd don't handle nested structures well. + # Use periods to flatten the fields. + payload.merge!( + 'exception.class' => exception.class.name, + 'exception.message' => exception.message + ) + + if exception.backtrace + payload['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(exception.backtrace) + end + end + end +end diff --git a/lib/gitlab/grape_logging/loggers/exception_logger.rb b/lib/gitlab/grape_logging/loggers/exception_logger.rb index 022eb15d28d..606b7c0dbce 100644 --- a/lib/gitlab/grape_logging/loggers/exception_logger.rb +++ b/lib/gitlab/grape_logging/loggers/exception_logger.rb @@ -11,19 +11,11 @@ module Gitlab # precedence so the logger never sees it. We need to # store and retrieve the exception from the environment. exception = request.env[::API::Helpers::API_EXCEPTION_ENV] + data = {} - return {} unless exception.is_a?(Exception) + return data unless exception.is_a?(Exception) - data = { - exception: { - class: exception.class.to_s, - message: exception.message - } - } - - if exception.backtrace - data[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(exception.backtrace) - end + Gitlab::ExceptionLogFormatter.format!(exception, data) data end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0ef14bf8ed1..909c0bdcacd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15989,6 +15989,9 @@ msgstr "" msgid "Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead." msgstr "" +msgid "Some of your epics may not be visible. A roadmap is limited to the first 1,000 epics, in your selected sort order." +msgstr "" + msgid "Someone edited the issue at the same time you did. Please check out %{linkStart}the issue%{linkEnd} and make sure your changes will not unintentionally remove theirs." msgstr "" diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb index 9267231390d..5dd296b6040 100644 --- a/spec/initializers/lograge_spec.rb +++ b/spec/initializers/lograge_spec.rb @@ -110,9 +110,9 @@ describe 'lograge', type: :request do log_data = JSON.parse(log_output.string) - expect(log_data['exception']['class']).to eq('RuntimeError') - expect(log_data['exception']['message']).to eq('bad request') - expect(log_data['exception']['backtrace']).to eq(Gitlab::Profiler.clean_backtrace(backtrace)) + expect(log_data['exception.class']).to eq('RuntimeError') + expect(log_data['exception.message']).to eq('bad request') + expect(log_data['exception.backtrace']).to eq(Gitlab::Profiler.clean_backtrace(backtrace)) end end end diff --git a/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb index 8d7826c0a56..e21af023bb8 100644 --- a/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb +++ b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb @@ -24,10 +24,8 @@ describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do let(:expected) do { - exception: { - class: 'RuntimeError', - message: 'This is a test' - } + 'exception.class' => 'RuntimeError', + 'exception.message' => 'This is a test' } end @@ -39,7 +37,7 @@ describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do before do current_backtrace = caller allow(exception).to receive(:backtrace).and_return(current_backtrace) - expected[:exception][:backtrace] = Gitlab::Profiler.clean_backtrace(current_backtrace) + expected['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(current_backtrace) end it 'includes the backtrace' do |