From cc55bd028782baa701d715c2cb50e5d0aacbdaed Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 5 Mar 2020 14:29:30 +0100 Subject: [PATCH 1/2] Track exceptions for namespace statistics This tracks exceptions for updating namespace statistics in Sentry instead of logs. --- app/workers/namespaces/root_statistics_worker.rb | 8 +------- app/workers/namespaces/schedule_aggregation_worker.rb | 8 ++------ spec/workers/namespaces/root_statistics_worker_spec.rb | 2 +- .../namespaces/schedule_aggregation_worker_spec.rb | 2 +- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/app/workers/namespaces/root_statistics_worker.rb b/app/workers/namespaces/root_statistics_worker.rb index 5fceeb8e03d3..70b2510488bc 100644 --- a/app/workers/namespaces/root_statistics_worker.rb +++ b/app/workers/namespaces/root_statistics_worker.rb @@ -16,13 +16,7 @@ def perform(namespace_id) namespace.aggregation_schedule.destroy rescue ::Namespaces::StatisticsRefresherService::RefresherError, ActiveRecord::RecordNotFound => ex - log_error(namespace.full_path, ex.message) if namespace - end - - private - - def log_error(namespace_path, error_message) - Gitlab::SidekiqLogger.error("Namespace statistics can't be updated for #{namespace_path}: #{error_message}") + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id, namespace: namespace&.full_path) end end end diff --git a/app/workers/namespaces/schedule_aggregation_worker.rb b/app/workers/namespaces/schedule_aggregation_worker.rb index 60210e2f5a77..94343a9e3781 100644 --- a/app/workers/namespaces/schedule_aggregation_worker.rb +++ b/app/workers/namespaces/schedule_aggregation_worker.rb @@ -16,8 +16,8 @@ def perform(namespace_id) return if root_ancestor.aggregation_scheduled? Namespace::AggregationSchedule.safe_find_or_create_by!(namespace_id: root_ancestor.id) - rescue ActiveRecord::RecordNotFound - log_error(namespace_id) + rescue ActiveRecord::RecordNotFound => ex + Gitlab::ErrorTracking.track_exception(ex, namespace_id: namespace_id) end private @@ -34,9 +34,5 @@ def aggregation_schedules_table_exists? Namespace::AggregationSchedule.table_exists? end - - def log_error(root_ancestor_id) - Gitlab::SidekiqLogger.error("Namespace can't be scheduled for aggregation: #{root_ancestor_id} does not exist") - end end end diff --git a/spec/workers/namespaces/root_statistics_worker_spec.rb b/spec/workers/namespaces/root_statistics_worker_spec.rb index 6bbdfe03ceb3..45e75c9b0dab 100644 --- a/spec/workers/namespaces/root_statistics_worker_spec.rb +++ b/spec/workers/namespaces/root_statistics_worker_spec.rb @@ -42,7 +42,7 @@ allow_any_instance_of(Namespace::AggregationSchedule) .to receive(:schedule_root_storage_statistics).and_return(nil) - expect(Gitlab::SidekiqLogger).to receive(:error).once + expect(Gitlab::ErrorTracking).to receive(:track_exception).once worker.perform(group.id) end diff --git a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb index be722f451e02..a2ce9891b5f1 100644 --- a/spec/workers/namespaces/schedule_aggregation_worker_spec.rb +++ b/spec/workers/namespaces/schedule_aggregation_worker_spec.rb @@ -48,7 +48,7 @@ context 'when namespace does not exist' do it 'logs the error' do - expect(Gitlab::SidekiqLogger).to receive(:error).once + expect(Gitlab::ErrorTracking).to receive(:track_exception).once worker.perform(12345) end -- GitLab From 4708bf84c1fe4e8853ea2a6d9df7be6b245f2dad Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 5 Mar 2020 16:55:44 +0100 Subject: [PATCH 2/2] Add a sidekiq_client.log logfile Sidekiq client, in our case Rails, can also emit log messages related to sidekiq jobs. Instead of having these messages printed to STDOUT (which results in `production.log`), this changes the Sidekiq.logger to a separate writing to `log/sidekiq_client`. This allows us to ingest these messages in the same index as the messages the sidekiq process emits. Having them in the same index allows us to gather statistics about jobs that haven't been processed. --- config/initializers/sidekiq.rb | 5 +++++ doc/administration/logs.md | 15 +++++++++++++++ lib/gitlab/sidekiq_logging/client_logger.rb | 11 +++++++++++ 3 files changed, 31 insertions(+) create mode 100644 lib/gitlab/sidekiq_logging/client_logger.rb diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb index 88e198116a01..fa4fc2d2c7b6 100644 --- a/config/initializers/sidekiq.rb +++ b/config/initializers/sidekiq.rb @@ -108,6 +108,11 @@ def enable_semi_reliable_fetch_mode? Sidekiq.configure_client do |config| config.redis = queues_config_hash + # We only need to do this for other clients. If Sidekiq-server is the + # client scheduling jobs, we have access to the regular sidekiq logger that + # writes to STDOUT + Sidekiq.logger = Gitlab::SidekiqLogging::ClientLogger.build + Sidekiq.logger.formatter = Gitlab::SidekiqLogging::JSONFormatter.new if enable_json_logs config.client_middleware(&Gitlab::SidekiqMiddleware.client_configurator) end diff --git a/doc/administration/logs.md b/doc/administration/logs.md index e45e39c46517..222d79ddc35b 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -400,6 +400,21 @@ For source installations, edit the `gitlab.yml` and set the Sidekiq log_format: json ``` +## `sidekiq_client.log` + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26586) in GitLab 12.9. + +This file lives in `/var/log/gitlab/gitlab-rails/sidekiq_client.log` for +Omnibus GitLab packages or in `/home/git/gitlab/log/sidekiq_client.log` for +installations from source. + +This file contains logging information about jobs before they are start +being processed by Sidekiq, for example before being enqueued. + +This logfile follows the same structure as +[`sidekiq.log`](#sidekiqlog), so it will be structured as JSON if +you've configured this for Sidekiq as mentioned above. + ## `gitlab-shell.log` This file lives in `/var/log/gitlab/gitaly/gitlab-shell.log` for diff --git a/lib/gitlab/sidekiq_logging/client_logger.rb b/lib/gitlab/sidekiq_logging/client_logger.rb new file mode 100644 index 000000000000..8be755a55db2 --- /dev/null +++ b/lib/gitlab/sidekiq_logging/client_logger.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Gitlab + module SidekiqLogging + class ClientLogger < Gitlab::Logger + def self.file_name_noext + 'sidekiq_client' + end + end + end +end -- GitLab