Skip to content

Clarify the best practice for how to include additional properties & filters in 'internal event tracking' shared example

Problem

We've got three different mechanisms for defining additional properties in 'internal event tracking':

  • let(:additional_properties)
  • let(:label)/let(:property)/let(:value)
  • let(:event_attribute_overrides)

The outcomes of using each are not intuitively clear, and there is no recommended way to define additional properties.

Desired Outcome

  1. A best practice is defined
  2. When adding or modifying tests, engineers will use the recommended best practice
    1. CLI & docs clearly communicate the recommended approach (for standard users)
    2. Where possible, the existing spec examples use the recommended approach (for copy+paste users)

Potential Solution

Recommend composable matchers for events with filters. Recommend shared example for other scenarios. Recommend using let(:additional_properties) when using shared example.

  1. Fully accommodate additional_properties in the shared example (so that the additional properties are expected in the snowplow calls)
  2. Add instructions to docs to prefer using let(:additional_properties) with the shared example
  3. Simplify shared example to test for all metrics that include the specified event (rather than trying to filter)
    • If a metric with filters is used with the shared example, we should raise an informative error message instructing them to use composable matchers instead.
  4. Add custom matchers to CLI
  5. Remove event_attribute_overrides in favor of recommending usage of custom matchers
example test swapped over
diff --git a/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb b/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb
index f3b66ac2fc41..e8970d7bc351 100644
--- a/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb
+++ b/ee/spec/workers/audit_events/audit_event_streaming_worker_spec.rb
@@ -141,21 +141,13 @@
       context 'when audit event type is tracked as an internal event' do
         let(:event_name) { AuditEvents::Strategies::ExternalDestinationStrategy::INTERNAL_EVENTS.first }
 
-        it 'makes http call' do
+        it 'makes http call and tracks the event' do
           expect(Gitlab::HTTP).to receive(:post).once
 
-          subject
-        end
-
-        it_behaves_like 'internal event tracking' do
-          let(:event) { 'trigger_audit_event' }
-          let(:label) { event_name }
-          let(:category) { 'AuditEvents::Strategies::GroupExternalDestinationStrategy' }
-          let(:event_attribute_overrides) { { project: nil, namespace: nil } }
-
-          before do
-            allow(Gitlab::HTTP).to receive(:post).once
-          end
+          expect { subject }
+            .to trigger_internal_events('trigger_audit_event')
+              .with(label: event_name, category: 'AuditEvents::Strategies::GroupExternalDestinationStrategy')
+            .and increment_usage_metrics('counts.delete_epic')
         end
       end

How to verify

Further actions needed

Edited by Sarah Yasonik