Merge branch '238056-snowplow-events' into 'master'

Add tracking events for the dependency proxy

See merge request gitlab-org/gitlab!65812

app/controllers/groups/dependency_proxy_for_containers_controller.rb

@@ -4,6 +4,7 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
   include DependencyProxy::Auth
   include DependencyProxy::GroupAccess
   include SendFileUpload
+  include ::PackagesHelper # for event tracking
 
   before_action :ensure_token_granted!
   before_action :ensure_feature_enabled!
@@ -22,6 +23,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
       response.headers['Etag'] = "\"#{result[:manifest].digest}\""
       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(
         result[:manifest].file,
         proxy: true,
@@ -38,6 +41,8 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
       .new(group, image, token, params[:sha]).execute
 
     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)
     else
       head result[:http_status]
@@ -54,6 +59,13 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
     params[:tag]
   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
     @dependency_proxy ||=
       group.dependency_proxy_setting || group.create_dependency_proxy_setting

app/models/packages/event.rb

@@ -4,7 +4,7 @@ class Packages::Event < ApplicationRecord
   belongs_to :package, optional: true
 
   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"
 
@@ -23,7 +23,11 @@ class Packages::Event < ApplicationRecord
     list_tags: 9,
     cli_metadata: 10,
     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 }

app/services/dependency_proxy/find_or_create_blob_service.rb

@@ -10,10 +10,12 @@ module DependencyProxy
     end
 
     def execute
+      from_cache = true
       file_name = @blob_sha.sub('sha256:', '') + '.gz'
       blob = @group.dependency_proxy_blobs.find_or_build(file_name)
 
       unless blob.persisted?
+        from_cache = false
         result = DependencyProxy::DownloadBlobService
           .new(@image, @blob_sha, @token).execute
 
@@ -28,7 +30,7 @@ module DependencyProxy
         blob.save!
       end
 
-      success(blob: blob)
+      success(blob: blob, from_cache: from_cache)
     end
 
     private

app/services/dependency_proxy/find_or_create_manifest_service.rb

@@ -17,10 +17,10 @@ module DependencyProxy
 
       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
-      respond
+      respond(from_cache: false)
     rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS
       respond
     end
@@ -44,9 +44,9 @@ module DependencyProxy
       @manifest && @manifest.digest == head_result[:digest] && @manifest.content_type == head_result[:content_type]
     end
 
-    def respond
+    def respond(from_cache: true)
       if @manifest.persisted?
-        success(manifest: @manifest)
+        success(manifest: @manifest, from_cache: from_cache)
       else
         error('Failed to download the manifest from the external registry', 503)
       end

spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
   let(:token_response) { { status: :success, token: 'abcd1234' } }
   let(:jwt) { build_jwt(user) }
   let(:token_header) { "Bearer #{jwt.encoded}" }
+  let(:snowplow_gitlab_standard_context) { { namespace: group, user: user } }
 
   shared_examples 'without a token' do
     before do
@@ -104,7 +105,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
   describe 'GET #manifest' do
     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
       allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
@@ -122,6 +123,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do
       it_behaves_like 'without a token'
       it_behaves_like 'without permission'
       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
         let(:token_response) do
@@ -186,7 +195,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
     let_it_be(:blob) { create(:dependency_proxy_blob) }
 
     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
       allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance|
@@ -204,6 +213,14 @@ RSpec.describe Groups::DependencyProxyForContainersController do
       it_behaves_like 'without a token'
       it_behaves_like 'without permission'
       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
         let(:blob_response) do

spec/services/dependency_proxy/find_or_create_blob_service_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do
       expect(subject[:status]).to eq(:success)
       expect(subject[:blob]).to be_a(DependencyProxy::Blob)
       expect(subject[:blob]).to be_persisted
+      expect(subject[:from_cache]).to eq false
     end
   end
 
@@ -36,6 +37,7 @@ RSpec.describe DependencyProxy::FindOrCreateBlobService do
       expect(subject[:status]).to eq(:success)
       expect(subject[:blob]).to be_a(DependencyProxy::Blob)
       expect(subject[:blob]).to eq(blob)
+      expect(subject[:from_cache]).to eq true
     end
   end
 

spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb

@@ -30,6 +30,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
           expect(subject[:status]).to eq(:success)
           expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
           expect(subject[:manifest]).to be_persisted
+          expect(subject[:from_cache]).to eq false
         end
       end
 
@@ -62,6 +63,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
           expect(subject[:status]).to eq(:success)
           expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
           expect(subject[:manifest]).to eq(dependency_proxy_manifest)
+          expect(subject[:from_cache]).to eq true
         end
       end
 
@@ -81,6 +83,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
           expect(subject[:manifest]).to eq(dependency_proxy_manifest)
           expect(subject[:manifest].content_type).to eq(content_type)
           expect(subject[:manifest].digest).to eq(digest)
+          expect(subject[:from_cache]).to eq false
         end
       end