Merge branch 'ali/make-tracking-event-easier-to-call' into 'master'

Make it easier to call Gitlab::Tracking.event with standard context

See merge request gitlab-org/gitlab!52026

doc/development/snowplow.md

@@ -311,6 +311,9 @@ Custom event tracking and instrumentation can be added by directly calling the `
 | `property` | String                    | nil           | As described in [Structured event taxonomy](#structured-event-taxonomy).                                                          |
 | `value`    | Numeric                   | nil           | As described in [Structured event taxonomy](#structured-event-taxonomy).                                                          |
 | `context`  | Array\[SelfDescribingJSON\] | nil           | An array of custom contexts to send with this event. Most events should not have any custom contexts.                             |
+| `project`  | Project                   | nil           | The project associated with the event |
+| `user`     | User                      | nil           | The user associated with the event |
+| `namespace` | Namespace                | nil           | The namespace associated with the event |
 
 Tracking can be viewed as either tracking user behavior, or can be used for instrumentation to monitor and visualize performance over time in an area or aspect of code.
 
@@ -321,10 +324,8 @@ class Projects::CreateService < BaseService
   def execute
     project = Project.create(params)
 
-    Gitlab::Tracking.event('Projects::CreateService', 'create_project',
-      label: project.errors.full_messages.to_sentence,
-      value: project.valid?
-    )
+    Gitlab::Tracking.event('Projects::CreateService', 'create_project', label: project.errors.full_messages.to_sentence,
+                           property: project.valid?.to_s, project: project, user: current_user, namespace: namespace)
   end
 end
 ```

ee/app/services/epics/issue_promote_service.rb

@@ -32,10 +32,8 @@ module Epics
     end
 
     def track_event
-      context = ::Gitlab::Tracking::StandardContext.new(namespace: @parent_group, project: issue.project, user: current_user)
-
       ::Gitlab::Tracking.event('epics', 'promote', property: 'issue_id', value: original_entity.id,
-                               standard_context: context)
+                               project: issue.project, user: current_user, namespace: @parent_group)
     end
 
     def create_new_entity

ee/spec/services/epics/issue_promote_service_spec.rb

@@ -65,7 +65,7 @@ RSpec.describe Epics::IssuePromoteService, :aggregate_failures do
             subject.execute(issue)
 
             expect_snowplow_event(category: 'epics', action: 'promote', property: 'issue_id', value: issue.id,
-                                  standard_context: { namespace: group, project: project, user: user })
+                                  project: project, user: user, namespace: group)
           end
 
           it 'creates a new epic with correct attributes' do
@@ -201,7 +201,7 @@ RSpec.describe Epics::IssuePromoteService, :aggregate_failures do
             expect(epic.notes.where(discussion_id: discussion.discussion_id).count).to eq(0)
             expect(issue.notes.where(discussion_id: discussion.discussion_id).count).to eq(1)
             expect_snowplow_event(category: 'epics', action: 'promote', property: 'issue_id', value: issue.id,
-                                  standard_context: { namespace: group, project: project, user: user })
+                                  project: project, user: user, namespace: group)
           end
 
           it 'copies note attachments' do
@@ -211,7 +211,7 @@ RSpec.describe Epics::IssuePromoteService, :aggregate_failures do
 
             expect(epic.notes.user.first.attachment).to be_kind_of(AttachmentUploader)
             expect_snowplow_event(category: 'epics', action: 'promote', property: 'issue_id', value: issue.id,
-                                  standard_context: { namespace: group, project: project, user: user })
+                                  project: project, user: user, namespace: group)
           end
         end
 

lib/gitlab/tracking.rb

@@ -24,8 +24,8 @@ module Gitlab
         Gitlab::CurrentSettings.snowplow_enabled?
       end
 
-      def event(category, action, label: nil, property: nil, value: nil, context: [], standard_context: nil)
-        context.push(standard_context.to_context) if standard_context
+      def event(category, action, label: nil, property: nil, value: nil, context: [], project: nil, user: nil, namespace: nil) # rubocop:disable Metrics/ParameterLists
+        context += [Tracking::StandardContext.new(project: project, user: user, namespace: namespace).to_context]
 
         snowplow.event(category, action, label: label, property: property, value: value, context: context)
         product_analytics.event(category, action, label: label, property: property, value: value, context: context)

