Merge branch 'sh-standardize-log-names' into 'master'

Standardize remote_ip and path keys for auth.log and api_json.log

Closes #66167

See merge request gitlab-org/gitlab-ce!31921

10 files changed, 36 insertions, 20 deletions

diff --git a/app/controllers/concerns/invisible_captcha.rb b/app/controllers/concerns/invisible_captcha.rb
index c9f66e5c194..45c0a5c58ef 100644
--- a/app/controllers/concerns/invisible_captcha.rb
+++ b/app/controllers/concerns/invisible_captcha.rb
@@ -41,9 +41,9 @@ module InvisibleCaptcha
     request_information = {
       message: message,
       env: :invisible_captcha_signup_bot_detected,
-      ip: request.ip,
+      remote_ip: request.ip,
       request_method: request.request_method,
-      fullpath: request.fullpath
+      path: request.fullpath
     }
 
     Gitlab::AuthLogger.error(request_information)
diff --git a/config/initializers/rack_attack_logging.rb b/config/initializers/rack_attack_logging.rb
index 7eb34bd69e5..b43fff24bb0 100644
--- a/config/initializers/rack_attack_logging.rb
+++ b/config/initializers/rack_attack_logging.rb
@@ -7,9 +7,9 @@ ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, r
     rack_attack_info = {
       message: 'Rack_Attack',
       env: req.env['rack.attack.match_type'],
-      ip: req.ip,
+      remote_ip: req.ip,
       request_method: req.request_method,
-      fullpath: req.fullpath
+      path: req.fullpath
     }
 
     if %w(throttle_authenticated_api throttle_authenticated_web).include? req.env['rack.attack.matched']
diff --git a/doc/administration/logs.md b/doc/administration/logs.md
index a57ef8ddc7d..9030c941cad 100644
--- a/doc/administration/logs.md
+++ b/doc/administration/logs.md
@@ -88,7 +88,7 @@ Introduced in GitLab 10.0, this file lives in
 It helps you see requests made directly to the API. For example:
 
 ```json
-{"time":"2018-10-29T12:49:42.123Z","severity":"INFO","duration":709.08,"db":14.59,"view":694.49,"status":200,"method":"GET","path":"/api/v4/projects","params":[{"key":"action","value":"git-upload-pack"},{"key":"changes","value":"_any"},{"key":"key_id","value":"secret"},{"key":"secret_token","value":"[FILTERED]"}],"host":"localhost","ip":"::1","ua":"Ruby","route":"/api/:version/projects","user_id":1,"username":"root","queue_duration":100.31,"gitaly_calls":30,"gitaly_duration":5.36}
+{"time":"2018-10-29T12:49:42.123Z","severity":"INFO","duration":709.08,"db":14.59,"view":694.49,"status":200,"method":"GET","path":"/api/v4/projects","params":[{"key":"action","value":"git-upload-pack"},{"key":"changes","value":"_any"},{"key":"key_id","value":"secret"},{"key":"secret_token","value":"[FILTERED]"}],"host":"localhost","remote_ip":"::1","ua":"Ruby","route":"/api/:version/projects","user_id":1,"username":"root","queue_duration":100.31,"gitaly_calls":30,"gitaly_duration":5.36}
 ```
 
 This entry above shows an access to an internal endpoint to check whether an
