Commit e2885e45 authored by David Fernandez's avatar David Fernandez Committed by Kamil Trzciński

Add tracking events for the dependency proxy

`pull_manifest` and `pull_blob`

Changelog: other
parent a0f715ed
...@@ -4,6 +4,7 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro ...@@ -4,6 +4,7 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
include DependencyProxy::Auth include DependencyProxy::Auth
include DependencyProxy::GroupAccess include DependencyProxy::GroupAccess
include SendFileUpload include SendFileUpload
include ::PackagesHelper # for event tracking
before_action :ensure_token_granted! before_action :ensure_token_granted!
before_action :ensure_feature_enabled! before_action :ensure_feature_enabled!
...@@ -22,6 +23,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro ...@@ -22,6 +23,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
response.headers['Etag'] = "\"#{result[:manifest].digest}\"" response.headers['Etag'] = "\"#{result[:manifest].digest}\""
content_type = result[:manifest].content_type content_type = result[:manifest].content_type
event_name = tracking_event_name(object_type: :manifest, from_cache: result[:from_cache])
track_package_event(event_name, :dependency_proxy, namespace: group, user: current_user)
send_upload( send_upload(
result[:manifest].file, result[:manifest].file,
proxy: true, proxy: true,
...@@ -38,6 +41,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro ...@@ -38,6 +41,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
.new(group, image, token, params[:sha]).execute .new(group, image, token, params[:sha]).execute
if result[:status] == :success if result[:status] == :success
event_name = tracking_event_name(object_type: :blob, from_cache: result[:from_cache])
track_package_event(event_name, :dependency_proxy, namespace: group, user: current_user)
send_upload(result[:blob].file) send_upload(result[:blob].file)
else else
head result[:http_status] head result[:http_status]
...@@ -54,6 +59,13 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro ...@@ -54,6 +59,13 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
params[:tag] params[:tag]
end end
def tracking_event_name(object_type:, from_cache:)
event_name = "pull_#{object_type}"
event_name = "#{event_name}_from_cache" if from_cache
event_name
end
def dependency_proxy def dependency_proxy
@dependency_proxy ||= @dependency_proxy ||=
group.dependency_proxy_setting || group.create_dependency_proxy_setting group.dependency_proxy_setting || group.create_dependency_proxy_setting
......
...@@ -4,7 +4,7 @@ class Packages::Event < ApplicationRecord ...@@ -4,7 +4,7 @@ class Packages::Event < ApplicationRecord
belongs_to :package, optional: true belongs_to :package, optional: true
UNIQUE_EVENTS_ALLOWED = %i[push_package delete_package pull_package pull_symbol_package push_symbol_package].freeze UNIQUE_EVENTS_ALLOWED = %i[push_package delete_package pull_package pull_symbol_package push_symbol_package].freeze
EVENT_SCOPES = ::Packages::Package.package_types.merge(container: 1000, tag: 1001).freeze EVENT_SCOPES = ::Packages::Package.package_types.merge(container: 1000, tag: 1001, dependency_proxy: 1002).freeze
EVENT_PREFIX = "i_package" EVENT_PREFIX = "i_package"
...@@ -23,7 +23,11 @@ class Packages::Event < ApplicationRecord ...@@ -23,7 +23,11 @@ class Packages::Event < ApplicationRecord
list_tags: 9, list_tags: 9,
cli_metadata: 10, cli_metadata: 10,
pull_symbol_package: 11, pull_symbol_package: 11,
push_symbol_package: 12 push_symbol_package: 12,
pull_manifest: 13,
pull_manifest_from_cache: 14,
pull_blob: 15,
pull_blob_from_cache: 16
} }
enum originator_type: { user: 0, deploy_token: 1, guest: 2 } enum originator_type: { user: 0, deploy_token: 1, guest: 2 }
......
...@@ -10,10 +10,12 @@ module DependencyProxy ...@@ -10,10 +10,12 @@ module DependencyProxy
end end
def execute def execute
from_cache = true
file_name = @blob_sha.sub('sha256:', '') + '.gz' file_name = @blob_sha.sub('sha256:', '') + '.gz'
blob = @group.dependency_proxy_blobs.find_or_build(file_name) blob = @group.dependency_proxy_blobs.find_or_build(file_name)
unless blob.persisted? unless blob.persisted?
from_cache = false
result = DependencyProxy::DownloadBlobService result = DependencyProxy::DownloadBlobService
.new(@image, @blob_sha, @token).execute .new(@image, @blob_sha, @token).execute
...@@ -28,7 +30,7 @@ module DependencyProxy ...@@ -28,7 +30,7 @@ module DependencyProxy
blob.save! blob.save!
end end
success(blob: blob) success(blob: blob, from_cache: from_cache)
end end
private private
......
...@@ -17,10 +17,10 @@ module DependencyProxy ...@@ -17,10 +17,10 @@ module DependencyProxy
head_result = DependencyProxy::HeadManifestService.new(@image, @tag, @token).execute head_result = DependencyProxy::HeadManifestService.new(@image, @tag, @token).execute
return success(manifest: @manifest) if cached_manifest_matches?(head_result) return success(manifest: @manifest, from_cache: true) if cached_manifest_matches?(head_result)
pull_new_manifest pull_new_manifest
respond respond(from_cache: false)
rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS
respond respond
end end
...@@ -44,9 +44,9 @@ module DependencyProxy ...@@ -44,9 +44,9 @@ module DependencyProxy
@manifest && @manifest.digest == head_result[:digest] && @manifest.content_type == head_result[:content_type] @manifest && @manifest.digest == head_result[:digest] && @manifest.content_type == head_result[:content_type]
end end
def respond def respond(from_cache: true)
if @manifest.persisted? if @manifest.persisted?
success(manifest: @manifest) success(manifest: @manifest, from_cache: from_cache)
else else
error('Failed to download the manifest from the external registry', 503) error('Failed to download the manifest from the external registry', 503)
end end
......
...@@ -12,6 +12,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -12,6 +12,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
let(:token_response) { { status: :success, token: 'abcd1234' } } let(:token_response) { { status: :success, token: 'abcd1234' } }
let(:jwt) { build_jwt(user) } let(:jwt) { build_jwt(user) }
let(:token_header) { "Bearer #{jwt.encoded}" } let(:token_header) { "Bearer #{jwt.encoded}" }
let(:snowplow_gitlab_standard_context) { { namespace: group, user: user } }
shared_examples 'without a token' do shared_examples 'without a token' do
before do before do
...@@ -104,7 +105,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -104,7 +105,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
describe 'GET #manifest' do describe 'GET #manifest' do
let_it_be(:manifest) { create(:dependency_proxy_manifest) } let_it_be(:manifest) { create(:dependency_proxy_manifest) }
let(:pull_response) { { status: :success, manifest: manifest } } let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
before do before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
...@@ -122,6 +123,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -122,6 +123,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do
it_behaves_like 'without a token' it_behaves_like 'without a token'
it_behaves_like 'without permission' it_behaves_like 'without permission'
it_behaves_like 'feature flag disabled with private group' it_behaves_like 'feature flag disabled with private group'
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest'
context 'with a cache entry' do
let(:pull_response) { { status: :success, manifest: manifest, from_cache: true } }
it_behaves_like 'returning response status', :success
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest_from_cache'
end
context 'remote token request fails' do context 'remote token request fails' do
let(:token_response) do let(:token_response) do
...@@ -186,7 +195,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -186,7 +195,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
let_it_be(:blob) { create(:dependency_proxy_blob) } let_it_be(:blob) { create(:dependency_proxy_blob) }
let(:blob_sha) { blob.file_name.sub('.gz', '') } let(:blob_sha) { blob.file_name.sub('.gz', '') }
let(:blob_response) { { status: :success, blob: blob } } let(:blob_response) { { status: :success, blob: blob, from_cache: false } }
before do before do
allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance| allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance|
...@@ -204,6 +213,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -204,6 +213,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do
it_behaves_like 'without a token' it_behaves_like 'without a token'
it_behaves_like 'without permission' it_behaves_like 'without permission'
it_behaves_like 'feature flag disabled with private group' it_behaves_like 'feature flag disabled with private group'
it_behaves_like 'a package tracking event', described_class.name, 'pull_blob'
context 'with a cache entry' do
let(:blob_response) { { status: :success, blob: blob, from_cache: true } }
it_behaves_like 'returning response status', :success
it_behaves_like 'a package tracking event', described_class.name, 'pull_blob_from_cache'
end
context 'remote blob request fails' do context 'remote blob request fails' do
let(:blob_response) do let(:blob_response) do
......
...@@ -26,6 +26,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do ...@@ -26,6 +26,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
expect(subject[:blob]).to be_a(DependencyProxy::Blob) expect(subject[:blob]).to be_a(DependencyProxy::Blob)
expect(subject[:blob]).to be_persisted expect(subject[:blob]).to be_persisted
expect(subject[:from_cache]).to eq false
end end
end end
...@@ -36,6 +37,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do ...@@ -36,6 +37,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
expect(subject[:blob]).to be_a(DependencyProxy::Blob) expect(subject[:blob]).to be_a(DependencyProxy::Blob)
expect(subject[:blob]).to eq(blob) expect(subject[:blob]).to eq(blob)
expect(subject[:from_cache]).to eq true
end end
end end
......
...@@ -30,6 +30,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -30,6 +30,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest) expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
expect(subject[:manifest]).to be_persisted expect(subject[:manifest]).to be_persisted
expect(subject[:from_cache]).to eq false
end end
end end
...@@ -62,6 +63,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -62,6 +63,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest) expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
expect(subject[:manifest]).to eq(dependency_proxy_manifest) expect(subject[:manifest]).to eq(dependency_proxy_manifest)
expect(subject[:from_cache]).to eq true
end end
end end
...@@ -81,6 +83,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -81,6 +83,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
expect(subject[:manifest]).to eq(dependency_proxy_manifest) expect(subject[:manifest]).to eq(dependency_proxy_manifest)
expect(subject[:manifest].content_type).to eq(content_type) expect(subject[:manifest].content_type).to eq(content_type)
expect(subject[:manifest].digest).to eq(digest) expect(subject[:manifest].digest).to eq(digest)
expect(subject[:from_cache]).to eq false
end end
end end
......
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