lib/gitlab/tracking/standard_context.rb

@@ -29,11 +29,10 @@ module Gitlab
       private
 
       def to_h
-        public_methods(false).each_with_object({}) do |method, hash|
-          next if method == :to_context
-
-          hash[method] = public_send(method) # rubocop:disable GitlabSecurity/PublicSend
-        end.merge(@data)
+        {
+          environment: environment,
+          source: source
+        }.merge(@data)
       end
     end
   end

spec/lib/gitlab/tracking/standard_context_spec.rb

@@ -9,40 +9,38 @@ RSpec.describe Gitlab::Tracking::StandardContext do
   let(:snowplow_context) { subject.to_context }
 
   describe '#to_context' do
-    context 'default fields' do
-      context 'environment' do
-        shared_examples 'contains environment' do |expected_environment|
-          it 'contains environment' do
-            expect(snowplow_context.to_json.dig(:data, :environment)).to eq(expected_environment)
-          end
-        end
-
-        context 'development or test' do
-          include_examples 'contains environment', 'development'
+    context 'environment' do
+      shared_examples 'contains environment' do |expected_environment|
+        it 'contains environment' do
+          expect(snowplow_context.to_json.dig(:data, :environment)).to eq(expected_environment)
         end
+      end
 
-        context 'staging' do
-          before do
-            allow(Gitlab).to receive(:staging?).and_return(true)
-          end
+      context 'development or test' do
+        include_examples 'contains environment', 'development'
+      end
 
-          include_examples 'contains environment', 'staging'
+      context 'staging' do
+        before do
+          allow(Gitlab).to receive(:staging?).and_return(true)
         end
 
-        context 'production' do
-          before do
-            allow(Gitlab).to receive(:com_and_canary?).and_return(true)
-          end
+        include_examples 'contains environment', 'staging'
+      end
 
-          include_examples 'contains environment', 'production'
+      context 'production' do
+        before do
+          allow(Gitlab).to receive(:com_and_canary?).and_return(true)
         end
-      end
 
-      it 'contains source' do
-        expect(snowplow_context.to_json.dig(:data, :source)).to eq(described_class::GITLAB_RAILS_SOURCE)
+        include_examples 'contains environment', 'production'
       end
     end
 
+    it 'contains source' do
+      expect(snowplow_context.to_json.dig(:data, :source)).to eq(described_class::GITLAB_RAILS_SOURCE)
+    end
+
     context 'with extra data' do
       subject { described_class.new(foo: 'bar') }
 
@@ -51,31 +49,8 @@ RSpec.describe Gitlab::Tracking::StandardContext do
       end
     end
 
-    context 'with namespace' do
-      subject { described_class.new(namespace: namespace) }
-
-      it 'creates a Snowplow context without namespace and project' do
-        expect(snowplow_context.to_json.dig(:data, :namespace_id)).to be_nil
-        expect(snowplow_context.to_json.dig(:data, :project_id)).to be_nil
-      end
-    end
-
-    context 'with project' do
-      subject { described_class.new(project: project) }
-
-      it 'creates a Snowplow context without namespace and project' do
-        expect(snowplow_context.to_json.dig(:data, :namespace_id)).to be_nil
-        expect(snowplow_context.to_json.dig(:data, :project_id)).to be_nil
-      end
-    end
-
-    context 'with project and namespace' do
-      subject { described_class.new(namespace: namespace, project: project) }
-
-      it 'creates a Snowplow context without namespace and project' do
-        expect(snowplow_context.to_json.dig(:data, :namespace_id)).to be_nil
-        expect(snowplow_context.to_json.dig(:data, :project_id)).to be_nil
-      end
+    it 'does not contain any ids' do
+      expect(snowplow_context.to_json[:data].keys).not_to include(:user_id, :project_id, :namespace_id)
     end
   end
 end

