From c5f039bfa01352832d99f5371165617fb4249937 Mon Sep 17 00:00:00 2001 From: Hercules Merscher Date: Mon, 6 Feb 2023 16:21:36 +0100 Subject: [PATCH 1/2] UNSTRUCTURED_RAILS_LOG disabled by default A few years ago, GitLab made the decision to switch to use structured logging. The flag UNSTRUCTURED_RAILS_LOG was then added to keep structured and unstructured log in use by lograge. Nowadays, nobody should be using unstructured logging at GitLab, and this flag will be now disabled by default. Eventually, this flag will be removed completely when confirmed that it's not impacting any deployment. Changelog: deprecated --- config/initializers/lograge.rb | 2 +- doc/administration/environment_variables.md | 2 +- lib/gitlab/app_logger.rb | 2 +- spec/lib/gitlab/app_logger_spec.rb | 25 ++++++++++++--------- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb index 61e357808d9b19..e5abc0b919bcfb 100644 --- a/config/initializers/lograge.rb +++ b/config/initializers/lograge.rb @@ -8,7 +8,7 @@ Rails.application.configure do config.lograge.enabled = true # Store the lograge JSON files in a separate file - config.lograge.keep_original_rails_log = Gitlab::Utils.to_boolean(ENV.fetch('UNSTRUCTURED_RAILS_LOG', 'true')) + config.lograge.keep_original_rails_log = Gitlab::Utils.to_boolean(ENV.fetch('UNSTRUCTURED_RAILS_LOG', 'false')) # Don't use the Logstash formatter since this requires logstash-event, an # unmaintained gem that monkey patches `Time` config.lograge.formatter = Lograge::Formatters::Json.new diff --git a/doc/administration/environment_variables.md b/doc/administration/environment_variables.md index d2edc3085f13bf..b6a3a563ae1fc0 100644 --- a/doc/administration/environment_variables.md +++ b/doc/administration/environment_variables.md @@ -35,7 +35,7 @@ You can use the following environment variables to override certain values: | `GITLAB_ROOT_PASSWORD` | string | Sets the password for the `root` user on installation. | | `GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN` | string | Sets the initial registration token used for runners. | | `RAILS_ENV` | string | The Rails environment; can be one of `production`, `development`, `staging`, or `test`. | -| `UNSTRUCTURED_RAILS_LOG` | string | Enables the unstructured log in addition to JSON logs (defaults to `true`). | +| `UNSTRUCTURED_RAILS_LOG` | string | Enables the unstructured log in addition to JSON logs (defaults to `false`). | | `GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS` | integer | The default TTL used for entries stored in the Rails-cache. Default is `28800`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95042) in 15.3. | ## Adding more variables diff --git a/lib/gitlab/app_logger.rb b/lib/gitlab/app_logger.rb index a39e7f318869f7..40bdc5385940a3 100644 --- a/lib/gitlab/app_logger.rb +++ b/lib/gitlab/app_logger.rb @@ -5,7 +5,7 @@ class AppLogger < Gitlab::MultiDestinationLogger LOGGERS = [Gitlab::AppTextLogger, Gitlab::AppJsonLogger].freeze def self.loggers - if Gitlab::Utils.to_boolean(ENV.fetch('UNSTRUCTURED_RAILS_LOG', 'true')) + if Gitlab::Utils.to_boolean(ENV.fetch('UNSTRUCTURED_RAILS_LOG', 'false')) LOGGERS else [Gitlab::AppJsonLogger] diff --git a/spec/lib/gitlab/app_logger_spec.rb b/spec/lib/gitlab/app_logger_spec.rb index 85ca60d539f926..e3415f4ad8ca5a 100644 --- a/spec/lib/gitlab/app_logger_spec.rb +++ b/spec/lib/gitlab/app_logger_spec.rb @@ -5,22 +5,27 @@ RSpec.describe Gitlab::AppLogger do subject { described_class } - it 'builds two Logger instances' do - expect(Gitlab::Logger).to receive(:new).and_call_original - expect(Gitlab::JsonLogger).to receive(:new).and_call_original + context 'when UNSTRUCTURED_RAILS_LOG is enabled' do + before do + stub_env('UNSTRUCTURED_RAILS_LOG', 'true') + end - subject.info('Hello World!') - end + it 'builds two Logger instances' do + expect(Gitlab::Logger).to receive(:new).and_call_original + expect(Gitlab::JsonLogger).to receive(:new).and_call_original - it 'logs info to AppLogger and AppJsonLogger' do - expect_any_instance_of(Gitlab::AppTextLogger).to receive(:info).and_call_original - expect_any_instance_of(Gitlab::AppJsonLogger).to receive(:info).and_call_original + subject.info('Hello World!') + end - subject.info('Hello World!') + it 'logs info to AppLogger and AppJsonLogger' do + expect_any_instance_of(Gitlab::AppTextLogger).to receive(:info).and_call_original + expect_any_instance_of(Gitlab::AppJsonLogger).to receive(:info).and_call_original + + subject.info('Hello World!') + end end it 'logs info to only the AppJsonLogger when unstructured logs are disabled' do - stub_env('UNSTRUCTURED_RAILS_LOG', 'false') expect_any_instance_of(Gitlab::AppTextLogger).not_to receive(:info).and_call_original expect_any_instance_of(Gitlab::AppJsonLogger).to receive(:info).and_call_original -- GitLab From 50d6107a21cd44d567a916a7af7d2842bc197579 Mon Sep 17 00:00:00 2001 From: Hercules Merscher Date: Mon, 20 Feb 2023 18:21:51 +0100 Subject: [PATCH 2/2] Lograge's idiosyncrasy --- spec/lib/gitlab/etag_caching/middleware_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/lib/gitlab/etag_caching/middleware_spec.rb b/spec/lib/gitlab/etag_caching/middleware_spec.rb index fa0b3d1c6ddfab..d25511843ffd94 100644 --- a/spec/lib/gitlab/etag_caching/middleware_spec.rb +++ b/spec/lib/gitlab/etag_caching/middleware_spec.rb @@ -145,8 +145,11 @@ expect(payload[:headers].env['HTTP_IF_NONE_MATCH']).to eq('W/"123"') end - it 'log subscriber processes action' do - expect_any_instance_of(ActionController::LogSubscriber).to receive(:process_action) + it "publishes process_action.action_controller event to be picked up by lograge's subscriber" do + # Lograge unhooks the default Rails subscriber (ActionController::LogSubscriber) + # and replaces with its own (Lograge::LogSubscribers::ActionController). + # When `lograge.keep_original_rails_log = true`, ActionController::LogSubscriber is kept. + expect_any_instance_of(Lograge::LogSubscribers::ActionController).to receive(:process_action) .with(instance_of(ActiveSupport::Notifications::Event)) .and_call_original -- GitLab