diff --git a/lib/api/api.rb b/lib/api/api.rb
index e500a93b31e..219ed45eff6 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -18,7 +18,7 @@ module API
                   formatter: Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new,
                   include: [
                     GrapeLogging::Loggers::FilterParameters.new(LOG_FILTERS),
-                    GrapeLogging::Loggers::ClientEnv.new,
+                    Gitlab::GrapeLogging::Loggers::ClientEnvLogger.new,
                     Gitlab::GrapeLogging::Loggers::RouteLogger.new,
                     Gitlab::GrapeLogging::Loggers::UserLogger.new,
                     Gitlab::GrapeLogging::Loggers::QueueDurationLogger.new,
diff --git a/lib/gitlab/action_rate_limiter.rb b/lib/gitlab/action_rate_limiter.rb
index fdb06d00548..0e8707af631 100644
--- a/lib/gitlab/action_rate_limiter.rb
+++ b/lib/gitlab/action_rate_limiter.rb
@@ -49,9 +49,9 @@ module Gitlab
       request_information = {
         message: 'Action_Rate_Limiter_Request',
         env: type,
-        ip: request.ip,
+        remote_ip: request.ip,
         request_method: request.request_method,
-        fullpath: request.fullpath
+        path: request.fullpath
       }
 
       if current_user
diff --git a/lib/gitlab/grape_logging/loggers/client_env_logger.rb b/lib/gitlab/grape_logging/loggers/client_env_logger.rb
new file mode 100644
index 00000000000..3acc6f6a2ef
--- /dev/null
+++ b/lib/gitlab/grape_logging/loggers/client_env_logger.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+# This is a fork of
+# https://github.com/aserafin/grape_logging/blob/master/lib/grape_logging/loggers/client_env.rb
+# to use remote_ip instead of ip.
+module Gitlab
+  module GrapeLogging
+    module Loggers
+      class ClientEnvLogger < ::GrapeLogging::Loggers::Base
+        def parameters(request, _)
+          { remote_ip: request.env["HTTP_X_FORWARDED_FOR"] || request.env["REMOTE_ADDR"], ua: request.env["HTTP_USER_AGENT"] }
+        end
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb
index b958f419a19..8b43d1264b2 100644
--- a/spec/controllers/projects/raw_controller_spec.rb
+++ b/spec/controllers/projects/raw_controller_spec.rb
@@ -67,9 +67,9 @@ describe Projects::RawController do
         attributes = {
           message: 'Action_Rate_Limiter_Request',
           env: :raw_blob_request_limit,
-          ip: '0.0.0.0',
+          remote_ip: '0.0.0.0',
           request_method: 'GET',
-          fullpath: "/#{project.full_path}/raw/#{file_path}"
+          path: "/#{project.full_path}/raw/#{file_path}"
         }
 
         expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb
index fed4fc810f2..35487682462 100644
--- a/spec/controllers/registrations_controller_spec.rb
+++ b/spec/controllers/registrations_controller_spec.rb
@@ -129,9 +129,9 @@ describe RegistrationsController do
         {
           message: auth_log_message,
           env: :invisible_captcha_signup_bot_detected,
-          ip: '0.0.0.0',
+          remote_ip: '0.0.0.0',
           request_method: 'POST',
-          fullpath: '/users'
+          path: '/users'
         }
       end
 
diff --git a/spec/lib/gitlab/action_rate_limiter_spec.rb b/spec/lib/gitlab/action_rate_limiter_spec.rb
index 8dbad32dfb4..8b510a475d2 100644
--- a/spec/lib/gitlab/action_rate_limiter_spec.rb
+++ b/spec/lib/gitlab/action_rate_limiter_spec.rb
@@ -74,9 +74,9 @@ describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do
       {
         message: 'Action_Rate_Limiter_Request',
         env: type,
-        ip: '127.0.0.1',
+        remote_ip: '127.0.0.1',
         request_method: 'GET',
-        fullpath: fullpath
+        path: fullpath
       }
     end
 
diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb
index d832963292c..478f09a7881 100644
--- a/spec/requests/rack_attack_global_spec.rb
+++ b/spec/requests/rack_attack_global_spec.rb
@@ -112,9 +112,9 @@ describe 'Rack Attack global throttles' do
         arguments = {
           message: 'Rack_Attack',
           env: :throttle,
-          ip: '127.0.0.1',
+          remote_ip: '127.0.0.1',
           request_method: 'GET',
-          fullpath: get_args.first,
+          path: get_args.first,
           user_id: user.id,
           username: user.username
         }
@@ -213,9 +213,9 @@ describe 'Rack Attack global throttles' do
         arguments = {
           message: 'Rack_Attack',
           env: :throttle,
-          ip: '127.0.0.1',
+          remote_ip: '127.0.0.1',
           request_method: 'GET',
-          fullpath: '/users/sign_in'
+          path: '/users/sign_in'
         }
 
         expect(Gitlab::AuthLogger).to receive(:error).with(arguments)
@@ -377,9 +377,9 @@ describe 'Rack Attack global throttles' do
         arguments = {
           message: 'Rack_Attack',
           env: :throttle,
-          ip: '127.0.0.1',
+          remote_ip: '127.0.0.1',
           request_method: 'GET',
-          fullpath: '/dashboard/snippets',
+          path: '/dashboard/snippets',
           user_id: user.id,
           username: user.username
         }