Skip to content

Fix flaky HLLRedisCounter test suite

What does this MR do and why?

It fixes flaky test suite for HLLRedisCounter It was flaky due to the known_events entries being memoized via class variable, which caused state to leak between test cases if multiple test suites referenced HLLRedisCounter in single process. This MR fix the issue by clearing memoized values before each test case, to prevent state leaking.

Fixes #379258 (closed)

Screenshots or screen recordings

You can emulate flaky scenario by executing two test files in the row, but first you need to comment out all but affected spec from https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb#L108

Screenshot_2022-11-16_at_08.11.24

diff --git a/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
index 5a9ac46891e1..b91778f0b0f4 100644
--- a/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
+++ b/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
@@ -118,102 +118,102 @@
     end
   end

-  describe '.track_event_in_context' do
-    before do
-      allow(described_class).to receive(:known_events).and_return(known_events)
-    end
-
-    context 'with valid context' do
-      where(:entity, :event_name, :context) do
-        entity1 | context_event | default_context
-        entity1 | context_event | ultimate_context
-        entity1 | context_event | gold_context
-      end
-
-      with_them do
-        it 'increments context event counter' do
-          expect(Gitlab::Redis::HLL).to receive(:add) do |kwargs|
-            expect(kwargs[:key]).to match(/^#{context}\_.*/)
-          end
-
-          described_class.track_event_in_context(event_name, values: entity, context: context)
-        end
-      end
-    end
-
-    context 'when sending empty context' do
-      it 'is not incrementing the counter' do
-        expect(Gitlab::Redis::HLL).not_to receive(:add)
-
-        described_class.track_event_in_context(context_event, values: entity1, context: '')
-      end
-    end
-  end
-
-  describe '.unique_events' do
-    context 'with events tracked in context' do
-      before do
-        allow(described_class).to receive(:known_events).and_return(known_events)
-        described_class.track_event_in_context(context_event, values: [entity1, entity3], context: default_context, time: 2.days.ago)
-        described_class.track_event_in_context(context_event, values: entity3, context: ultimate_context, time: 2.days.ago)
-        described_class.track_event_in_context(context_event, values: entity3, context: gold_context, time: 2.days.ago)
-        described_class.track_event_in_context(context_event, values: entity3, context: invalid_context, time: 2.days.ago)
-        described_class.track_event_in_context(context_event, values: [entity1, entity2], context: '', time: 2.weeks.ago)
-      end
-
-      subject(:unique_events) { described_class.unique_events(event_names: context_event, start_date: 4.weeks.ago, end_date: Date.current, context: context) }
-
-      context 'with correct arguments' do
-        where(:context, :value) do
-          ref(:default_context)  | 2
-          ref(:ultimate_context) | 1
-          ref(:gold_context)     | 1
-          ''                     | 0
-        end
-
-        with_them do
-          it { is_expected.to eq value }
-        end
-      end
-
-      context 'with invalid context' do
-        let(:context) { invalid_context }
-        let(:event_names) { context_event }
-
-        it 'raise error' do
-          expect { unique_events }.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::InvalidContext)
-        end
-      end
-    end
-
-    it 'does not include instrumented categories' do
-      expect(described_class.unique_events_data.keys)
-        .not_to include(*described_class.categories_collected_from_metrics_definitions)
-    end
-  end
-
-  describe '.track_event' do
-    before do
-      allow(described_class).to receive(:known_events).and_return(known_events)
-    end
-
-    context 'with settings usage ping disabled' do
-      before do
-        stub_application_setting(usage_ping_enabled: false)
-      end
-
-      context 'with license usage ping enabled' do
-        before do
-          # License.current.customer_service_enabled? == true
-          create_current_license(operational_metrics_enabled: true)
-        end
-
-        it 'tracks the event' do
-          expect(Gitlab::Redis::HLL).to receive(:add)
-
-          described_class.track_event(context_event, values: entity1, time: Date.current)
-        end
-      end
-    end
-  end
+  # describe '.track_event_in_context' do
+  #   before do
+  #     allow(described_class).to receive(:known_events).and_return(known_events)
+  #   end
+  #
+  #   context 'with valid context' do
+  #     where(:entity, :event_name, :context) do
+  #       entity1 | context_event | default_context
+  #       entity1 | context_event | ultimate_context
+  #       entity1 | context_event | gold_context
+  #     end
+  #
+  #     with_them do
+  #       it 'increments context event counter' do
+  #         expect(Gitlab::Redis::HLL).to receive(:add) do |kwargs|
+  #           expect(kwargs[:key]).to match(/^#{context}\_.*/)
+  #         end
+  #
+  #         described_class.track_event_in_context(event_name, values: entity, context: context)
+  #       end
+  #     end
+  #   end
+  #
+  #   context 'when sending empty context' do
+  #     it 'is not incrementing the counter' do
+  #       expect(Gitlab::Redis::HLL).not_to receive(:add)
+  #
+  #       described_class.track_event_in_context(context_event, values: entity1, context: '')
+  #     end
+  #   end
+  # end
+
+  # describe '.unique_events' do
+  #   context 'with events tracked in context' do
+  #     before do
+  #       allow(described_class).to receive(:known_events).and_return(known_events)
+  #       described_class.track_event_in_context(context_event, values: [entity1, entity3], context: default_context, time: 2.days.ago)
+  #       described_class.track_event_in_context(context_event, values: entity3, context: ultimate_context, time: 2.days.ago)
+  #       described_class.track_event_in_context(context_event, values: entity3, context: gold_context, time: 2.days.ago)
+  #       described_class.track_event_in_context(context_event, values: entity3, context: invalid_context, time: 2.days.ago)
+  #       described_class.track_event_in_context(context_event, values: [entity1, entity2], context: '', time: 2.weeks.ago)
+  #     end
+  #
+  #     subject(:unique_events) { described_class.unique_events(event_names: context_event, start_date: 4.weeks.ago, end_date: Date.current, context: context) }
+  #
+  #     context 'with correct arguments' do
+  #       where(:context, :value) do
+  #         ref(:default_context)  | 2
+  #         ref(:ultimate_context) | 1
+  #         ref(:gold_context)     | 1
+  #         ''                     | 0
+  #       end
+  #
+  #       with_them do
+  #         it { is_expected.to eq value }
+  #       end
+  #     end
+  #
+  #     context 'with invalid context' do
+  #       let(:context) { invalid_context }
+  #       let(:event_names) { context_event }
+  #
+  #       it 'raise error' do
+  #         expect { unique_events }.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::InvalidContext)
+  #       end
+  #     end
+  #   end
+  #
+  #   it 'does not include instrumented categories' do
+  #     expect(described_class.unique_events_data.keys)
+  #       .not_to include(*described_class.categories_collected_from_metrics_definitions)
+  #   end
+  # end
+
+  # describe '.track_event' do
+  #   before do
+  #     allow(described_class).to receive(:known_events).and_return(known_events)
+  #   end
+  #
+  #   context 'with settings usage ping disabled' do
+  #     before do
+  #       stub_application_setting(usage_ping_enabled: false)
+  #     end
+  #
+  #     context 'with license usage ping enabled' do
+  #       before do
+  #         # License.current.customer_service_enabled? == true
+  #         create_current_license(operational_metrics_enabled: true)
+  #       end
+  #
+  #       it 'tracks the event' do
+  #         expect(Gitlab::Redis::HLL).to receive(:add)
+  #
+  #         described_class.track_event(context_event, values: entity1, time: Date.current)
+  #       end
+  #     end
+  #   end
+  # end
 end

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mikołaj Wawrzyniak

Merge request reports