spec/lib/gitlab/tracking_spec.rb

@@ -42,36 +42,31 @@ RSpec.describe Gitlab::Tracking do
     end
 
     shared_examples 'delegates to destination' do |klass|
-      context 'with standard context' do
-        it "delegates to #{klass} destination" do
-          expect_any_instance_of(klass).to receive(:event) do |_, category, action, args|
-            expect(category).to eq('category')
-            expect(action).to eq('action')
-            expect(args[:label]).to eq('label')
-            expect(args[:property]).to eq('property')
-            expect(args[:value]).to eq(1.5)
-            expect(args[:context].length).to eq(1)
-            expect(args[:context].first.to_json[:schema]).to eq(Gitlab::Tracking::StandardContext::GITLAB_STANDARD_SCHEMA_URL)
-            expect(args[:context].first.to_json[:data]).to include(foo: 'bar')
-          end
+      it "delegates to #{klass} destination" do
+        other_context = double(:context)
 
-          described_class.event('category', 'action', label: 'label', property: 'property', value: 1.5,
-                                standard_context: Gitlab::Tracking::StandardContext.new(foo: 'bar'))
-        end
-      end
+        project = double(:project)
+        user = double(:user)
+        namespace = double(:namespace)
 
-      context 'without standard context' do
-        it "delegates to #{klass} destination" do
-          expect_any_instance_of(klass).to receive(:event) do |_, category, action, args|
-            expect(category).to eq('category')
-            expect(action).to eq('action')
-            expect(args[:label]).to eq('label')
-            expect(args[:property]).to eq('property')
-            expect(args[:value]).to eq(1.5)
-          end
+        expect(Gitlab::Tracking::StandardContext)
+          .to receive(:new)
+          .with(project: project, user: user, namespace: namespace)
+          .and_call_original
 
-          described_class.event('category', 'action', label: 'label', property: 'property', value: 1.5)
+        expect_any_instance_of(klass).to receive(:event) do |_, category, action, args|
+          expect(category).to eq('category')
+          expect(action).to eq('action')
+          expect(args[:label]).to eq('label')
+          expect(args[:property]).to eq('property')
+          expect(args[:value]).to eq(1.5)
+          expect(args[:context].length).to eq(2)
+          expect(args[:context].first).to eq(other_context)
+          expect(args[:context].last.to_json[:schema]).to eq(Gitlab::Tracking::StandardContext::GITLAB_STANDARD_SCHEMA_URL)
         end
+
+        described_class.event('category', 'action', label: 'label', property: 'property', value: 1.5,
+                              context: [other_context], project: project, user: user, namespace: namespace)
       end
     end
 

spec/support/helpers/snowplow_helpers.rb

@@ -46,7 +46,7 @@ module SnowplowHelpers
   #       }
   #     ]
   #   )
-  def expect_snowplow_event(category:, action:, context: nil, standard_context: nil, **kwargs)
+  def expect_snowplow_event(category:, action:, context: nil, **kwargs)
     if context
       kwargs[:context] = []
       context.each do |c|
@@ -56,14 +56,6 @@ module SnowplowHelpers
       end
     end
 
-    if standard_context
-      expect(Gitlab::Tracking::StandardContext)
-        .to have_received(:new)
-        .with(**standard_context)
-
-      kwargs[:standard_context] = an_instance_of(Gitlab::Tracking::StandardContext)
-    end
-
     expect(Gitlab::Tracking).to have_received(:event) # rubocop:disable RSpec/ExpectGitlabTracking
       .with(category, action, **kwargs).at_least(:once)
   end

spec/support/snowplow.rb

@@ -18,7 +18,6 @@ RSpec.configure do |config|
     stub_application_setting(snowplow_enabled: true)
 
     allow(SnowplowTracker::SelfDescribingJson).to receive(:new).and_call_original
-    allow(Gitlab::Tracking::StandardContext).to receive(:new).and_call_original
     allow(Gitlab::Tracking).to receive(:event).and_call_original # rubocop:disable RSpec/ExpectGitlabTracking
   end