From 2a3715a009cd3dd593ec36cbebde4bba942faec0 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 24 Sep 2025 14:39:57 +0800
Subject: [PATCH] Log data discrepancies when use_user_group_member_roles flag
 is enabled

Previously, track_diff method operates on the assumption that
use_user_group_member_roles is disabled and receives the results of
executing union_query as its argument. It then executes
user_group_member_roles_query to fetch the other set of results,
compares them, and logs any discrepancies.

In this change we update track_diff method to log data discrepancies
between results of union_query and user_group_member_roles_query
whether use_user_group_member_roles feature flag is enabled or disabled.
---
 .../user_member_roles_in_groups_preloader.rb  | 27 ++++++++++++++-----
 ...r_member_roles_in_groups_preloader_spec.rb | 22 ++++++++++++++-
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb b/ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb
index adbafb0f814842..58c4d2774d80d1 100644
--- a/ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb
+++ b/ee/app/models/preloaders/user_member_roles_in_groups_preloader.rb
@@ -32,8 +32,8 @@ def abilities_for_user_grouped_by_group(group_ids)
 
       log_statistics(group_ids)
 
-      get_results(query).tap do |existing_query_results|
-        track_diff(existing_query_results)
+      get_results(query).tap do |results|
+        track_diff(results)
       end
     end
 
@@ -113,7 +113,7 @@ def union_query
     end
 
     def query
-      if ::Feature.enabled?(:use_user_group_member_roles, Feature.current_request)
+      if use_user_group_member_roles?
         user_group_member_roles_query
       else
         union_query
@@ -163,7 +163,7 @@ def log_statistics(group_ids)
       )
     end
 
-    def track_diff(existing_query_results)
+    def track_diff(results)
       return unless ::Gitlab::Saas.feature_available?(:gitlab_com_subscriptions)
       return if ::Feature.disabled?(:track_user_group_member_roles_accuracy, Feature.current_request)
 
@@ -178,11 +178,17 @@ def track_diff(existing_query_results)
       cached_permissions = []
       uncached_permissions = []
 
-      cached_results = get_results(user_group_member_roles_query)
+      if use_user_group_member_roles?
+        cached_results = results
+        union_query_results = get_results(union_query)
+      else
+        cached_results = get_results(user_group_member_roles_query)
+        union_query_results = results
+      end
 
       group_ids.each do |id|
         cached_permissions = extract_permissions(cached_results, id)
-        uncached_permissions = extract_permissions(existing_query_results, id)
+        uncached_permissions = extract_permissions(union_query_results, id)
 
         next if cached_permissions == uncached_permissions
 
@@ -191,7 +197,9 @@ def track_diff(existing_query_results)
 
       return if group_ids_with_diff.empty?
 
-      log_info = { class: self.class.name, event: 'Inaccurate user_group_member_roles data', user_id: user.id }
+      log_info = { class: self.class.name, event: 'Inaccurate user_group_member_roles data', user_id: user.id,
+                   use_user_group_member_roles: use_user_group_member_roles? }
+
       log_info = if group_ids_with_diff.length > 1
                    log_info.merge(group_ids: group_ids_with_diff)
                  else
@@ -208,5 +216,10 @@ def track_diff(existing_query_results)
     def extract_permissions(result, group_id)
       result.fetch(group_id, []).sort.join(', ')
     end
+
+    def use_user_group_member_roles?
+      ::Feature.enabled?(:use_user_group_member_roles, Feature.current_request)
+    end
+    strong_memoize_attr :use_user_group_member_roles?
   end
 end
diff --git a/ee/spec/models/preloaders/user_member_roles_in_groups_preloader_spec.rb b/ee/spec/models/preloaders/user_member_roles_in_groups_preloader_spec.rb
index b434ce863a8e08..a46b4ef35608c1 100644
--- a/ee/spec/models/preloaders/user_member_roles_in_groups_preloader_spec.rb
+++ b/ee/spec/models/preloaders/user_member_roles_in_groups_preloader_spec.rb
@@ -244,7 +244,8 @@
       {
         class: described_class.name,
         event: 'Inaccurate user_group_member_roles data',
-        user_id: user.id
+        user_id: user.id,
+        use_user_group_member_roles: false
       }
     end
 
@@ -270,6 +271,7 @@
     # user's member record for sub_group_1.
     context 'when result of query using user_group_member_roles table is different' do
       it 'logs' do
+        expect(Gitlab::AppLogger).to receive(:info) # earlier call in #log_statistics
         expect(Gitlab::AppLogger).to receive(:info).with({
           **log_payload,
           permissions: expected_abilities.join(', '),
@@ -279,6 +281,24 @@
         result
       end
 
+      context 'when use_user_group_member_roles feature flag is enabled' do
+        before do
+          stub_feature_flags(use_user_group_member_roles: true)
+        end
+
+        it 'logs' do
+          expect(Gitlab::AppLogger).to receive(:info) # earlier call in #log_statistics
+          expect(Gitlab::AppLogger).to receive(:info).with({
+            **log_payload,
+            permissions: expected_abilities.join(', '),
+            user_group_member_roles_permissions: '',
+            use_user_group_member_roles: true
+          })
+
+          result
+        end
+      end
+
       context 'when there are multiple groups with different results' do
         let(:groups_list) { [sub_group_1, sub_group_2] }
 
-- 
GitLab