From af811bc379e87581e07d87396fa699ffc7dcea3f Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Thu, 9 Feb 2023 16:41:39 +0100 Subject: [PATCH 1/4] Update gitlab-labkit to v0.30.1 For gitlab-rails, the only relevant change is that we now reject log keys in test and dev envs if we know they would be overwritten once these log events leave the rails app. --- Gemfile | 2 +- Gemfile.checksum | 2 +- Gemfile.lock | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index 9f398ec2836c84..b2371f31105c01 100644 --- a/Gemfile +++ b/Gemfile @@ -348,7 +348,7 @@ gem 'pg_query', '~> 2.2', '>= 2.2.1' gem 'premailer-rails', '~> 1.10.3' -gem 'gitlab-labkit', '~> 0.29.0' +gem 'gitlab-labkit', '~> 0.30.1' gem 'thrift', '>= 0.16.0' # I18n diff --git a/Gemfile.checksum b/Gemfile.checksum index 41b0e8a3d8bf51..0dc38fdb8fe5a2 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -203,7 +203,7 @@ {"name":"gitlab-dangerfiles","version":"3.6.7","platform":"ruby","checksum":"ebd898ec0e8ed3edea281b2f703000c502c6b412cbcadc1265ddbc31ffb0c579"}, {"name":"gitlab-experiment","version":"0.7.1","platform":"ruby","checksum":"166dddb3aa83428bcaa93c35684ed01dc4d61f321fd2ae40b020806dc54a7824"}, {"name":"gitlab-fog-azure-rm","version":"1.4.0","platform":"ruby","checksum":"af4163c32b028aa5208814a3f4765a5817d50527e6c61931f766bf18a2e0eb7e"}, -{"name":"gitlab-labkit","version":"0.29.0","platform":"ruby","checksum":"eb19ac5c11698683775ab847a3441d7af87d72fbaec38d635149fb65c5d9b427"}, +{"name":"gitlab-labkit","version":"0.30.1","platform":"ruby","checksum":"bdedbd86014c83dfd6a50d20dbc1709697bba2bb9e3666383e5f28cbd312b113"}, {"name":"gitlab-license","version":"2.2.1","platform":"ruby","checksum":"39fcf6be8b2887df8afe01b5dcbae8d08b7c5d937ff56b0fb40484a8c4f02d30"}, {"name":"gitlab-mail_room","version":"0.0.9","platform":"ruby","checksum":"6700374b5c0aa9d9ad4e711aeb677f0b7d415a6d01d3baa699efab25349d851c"}, {"name":"gitlab-markup","version":"1.8.1","platform":"ruby","checksum":"ab1f9fd016977497c2af25b76341dea670533014f406861834a0bd99f646707b"}, diff --git a/Gemfile.lock b/Gemfile.lock index a6df239f1a2271..21966ce37c5849 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -584,7 +584,7 @@ GEM fog-json (~> 1.2.0) mime-types ms_rest_azure (~> 0.12.0) - gitlab-labkit (0.29.0) + gitlab-labkit (0.30.1) actionpack (>= 5.0.0, < 8.0.0) activesupport (>= 5.0.0, < 8.0.0) grpc (>= 1.37) @@ -1679,7 +1679,7 @@ DEPENDENCIES gitlab-dangerfiles (~> 3.6.7) gitlab-experiment (~> 0.7.1) gitlab-fog-azure-rm (~> 1.4.0) - gitlab-labkit (~> 0.29.0) + gitlab-labkit (~> 0.30.1) gitlab-license (~> 2.2.1) gitlab-mail_room (~> 0.0.9) gitlab-markup (~> 1.8.0) -- GitLab From f2a394e58b6a5e1dd44065648b455dc7e76f0d5a Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Mon, 13 Feb 2023 15:12:17 +0100 Subject: [PATCH 2/4] Prefix BitBucket importer stage log key --- .../bitbucket_server_import/importer.rb | 42 +++++++++---------- .../bitbucket_server_import/importer_spec.rb | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index 242979da367ac8..ea9b79c12fd6df 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -55,7 +55,7 @@ def execute handle_errors metrics.track_finished_import - log_info(stage: "complete") + log_info(import_stage: "complete") Gitlab::Cache::Import::Caching.expire(already_imported_cache_key, Gitlab::Cache::Import::Caching::SHORTER_TIMEOUT) true @@ -139,16 +139,16 @@ def restore_branch_shas(shas_to_restore) end def import_repository - log_info(stage: 'import_repository', message: 'starting import') + log_info(import_stage: 'import_repository', message: 'starting import') project.repository.import_repository(project.import_url) project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap) - log_info(stage: 'import_repository', message: 'finished import') + log_info(import_stage: 'import_repository', message: 'finished import') rescue ::Gitlab::Git::CommandError => e Gitlab::ErrorTracking.log_exception( e, - stage: 'import_repository', message: 'failed import', error: e.message + import_stage: 'import_repository', message: 'failed import', error: e.message ) # Expire cache to prevent scenarios such as: @@ -179,10 +179,10 @@ def download_lfs_objects def import_pull_requests page = 0 - log_info(stage: 'import_pull_requests', message: "starting") + log_info(import_stage: 'import_pull_requests', message: "starting") loop do - log_debug(stage: 'import_pull_requests', message: "importing page #{page} and batch-size #{BATCH_SIZE} from #{page * BATCH_SIZE} to #{(page + 1) * BATCH_SIZE}") + log_debug(import_stage: 'import_pull_requests', message: "importing page #{page} and batch-size #{BATCH_SIZE} from #{page * BATCH_SIZE} to #{(page + 1) * BATCH_SIZE}") pull_requests = client.pull_requests(project_key, repository_slug, page_offset: page, limit: BATCH_SIZE).to_a @@ -196,21 +196,21 @@ def import_pull_requests pull_requests.each do |pull_request| if already_imported?(pull_request) - log_info(stage: 'import_pull_requests', message: 'already imported', iid: pull_request.iid) + log_info(import_stage: 'import_pull_requests', message: 'already imported', iid: pull_request.iid) else import_bitbucket_pull_request(pull_request) end rescue StandardError => e Gitlab::ErrorTracking.log_exception( e, - stage: 'import_pull_requests', iid: pull_request.iid, error: e.message + import_stage: 'import_pull_requests', iid: pull_request.iid, error: e.message ) backtrace = Gitlab::BacktraceCleaner.clean_backtrace(e.backtrace) errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, backtrace: backtrace.join("\n"), raw_response: pull_request.raw } end - log_debug(stage: 'import_pull_requests', message: "finished page #{page} and batch-size #{BATCH_SIZE}") + log_debug(import_stage: 'import_pull_requests', message: "finished page #{page} and batch-size #{BATCH_SIZE}") page += 1 end end @@ -235,7 +235,7 @@ def delete_temp_branches rescue BitbucketServer::Connection::ConnectionError => e Gitlab::ErrorTracking.log_exception( e, - stage: 'delete_temp_branches', branch: branch.name, error: e.message + import_stage: 'delete_temp_branches', branch: branch.name, error: e.message ) @errors << { type: :delete_temp_branches, branch_name: branch.name, errors: e.message } @@ -243,7 +243,7 @@ def delete_temp_branches end def import_bitbucket_pull_request(pull_request) - log_info(stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid) + log_info(import_stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid) description = '' description += author_line(pull_request) @@ -274,12 +274,12 @@ def import_bitbucket_pull_request(pull_request) metrics.merge_requests_counter.increment end - log_info(stage: 'import_bitbucket_pull_requests', message: 'finished', iid: pull_request.iid) + log_info(import_stage: 'import_bitbucket_pull_requests', message: 'finished', iid: pull_request.iid) mark_as_imported(pull_request) end def import_pull_request_comments(pull_request, merge_request) - log_info(stage: 'import_pull_request_comments', message: 'starting', iid: merge_request.iid) + log_info(import_stage: 'import_pull_request_comments', message: 'starting', iid: merge_request.iid) comments, other_activities = client.activities(project_key, repository_slug, pull_request.iid).partition(&:comment?) @@ -291,7 +291,7 @@ def import_pull_request_comments(pull_request, merge_request) import_inline_comments(inline_comments.map(&:comment), merge_request) import_standalone_pr_comments(pr_comments.map(&:comment), merge_request) - log_info(stage: 'import_pull_request_comments', message: 'finished', iid: merge_request.iid, + log_info(import_stage: 'import_pull_request_comments', message: 'finished', iid: merge_request.iid, merge_event_found: merge_event.present?, inline_comments_count: inline_comments.count, standalone_pr_comments: pr_comments.count) @@ -299,7 +299,7 @@ def import_pull_request_comments(pull_request, merge_request) # rubocop: disable CodeReuse/ActiveRecord def import_merge_event(merge_request, merge_event) - log_info(stage: 'import_merge_event', message: 'starting', iid: merge_request.iid) + log_info(import_stage: 'import_merge_event', message: 'starting', iid: merge_request.iid) committer = merge_event.committer_email @@ -309,12 +309,12 @@ def import_merge_event(merge_request, merge_event) metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request) metric.update(merged_by_id: user_id, merged_at: timestamp) - log_info(stage: 'import_merge_event', message: 'finished', iid: merge_request.iid) + log_info(import_stage: 'import_merge_event', message: 'finished', iid: merge_request.iid) end # rubocop: enable CodeReuse/ActiveRecord def import_inline_comments(inline_comments, merge_request) - log_info(stage: 'import_inline_comments', message: 'starting', iid: merge_request.iid) + log_info(import_stage: 'import_inline_comments', message: 'starting', iid: merge_request.iid) inline_comments.each do |comment| position = build_position(merge_request, comment) @@ -329,7 +329,7 @@ def import_inline_comments(inline_comments, merge_request) end end - log_info(stage: 'import_inline_comments', message: 'finished', iid: merge_request.iid) + log_info(import_stage: 'import_inline_comments', message: 'finished', iid: merge_request.iid) end def create_diff_note(merge_request, comment, position, discussion_id = nil) @@ -344,7 +344,7 @@ def create_diff_note(merge_request, comment, position, discussion_id = nil) return note end - log_info(stage: 'create_diff_note', message: 'creating fallback DiffNote', iid: merge_request.iid) + log_info(import_stage: 'create_diff_note', message: 'creating fallback DiffNote', iid: merge_request.iid) # Bitbucket Server supports the ability to comment on any line, not just the # line in the diff. If we can't add the note as a DiffNote, fallback to creating @@ -353,7 +353,7 @@ def create_diff_note(merge_request, comment, position, discussion_id = nil) rescue StandardError => e Gitlab::ErrorTracking.log_exception( e, - stage: 'create_diff_note', comment_id: comment.id, error: e.message + import_stage: 'create_diff_note', comment_id: comment.id, error: e.message ) errors << { type: :pull_request, id: comment.id, errors: e.message } @@ -394,7 +394,7 @@ def import_standalone_pr_comments(pr_comments, merge_request) rescue StandardError => e Gitlab::ErrorTracking.log_exception( e, - stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message + import_stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message ) errors << { type: :pull_request, comment_id: comment.id, errors: e.message } diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index ab4be5a909a5d1..3cff241105426f 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::BitbucketServerImport::Importer do +RSpec.describe Gitlab::BitbucketServerImport::Importer, feature_category: :importers do include ImportSpecHelper let(:import_url) { 'http://my-bitbucket' } -- GitLab From 2a4b34442e7a4e3cc57a8c8bc03d8499084d797a Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Mon, 13 Feb 2023 15:15:59 +0100 Subject: [PATCH 3/4] Rename Geo log cursor host log key --- .rubocop_todo/rspec/missing_feature_category.yml | 1 - ee/lib/gitlab/geo/log_cursor/logger.rb | 2 +- ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb | 4 ++-- ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb | 6 +++--- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 325ded0684189d..4d775d512c625e 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -1029,7 +1029,6 @@ RSpec/MissingFeatureCategory: - 'ee/spec/lib/gitlab/geo/log_cursor/events/repository_updated_event_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/events/reset_checksum_event_spec.rb' - 'ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb' - - 'ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb' - 'ee/spec/lib/gitlab/geo/log_helpers_spec.rb' - 'ee/spec/lib/gitlab/geo/logger_spec.rb' - 'ee/spec/lib/gitlab/geo/oauth/login_state_spec.rb' diff --git a/ee/lib/gitlab/geo/log_cursor/logger.rb b/ee/lib/gitlab/geo/log_cursor/logger.rb index 71bcedad233294..b4080ca6679d08 100644 --- a/ee/lib/gitlab/geo/log_cursor/logger.rb +++ b/ee/lib/gitlab/geo/log_cursor/logger.rb @@ -52,7 +52,7 @@ def cursor_delay(created_at) def base_log_data(message, params = {}) { pid: PID, - host: Gitlab.config.gitlab.host, + gitlab_host: Gitlab.config.gitlab.host, class: caller_name, message: message }.merge(params) diff --git a/ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb b/ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb index 3e16b7b4aa5c5c..d5a14852a4b437 100644 --- a/ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb +++ b/ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Geo::LogCursor::Lease, :clean_gitlab_redis_shared_state do +RSpec.describe Gitlab::Geo::LogCursor::Lease, :clean_gitlab_redis_shared_state, feature_category: :geo_replication do include ExclusiveLeaseHelpers describe '.exclusive_lease' do @@ -31,7 +31,7 @@ { pid: 111, class: 'Gitlab::Geo::LogCursor::Lease', - host: "localhost", + gitlab_host: "localhost", message: 'Lease renewed.' }) diff --git a/ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb b/ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb index 60edd2ca90ceda..40b3cf68af8747 100644 --- a/ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb +++ b/ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb @@ -2,10 +2,10 @@ require 'spec_helper' -RSpec.describe Gitlab::Geo::LogCursor::Logger, :geo do +RSpec.describe Gitlab::Geo::LogCursor::Logger, :geo, feature_category: :geo_replication do subject(:logger) { described_class.new(LoggerSpec) } - let(:data) { { pid: 111, class: 'LoggerSpec', host: 'localhost', message: 'Test' } } + let(:data) { { pid: 111, class: 'LoggerSpec', gitlab_host: 'localhost', message: 'Test' } } before do stub_const('LoggerSpec', Class.new) @@ -36,7 +36,7 @@ { pid: 111, class: "LoggerSpec", - host: 'localhost', + gitlab_host: 'localhost', message: 'Test', cursor_delay_s: 0.0 } -- GitLab From 04eb62686247c2d7dbdcd894bc25ab78b08cc858 Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Wed, 15 Feb 2023 13:08:37 +0100 Subject: [PATCH 4/4] Prefix GL Pages cache_control log keys --- lib/gitlab/pages/cache_control.rb | 6 +++--- spec/lib/gitlab/pages/cache_control_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/gitlab/pages/cache_control.rb b/lib/gitlab/pages/cache_control.rb index a0b3ab87f381c9..81da34f1219013 100644 --- a/lib/gitlab/pages/cache_control.rb +++ b/lib/gitlab/pages/cache_control.rb @@ -52,9 +52,9 @@ def clear_cache ::Gitlab::AppLogger.info( message: 'clear pages cache', - keys: keys, - type: @type, - id: @id + pages_keys: keys, + pages_type: @type, + pages_id: @id ) Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do diff --git a/spec/lib/gitlab/pages/cache_control_spec.rb b/spec/lib/gitlab/pages/cache_control_spec.rb index 3ffc1998ebc861..72240f52580e50 100644 --- a/spec/lib/gitlab/pages/cache_control_spec.rb +++ b/spec/lib/gitlab/pages/cache_control_spec.rb @@ -22,9 +22,9 @@ .to receive(:info) .with( message: 'clear pages cache', - keys: cached_keys, - type: type, - id: 1 + pages_keys: cached_keys, + pages_type: type, + pages_id: 1 ) expect(Rails.cache) -- GitLab