diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 6ba851aac1..92d1fe1625 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -1164,22 +1164,34 @@ func (ts *TelemetryService) trackGroups() { mlog.Debug("Could not get distinct_group_member_count", mlog.Err(err)) } + distinctCustomGroupMemberCount, err := ts.dbStore.Group().DistinctGroupMemberCountForSource(model.GroupSourceCustom) + if err != nil { + mlog.Debug("Could not get distinct_custom_group_member_count", mlog.Err(err)) + } + + distinctLdapGroupMemberCount, err := ts.dbStore.Group().DistinctGroupMemberCountForSource(model.GroupSourceLdap) + if err != nil { + mlog.Debug("Could not get distinct_ldap_group_member_count", mlog.Err(err)) + } + groupCountWithAllowReference, err := ts.dbStore.Group().GroupCountWithAllowReference() if err != nil { mlog.Debug("Could not get group_count_with_allow_reference", mlog.Err(err)) } ts.SendTelemetry(TrackGroups, map[string]interface{}{ - "group_count": groupCount, - "ldap_group_count": ldapGroupCount, - "custom_group_count": customGroupCount, - "group_team_count": groupTeamCount, - "group_channel_count": groupChannelCount, - "group_synced_team_count": groupSyncedTeamCount, - "group_synced_channel_count": groupSyncedChannelCount, - "group_member_count": groupMemberCount, - "distinct_group_member_count": distinctGroupMemberCount, - "group_count_with_allow_reference": groupCountWithAllowReference, + "group_count": groupCount, + "ldap_group_count": ldapGroupCount, + "custom_group_count": customGroupCount, + "group_team_count": groupTeamCount, + "group_channel_count": groupChannelCount, + "group_synced_team_count": groupSyncedTeamCount, + "group_synced_channel_count": groupSyncedChannelCount, + "group_member_count": groupMemberCount, + "distinct_group_member_count": distinctGroupMemberCount, + "distinct_custom_group_member_count": distinctCustomGroupMemberCount, + "distinct_ldap_group_member_count": distinctLdapGroupMemberCount, + "group_count_with_allow_reference": groupCountWithAllowReference, }) } diff --git a/services/telemetry/telemetry_test.go b/services/telemetry/telemetry_test.go index 1aa2ed5ef0..3b6fc9d289 100644 --- a/services/telemetry/telemetry_test.go +++ b/services/telemetry/telemetry_test.go @@ -242,6 +242,7 @@ func initializeMocks(cfg *model.Config, cloudLicense bool) (*mocks.ServerIface, groupStore.On("GroupCountWithAllowReference").Return(int64(13), nil) groupStore.On("GroupCountBySource", model.GroupSourceCustom).Return(int64(10), nil) groupStore.On("GroupCountBySource", model.GroupSourceLdap).Return(int64(2), nil) + groupStore.On("DistinctGroupMemberCountForSource", mock.AnythingOfType("model.GroupSource")).Return(int64(1), nil) schemeStore := storeMocks.SchemeStore{} schemeStore.On("CountByScope", "channel").Return(int64(8), nil) diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index f2ce731216..4a7b743ab1 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -3908,6 +3908,24 @@ func (s *OpenTracingLayerGroupStore) DistinctGroupMemberCount() (int64, error) { return result, err } +func (s *OpenTracingLayerGroupStore) DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.DistinctGroupMemberCountForSource") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.GroupStore.DistinctGroupMemberCountForSource(source) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerGroupStore) Get(groupID string) (*model.Group, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "GroupStore.Get") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index cac57e48bd..182381cfb8 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -4394,6 +4394,27 @@ func (s *RetryLayerGroupStore) DistinctGroupMemberCount() (int64, error) { } +func (s *RetryLayerGroupStore) DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) { + + tries := 0 + for { + result, err := s.GroupStore.DistinctGroupMemberCountForSource(source) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerGroupStore) Get(groupID string) (*model.Group, error) { tries := 0 diff --git a/store/sqlstore/group_store.go b/store/sqlstore/group_store.go index 0f410315e3..b03ead29c2 100644 --- a/store/sqlstore/group_store.go +++ b/store/sqlstore/group_store.go @@ -1740,6 +1740,26 @@ func (s *SqlGroupStore) DistinctGroupMemberCount() (int64, error) { return s.countTableWithSelectAndWhere("COUNT(DISTINCT UserId)", "GroupMembers", nil) } +func (s *SqlGroupStore) DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) { + builder := s.getQueryBuilder(). + Select("COUNT(DISTINCT GroupMembers.UserId)"). + From("GroupMembers"). + Join("UserGroups ON GroupMembers.GroupId = UserGroups.Id"). + Where(sq.Eq{"UserGroups.Source": source, "GroupMembers.DeleteAt": 0}) + + query, args, err := builder.ToSql() + if err != nil { + return 0, errors.Wrap(err, "distinct_group_member_count_for_source_tosql") + } + + var count int64 + if err = s.GetReplicaX().Get(&count, query, args...); err != nil { + return 0, errors.Wrapf(err, "failed to select distinct groupmember count for source %q", source) + } + + return count, nil +} + func (s *SqlGroupStore) GroupCountWithAllowReference() (int64, error) { return s.countTableWithSelectAndWhere("COUNT(*)", "UserGroups", sq.Eq{"AllowReference": true, "DeleteAt": 0}) } diff --git a/store/store.go b/store/store.go index 684fdd3d9b..7a47597150 100644 --- a/store/store.go +++ b/store/store.go @@ -886,6 +886,8 @@ type GroupStore interface { // DistinctGroupMemberCount returns the count of records in the GroupMembers table with distinct userID values. DistinctGroupMemberCount() (int64, error) + DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) + // GroupCountWithAllowReference returns the count of records in the Groups table with AllowReference set to true. GroupCountWithAllowReference() (int64, error) diff --git a/store/storetest/group_store.go b/store/storetest/group_store.go index 675b1604b6..ea15f2caac 100644 --- a/store/storetest/group_store.go +++ b/store/storetest/group_store.go @@ -90,6 +90,8 @@ func TestGroupStore(t *testing.T, ss store.Store) { t.Run("GetMember", func(t *testing.T) { groupTestGetMember(t, ss) }) t.Run("GetNonMemberUsersPage", func(t *testing.T) { groupTestGetNonMemberUsersPage(t, ss) }) + + t.Run("DistinctGroupMemberCountForSource", func(t *testing.T) { groupTestDistinctGroupMemberCountForSource(t, ss) }) } func testGroupStoreCreate(t *testing.T, ss store.Store) { @@ -5062,3 +5064,82 @@ func groupTestGetNonMemberUsersPage(t *testing.T, ss store.Store) { require.Error(t, err) require.Nil(t, users) } + +func groupTestDistinctGroupMemberCountForSource(t *testing.T, ss store.Store) { + // get the before counts + customGroupCountBefore, err := ss.Group().DistinctGroupMemberCountForSource(model.GroupSourceCustom) + require.NoError(t, err) + ldapGroupCountBefore, err := ss.Group().DistinctGroupMemberCountForSource(model.GroupSourceLdap) + require.NoError(t, err) + + // create 2 groups, 1 custom and 1 ldap + g1 := &model.Group{ + Name: model.NewString(model.NewId()), + DisplayName: model.NewId(), + Description: model.NewId(), + Source: model.GroupSourceCustom, + RemoteId: model.NewString(model.NewId()), + } + customGroup, err := ss.Group().Create(g1) + require.NoError(t, err) + + g2 := &model.Group{ + Name: model.NewString(model.NewId()), + DisplayName: model.NewId(), + Description: model.NewId(), + Source: model.GroupSourceLdap, + RemoteId: model.NewString(model.NewId()), + } + ldapGroup, err := ss.Group().Create(g2) + require.NoError(t, err) + + // create a couple of users + u1 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user1, nErr := ss.User().Save(u1) + require.NoError(t, nErr) + + u2 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + } + user2, nErr := ss.User().Save(u2) + require.NoError(t, nErr) + + // add both new users to both new groups + _, err = ss.Group().UpsertMember(customGroup.Id, user1.Id) + require.NoError(t, err) + _, err = ss.Group().UpsertMember(ldapGroup.Id, user1.Id) + require.NoError(t, err) + + _, err = ss.Group().UpsertMember(customGroup.Id, user2.Id) + require.NoError(t, err) + _, err = ss.Group().UpsertMember(ldapGroup.Id, user2.Id) + require.NoError(t, err) + + // remove one user from a group to ensure the 'where deleteat = 0' clause is working + _, err = ss.Group().DeleteMember(ldapGroup.Id, user1.Id) + require.NoError(t, err) + + defer func() { + ss.Group().DeleteMember(ldapGroup.Id, user2.Id) + ss.Group().DeleteMember(customGroup.Id, user1.Id) + ss.Group().DeleteMember(customGroup.Id, user2.Id) + + ss.Group().Delete(customGroup.Id) + ss.Group().Delete(ldapGroup.Id) + + ss.User().PermanentDelete(user1.Id) + ss.User().PermanentDelete(user2.Id) + }() + + customGroupCount, err := ss.Group().DistinctGroupMemberCountForSource(model.GroupSourceCustom) + require.NoError(t, err) + require.Equal(t, customGroupCountBefore+2, customGroupCount) + + ldapGroupCount, err := ss.Group().DistinctGroupMemberCountForSource(model.GroupSourceLdap) + require.NoError(t, err) + require.Equal(t, ldapGroupCountBefore+1, ldapGroupCount) +} diff --git a/store/storetest/mocks/GroupStore.go b/store/storetest/mocks/GroupStore.go index 85498100dd..f3f2f839a9 100644 --- a/store/storetest/mocks/GroupStore.go +++ b/store/storetest/mocks/GroupStore.go @@ -372,6 +372,27 @@ func (_m *GroupStore) DistinctGroupMemberCount() (int64, error) { return r0, r1 } +// DistinctGroupMemberCountForSource provides a mock function with given fields: source +func (_m *GroupStore) DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) { + ret := _m.Called(source) + + var r0 int64 + if rf, ok := ret.Get(0).(func(model.GroupSource) int64); ok { + r0 = rf(source) + } else { + r0 = ret.Get(0).(int64) + } + + var r1 error + if rf, ok := ret.Get(1).(func(model.GroupSource) error); ok { + r1 = rf(source) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Get provides a mock function with given fields: groupID func (_m *GroupStore) Get(groupID string) (*model.Group, error) { ret := _m.Called(groupID) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 8fc2af7d5e..11be1b74e2 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -3563,6 +3563,22 @@ func (s *TimerLayerGroupStore) DistinctGroupMemberCount() (int64, error) { return result, err } +func (s *TimerLayerGroupStore) DistinctGroupMemberCountForSource(source model.GroupSource) (int64, error) { + start := timemodule.Now() + + result, err := s.GroupStore.DistinctGroupMemberCountForSource(source) + + elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("GroupStore.DistinctGroupMemberCountForSource", success, elapsed) + } + return result, err +} + func (s *TimerLayerGroupStore) Get(groupID string) (*model.Group, error) { start := time.Now()