Commit 4ef7f0a7 authored by Maksim Stankevic's avatar Maksim Stankevic

Delete tags service logging enhancement

parent 93e52c14
......@@ -36,11 +36,11 @@ module Projects
def log_response(response)
log_data = LOG_DATA_BASE.merge(
container_repository_id: @container_repository.id,
message: 'deleted tags'
)
message: 'deleted tags',
deleted_tags_count: response[:deleted]&.size
).compact
if response[:status] == :success
log_data[:deleted_tags_count] = response[:deleted].size
log_info(log_data)
else
log_data[:message] = response[:message]
......
......@@ -14,6 +14,7 @@ module Projects
def initialize(container_repository, tag_names)
@container_repository = container_repository
@tag_names = tag_names
@deleted_tags = []
end
# Delete tags by name with a single DELETE request. This is only supported
......@@ -25,7 +26,7 @@ module Projects
delete_tags
rescue TimeoutError => e
::Gitlab::ErrorTracking.track_exception(e, tags_count: @tag_names&.size, container_repository_id: @container_repository&.id)
error('timeout while deleting tags')
error('timeout while deleting tags', nil, pass_back: { deleted: @deleted_tags })
end
private
......@@ -33,13 +34,15 @@ module Projects
def delete_tags
start_time = Time.zone.now
deleted_tags = @tag_names.select do |name|
@tag_names.each do |name|
raise TimeoutError if timeout?(start_time)
@container_repository.delete_tag_by_name(name)
if @container_repository.delete_tag_by_name(name)
@deleted_tags.append(name)
end
end
deleted_tags.any? ? success(deleted: deleted_tags) : error('could not delete tags')
@deleted_tags.any? ? success(deleted: @deleted_tags) : error('could not delete tags')
end
def timeout?(start_time)
......
---
title: Improving Container Registry Delete Tags Service to log number of successfully
deleted tags even if deletion process was interrupted by a timeout
merge_request: 46079
author: Maksim Stankevic, @maksimstankevic
type: changed
......@@ -27,13 +27,17 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
end
end
RSpec.shared_examples 'logging an error response' do |message: 'could not delete tags'|
RSpec.shared_examples 'logging an error response' do |message: 'could not delete tags', extra_log: {}|
it 'logs an error message' do
expect(service).to receive(:log_error).with(
log_data = {
service_class: 'Projects::ContainerRepository::DeleteTagsService',
message: message,
container_repository_id: repository.id
)
}
log_data.merge!(extra_log) if extra_log.any?
expect(service).to receive(:log_error).with(log_data)
subject
end
......@@ -115,7 +119,7 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
it { is_expected.to include(status: :error, message: 'timeout while deleting tags') }
it_behaves_like 'logging an error response', message: 'timeout while deleting tags'
it_behaves_like 'logging an error response', message: 'timeout while deleting tags', extra_log: { deleted_tags_count: 0 }
end
end
end
......
......@@ -67,7 +67,7 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
stub_delete_reference_requests('A' => 200)
end
it { is_expected.to include(status: :error, message: 'timeout while deleting tags') }
it { is_expected.to eq(status: :error, message: 'timeout while deleting tags', deleted: ['A']) }
it 'tracks the exception' do
expect(::Gitlab::ErrorTracking)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment