Merge branch '11752-improve-active-users-count-license-query' into 'master'

[RUN AS-IF-FOSS] Resolve admin/license timeout on large instances

See merge request gitlab-org/gitlab!46336

app/models/analytics/instance_statistics/measurement.rb

@@ -15,22 +15,10 @@ module Analytics
         pipelines_succeeded: 7,
         pipelines_failed: 8,
         pipelines_canceled: 9,
-        pipelines_skipped: 10
+        pipelines_skipped: 10,
+        billable_users: 11
       }
 
-      IDENTIFIER_QUERY_MAPPING = {
-        identifiers[:projects] => -> { Project },
-        identifiers[:users] => -> { User },
-        identifiers[:issues] => -> { Issue },
-        identifiers[:merge_requests] => -> { MergeRequest },
-        identifiers[:groups] => -> { Group },
-        identifiers[:pipelines] => -> { Ci::Pipeline },
-        identifiers[:pipelines_succeeded] => -> { Ci::Pipeline.success },
-        identifiers[:pipelines_failed] => -> { Ci::Pipeline.failed },
-        identifiers[:pipelines_canceled] => -> { Ci::Pipeline.canceled },
-        identifiers[:pipelines_skipped] => -> { Ci::Pipeline.skipped }
-      }.freeze
-
       validates :recorded_at, :identifier, :count, presence: true
       validates :recorded_at, uniqueness: { scope: :identifier }
 
@@ -39,9 +27,35 @@ module Analytics
       scope :recorded_after, -> (date) { where(self.model.arel_table[:recorded_at].gteq(date)) if date.present? }
       scope :recorded_before, -> (date) { where(self.model.arel_table[:recorded_at].lteq(date)) if date.present? }
 
+      def self.identifier_query_mapping
+        {
+          identifiers[:projects] => -> { Project },
+          identifiers[:users] => -> { User },
+          identifiers[:issues] => -> { Issue },
+          identifiers[:merge_requests] => -> { MergeRequest },
+          identifiers[:groups] => -> { Group },
+          identifiers[:pipelines] => -> { Ci::Pipeline },
+          identifiers[:pipelines_succeeded] => -> { Ci::Pipeline.success },
+          identifiers[:pipelines_failed] => -> { Ci::Pipeline.failed },
+          identifiers[:pipelines_canceled] => -> { Ci::Pipeline.canceled },
+          identifiers[:pipelines_skipped] => -> { Ci::Pipeline.skipped }
+        }
+      end
+
+      # Customized min and max calculation, in some cases using the original scope is too slow.
+      def self.identifier_min_max_queries
+        {}
+      end
+
       def self.measurement_identifier_values
         identifiers.values
       end
+
+      def self.find_latest_or_fallback(identifier)
+        with_identifier(identifier).order_by_latest.first || identifier_query_mapping[identifiers[identifier]].call
+      end
     end
   end
 end
+
+Analytics::InstanceStatistics::Measurement.prepend_if_ee('EE::Analytics::InstanceStatistics::Measurement')

app/workers/analytics/instance_statistics/counter_job_worker.rb

@@ -11,18 +11,24 @@ module Analytics
       idempotent!
 
       def perform(measurement_identifier, min_id, max_id, recorded_at)
-        query_scope = ::Analytics::InstanceStatistics::Measurement::IDENTIFIER_QUERY_MAPPING[measurement_identifier].call
+        query_scope = ::Analytics::InstanceStatistics::Measurement.identifier_query_mapping[measurement_identifier].call
 
         count = if min_id.nil? || max_id.nil? # table is empty
                   0
                 else
-                  Gitlab::Database::BatchCount.batch_count(query_scope, start: min_id, finish: max_id)
+                  counter(query_scope, min_id, max_id)
                 end
 
         return if count == Gitlab::Database::BatchCounter::FALLBACK
 
         InstanceStatistics::Measurement.insert_all([{ recorded_at: recorded_at, count: count, identifier: measurement_identifier }])
       end
+
+      private
+
+      def counter(query_scope, min_id, max_id)
+        Gitlab::Database::BatchCount.batch_count(query_scope, start: min_id, finish: max_id)
+      end
     end
   end
 end

changelogs/unreleased/11752-improve-active-users-count-license-query.yml

+---
+title: Resolve admin/license timeout on large instances
+merge_request: 46336
+author:
+type: performance

db/migrate/20201103171537_add_index_active_billable_users_to_user.rb

+# frozen_string_literal: true
+
+class AddIndexActiveBillableUsersToUser < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'active_billable_users'
+  HUMAN_TYPE = 'NULL'
+  HUMAN_SVC_BOT_TYPES = "#{HUMAN_TYPE}, 6, 4"
+  BOT_TYPES = '2,6,1,3,7,8'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :users, :id, name: INDEX_NAME, where: "(state = 'active' AND (user_type is #{HUMAN_TYPE} or user_type in (#{HUMAN_SVC_BOT_TYPES}))) and ((users.user_type IS #{HUMAN_TYPE}) OR (users.user_type <> ALL ('{#{BOT_TYPES}}')))"
+  end
+
+  def down
+    remove_concurrent_index_by_name(:users, INDEX_NAME)
+  end
+end

db/schema_migrations/20201103171537

+3120428427e3c0c94799501b1d31f384c98899f2ef9bda6f95066c94afd5ecf8
\ No newline at end of file

db/structure.sql

@@ -19862,6 +19862,8 @@ CREATE INDEX product_analytics_events_exper_project_id_collector_tstamp_idx9 ON
 
 CREATE INDEX product_analytics_events_experi_project_id_collector_tstamp_idx ON gitlab_partitions_static.product_analytics_events_experimental_00 USING btree (project_id, collector_tstamp);
 
+CREATE INDEX active_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[NULL::integer, 6, 4]))) AND ((user_type IS NULL) OR (user_type <> ALL ('{2,6,1,3,7,8}'::smallint[]))));
+
 CREATE INDEX analytics_index_audit_events_on_created_at_and_author_id ON audit_events USING btree (created_at, author_id);
 
 CREATE INDEX analytics_index_events_on_created_at_and_author_id ON events USING btree (created_at, author_id);

doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md

@@ -458,7 +458,7 @@ user.skip_reconfirmation!
 User.active.count
 
 # Users taking a seat on the instance
-License.current.current_active_users_count
+User.billable.count
 
 # The historical max on the instance as of the past year
 ::HistoricalData.max_historical_user_count

doc/subscriptions/self_managed/index.md

@@ -157,7 +157,7 @@ We recommend following these steps during renewal:
    | Field | Description |
    |:------|:------------|
    | Users in License | The number of users you've paid for in the current license loaded on the system. This does not include the amount you've paid for `Users over license` during renewal. |
-   | Active users | The number of current active users on your system. |
+   | Active users | The daily count of active users on your system. |
    | Maximum users | The highest number of active users on your system during the term of the loaded license. If this number exceeds your users in license count at any point, you incur users over license. |
    | Users over license | The number of users that exceed the `Users in License` for the current license term. Charges for this number of users will be incurred at the next renewal. |
 

ee/app/helpers/license_helper.rb

@@ -6,16 +6,7 @@ module LicenseHelper
 
   delegate :new_admin_license_path, to: 'Gitlab::Routing.url_helpers'
 
-  def current_active_user_count
-    License.current&.current_active_users_count || active_user_count
-  end
-
-  def maximum_user_count
-    License.current&.maximum_user_count || 0
-  end
-
   def seats_calculation_message(license)
-    return unless license.present?
     return unless license.exclude_guests_from_active_count?
 
     s_("Users with a Guest role or those who don't belong to a Project or Group will not use a seat from your license.")
@@ -49,11 +40,13 @@ module LicenseHelper
     !Gitlab::CurrentSettings.should_check_namespace_plan? && show_promotions? && show_callout?('promote_advanced_search_dismissed') && !License.feature_available?(:elastic_search)
   end
 
-  extend self
-
-  private
-
-  def active_user_count
-    License.current_active_users.count
+  def licensed_users(license)
+    if license.restricted?(:active_user_count)
+      number_with_delimiter(license.restrictions[:active_user_count])
+    else
+      _('Unlimited')
+    end
   end
+
+  extend self
 end

ee/app/helpers/license_monitoring_helper.rb

@@ -12,6 +12,14 @@ module LicenseMonitoringHelper
     current_user&.admin? && current_license.active_user_count_threshold_reached?
   end
 
+  def users_over_license
+    strong_memoize(:users_over_license) do
+      license_is_over_capacity? ? current_license_overage : 0
+    end
+  end
+
+  private
+
   def license_is_over_capacity?
     return if ::Gitlab.com?
     return if license_not_available_or_trial?
@@ -19,8 +27,6 @@ module LicenseMonitoringHelper
     current_license_overage > 0
   end
 
-  private
-
   def license_not_available_or_trial?
     current_license.nil? || current_license.trial?
   end

ee/app/models/ee/analytics/instance_statistics/measurement.rb

+# frozen_string_literal: true
+
+module EE
+  module Analytics
+    module InstanceStatistics
+      # Measurement EE mixin
+      #
+      # This module is intended to encapsulate EE-specific model logic
+      # and be prepended in the `Measurement` model
+      module Measurement
+        extend ActiveSupport::Concern
+
+        class_methods do
+          extend ::Gitlab::Utils::Override
+
+          override :identifier_query_mapping
+          def identifier_query_mapping
+            super.merge(
+              {
+                identifiers[:billable_users] => -> { ::User.billable }
+              }
+            )
+          end
+
+          override :identifier_min_max_queries
+          def identifier_min_max_queries
+            super.merge(
+              {
+                identifiers[:billable_users] => {
+                  minimum_query: -> { ::User.minimum(:id) },
+                  maximum_query: -> { ::User.maximum(:id) }
+                }
+              }
+            )
+          end
+        end
+      end
+    end
+  end
+end

ee/app/models/ee/user.rb

@@ -74,7 +74,14 @@ module EE
 
       scope :managed_by, ->(group) { where(managing_group: group) }
 
-      scope :excluding_guests, -> { joins(:members).merge(::Member.non_guests).distinct }
+      scope :excluding_guests, -> do
+        subquery = ::Member
+          .select(1)
+          .where(::Member.arel_table[:user_id].eq(::User.arel_table[:id]))
+          .merge(::Member.non_guests)
+
+        where('EXISTS (?)', subquery)
+      end
 
       scope :subscribed_for_admin_email, -> { where(admin_email_unsubscribed_at: nil) }
       scope :ldap, -> { joins(:identities).where('identities.provider LIKE ?', 'ldap%') }
@@ -140,6 +147,16 @@ module EE
           all
         end
       end
+
+      def billable
+        scope = active.without_bots
+
+        License.with_valid_license do |license|
+          scope = scope.excluding_guests if license.exclude_guests_from_active_count?
+        end
+
+        scope
+      end
     end
 
     def cannot_be_admin_and_auditor

ee/app/models/historical_data.rb

@@ -12,7 +12,7 @@ class HistoricalData < ApplicationRecord
     def track!
       create!(
         recorded_at:        Time.current,
-        active_user_count:  License.load_license&.current_active_users_count
+        active_user_count:  License.load_license&.daily_billable_users_count
       )
     end
 

ee/app/models/license.rb

@@ -2,6 +2,7 @@
 
 class License < ApplicationRecord
   include ActionView::Helpers::NumberHelper
+  include Gitlab::Utils::StrongMemoize
 
   STARTER_PLAN = 'starter'.freeze
   PREMIUM_PLAN = 'premium'.freeze
@@ -308,10 +309,6 @@ class License < ApplicationRecord
       yield(current_license) if block_given?
     end
 
-    def current_active_users
-      User.active.without_bots
-    end
-
     private
 
     def load_future_dated
@@ -426,11 +423,9 @@ class License < ApplicationRecord
     end
   end
 
-  def current_active_users_count
-    @current_active_users_count ||= begin
-      scope = self.class.current_active_users
-      scope = scope.excluding_guests if exclude_guests_from_active_count?
-      scope.count
+  def daily_billable_users_count
+    strong_memoize(:daily_billable_users_count) do
+      ::Analytics::InstanceStatistics::Measurement.find_latest_or_fallback(:billable_users).count
     end
   end
 
@@ -459,7 +454,7 @@ class License < ApplicationRecord
   def overage(user_count = nil)
     return 0 if restricted_user_count.nil?
 
-    user_count ||= current_active_users_count
+    user_count ||= daily_billable_users_count
 
     [user_count - restricted_user_count, 0].max
   end
@@ -473,7 +468,7 @@ class License < ApplicationRecord
   end
 
   def maximum_user_count
-    [historical_max, current_active_users_count].max
+    [historical_max, daily_billable_users_count].max
   end
 
   def historical_max_with_default_period
@@ -516,18 +511,18 @@ class License < ApplicationRecord
 
   def active_user_count_threshold_reached?
     return false if restricted_user_count.nil?
-    return false if current_active_users_count <= 1
-    return false if current_active_users_count > restricted_user_count
+    return false if daily_billable_users_count <= 1
+    return false if daily_billable_users_count > restricted_user_count
 
     active_user_count_threshold[:value] >= if active_user_count_threshold[:percentage]
-                                             remaining_user_count.fdiv(current_active_users_count) * 100
+                                             remaining_user_count.fdiv(daily_billable_users_count) * 100
                                            else
                                              remaining_user_count
                                            end
   end
 
   def remaining_user_count
-    restricted_user_count - current_active_users_count
+    restricted_user_count - daily_billable_users_count
   end
 
   private
@@ -576,12 +571,12 @@ class License < ApplicationRecord
     return unless restricted_user_count
 
     if previous_user_count && (prior_historical_max <= previous_user_count)
-      return if restricted_user_count >= current_active_users_count
+      return if restricted_user_count >= daily_billable_users_count
     else
       return if restricted_user_count_with_threshold >= prior_historical_max
     end
 
-    user_count = prior_historical_max == 0 ? current_active_users_count : prior_historical_max
+    user_count = prior_historical_max == 0 ? daily_billable_users_count : prior_historical_max
 
     add_limit_error(current_period: prior_historical_max == 0, user_count: user_count)
   end
@@ -594,12 +589,12 @@ class License < ApplicationRecord
     expected_trueup_qty = if previous_user_count
                             max_historical - previous_user_count
                           else
-                            max_historical - current_active_users_count
+                            max_historical - daily_billable_users_count
                           end
 
     if trueup_qty >= expected_trueup_qty
-      if restricted_user_count < current_active_users_count
-        add_limit_error(user_count: current_active_users_count)
+      if restricted_user_count < daily_billable_users_count
+        add_limit_error(user_count: daily_billable_users_count)
       end
     else
       message = ["You have applied a True-up for #{trueup_qty} #{"user".pluralize(trueup_qty)}"]

ee/app/views/admin/licenses/_breakdown.html.haml

-- if @license.restricted?(:active_user_count)
-  - restricted = @license.restrictions[:active_user_count]
-  - licensed_users = number_with_delimiter(restricted)
-- else
-  - licensed_users = _('Unlimited')
-
-- max_user_count = maximum_user_count
-
-- if license_is_over_capacity?
-  - users_over_license = current_license_overage
-- else
-  - users_over_license = 0
-
 - true_up_url = 'https://about.gitlab.com/license-faq/'
 - true_up_link_start = '<a href="%{url}">'.html_safe % { url: true_up_url }
 - billable_users_url = help_page_path('subscriptions/self_managed/index', anchor: 'billable-users')
@@ -23,7 +10,7 @@
       .well-segment.well-centered
         %h3.center
           = _('Users in License:')
-          = licensed_users
+          = licensed_users(@license)
         %hr
         - if @license.will_expire?
           = _('Your license is valid from')
@@ -37,7 +24,7 @@
       .well-segment.well-centered
         %h3.center
           = _('Billable Users:')
-          = number_with_delimiter current_active_user_count
+          = number_with_delimiter @license.daily_billable_users_count
         %hr
           %p
           = _('This is the number of %{billable_users_link_start}billable users%{link_end} on your installation, and this is the minimum number you need to purchase when you renew your license.').html_safe % { billable_users_link_start: billable_users_link_start, link_end: link_end }
@@ -47,7 +34,7 @@
       .well-segment.well-centered
         %h3.center
           = _('Maximum Users:')
-          = number_with_delimiter max_user_count
+          = number_with_delimiter @license.maximum_user_count
         %hr
           = _('This is the highest peak of users on your installation since the license started.')
   .col-sm-6.d-flex.pr-0
@@ -57,9 +44,9 @@
           = _('Users over License:')
           = number_with_delimiter users_over_license
         %hr
-          - if license_is_over_capacity?
+          - if users_over_license > 0
             .gl-alert.gl-alert-info.gl-mb-3{ role: 'alert' }
               = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
               .gl-alert-body
                 = s_('Your instance has exceeded your subscription\'s licensed user count.')
-          = _('You\'ll be charged for %{true_up_link_start}users over license%{link_end} on a quartely or annual basis, depending on the terms of your agreement.').html_safe % { true_up_link_start: true_up_link_start, link_end: link_end }
+          = _('You\'ll be charged for %{true_up_link_start}users over license%{link_end} on a quarterly or annual basis, depending on the terms of your agreement.').html_safe % { true_up_link_start: true_up_link_start, link_end: link_end }

ee/app/views/license_mailer/approaching_active_user_count_limit.html.haml

@@ -5,7 +5,7 @@
 %p
   = _("Dear Administrator,")
 %p
-  = html_escape(_("We would like to inform you that your subscription GitLab Enterprise Edition %{plan_name} is nearing its user limit. You have %{active_user_count} active users, which is almost at the user limit of %{maximum_user_count}.")) % { plan_name: @license.plan.titleize, active_user_count: @license.current_active_users_count, maximum_user_count: @license.restricted_user_count }
+  = html_escape(_("We would like to inform you that your subscription GitLab Enterprise Edition %{plan_name} is nearing its user limit. You have %{active_user_count} active users, which is almost at the user limit of %{maximum_user_count}.")) % { plan_name: @license.plan.titleize, active_user_count: @license.daily_billable_users_count, maximum_user_count: @license.restricted_user_count }
 %p
   = html_escape(_("If the number of active users exceeds the user limit, you will be charged for the number of %{users_over_license_link} at your next license reconciliation.")) % { users_over_license_link: users_over_license_link }
 

ee/lib/api/license.rb

@@ -51,7 +51,7 @@ module API
       get do
         licenses = LicensesFinder.new(current_user).execute
 
-        present licenses, with: EE::API::Entities::GitlabLicense, current_active_users_count: ::License.current&.current_active_users_count
+        present licenses, with: EE::API::Entities::GitlabLicense, current_active_users_count: ::License.current&.daily_billable_users_count
       end
     end
   end

ee/lib/ee/api/entities/gitlab_license_with_active_users.rb

@@ -4,8 +4,8 @@ module EE
   module API
     module Entities
       class GitlabLicenseWithActiveUsers < GitlabLicense
-        expose :active_users do |license, options|
-          ::License.current_active_users.count
+        expose :active_users do |license, _options|
+          license.daily_billable_users_count
         end
       end
     end

ee/spec/helpers/license_helper_spec.rb

@@ -8,48 +8,6 @@ RSpec.describe LicenseHelper do
     allow(Rails.application.routes).to receive(:default_url_options).and_return(url_options)
   end
 
-  describe '#current_active_user_count' do
-    let(:license) { create(:license) }
-
-    context 'when there is a license' do
-      it 'returns License#current_active_users_count' do
-        allow(License).to receive(:current).and_return(license)
-
-        expect(license).to receive(:current_active_users_count).and_return(311)
-        expect(current_active_user_count).to eq(311)
-      end
-    end
-
-    context 'when there is NOT a license' do
-      it 'returns the number of active users' do
-        allow(License).to receive(:current).and_return(nil)
-
-        expect(current_active_user_count).to eq(License.current_active_users.count)
-      end
-    end
-  end
-
-  describe '#maximum_user_count' do
-    context 'when current license is set' do
-      it 'returns the maximum_user_count for the current license' do
-        license = double
-        allow(License).to receive(:current).and_return(license)
-        count = 5
-        allow(license).to receive(:maximum_user_count).and_return(count)
-
-        expect(maximum_user_count).to eq(count)
-      end
-    end
-
-    context 'when current license is not set' do
-      it 'returns 0' do
-        allow(License).to receive(:current).and_return(nil)
-
-        expect(maximum_user_count).to eq(0)
-      end
-    end
-  end
-
   describe '#current_license_title' do
     context 'when there is a current license' do
       it 'returns the plan titleized if it has a plan associated to it' do
@@ -80,31 +38,43 @@ RSpec.describe LicenseHelper do
   describe '#seats_calculation_message' do
     subject { seats_calculation_message(license) }
 
-    context 'with a license' do
-      let(:license) { double("License", 'exclude_guests_from_active_count?' => exclude_guests) }
+    let(:license) { double('License', 'exclude_guests_from_active_count?' => exclude_guests) }
 
-      context 'and guest are excluded from the active count' do
-        let(:exclude_guests) { true }
+    context 'and guest are excluded from the active count' do
+      let(:exclude_guests) { true }
 
-        it 'returns the message' do
-          expect(subject).to eq("Users with a Guest role or those who don't belong to a Project or Group will not use a seat from your license.")
-        end
+      it 'returns the message' do
+        expect(subject).to eq("Users with a Guest role or those who don't belong to a Project or Group will not use a seat from your license.")
       end
+    end
 
-      context 'and guest are NOT excluded from the active count' do
-        let(:exclude_guests) { false }
+    context 'and guest are NOT excluded from the active count' do
+      let(:exclude_guests) { false }
 
-        it 'returns nil' do
-          expect(subject).to be_blank
-        end
+      it 'returns nil' do
+        expect(subject).to be_blank
       end
     end
+  end
 
-    context 'when the license is blank' do
-      let(:license) { nil }
+  describe '#licensed_users' do
+    context 'with a restricted license count' do
+      let(:license) do
+        double('License', restricted?: { active_user_count: true }, restrictions: { active_user_count: 5 })
+      end
 
-      it 'returns nil' do
-        expect(subject).to be_blank
+      it 'returns a number as string' do
+        license = double('License', restricted?: true, restrictions: { active_user_count: 5 })
+
+        expect(licensed_users(license)).to eq '5'
+      end
+    end
+
+    context 'without a restricted license count' do
+      let(:license) { double('License', restricted?: false) }
+
+      it 'returns Unlimited' do
+        expect(licensed_users(license)).to eq 'Unlimited'
       end
     end
   end

ee/spec/helpers/license_monitoring_helper_spec.rb

@@ -3,15 +3,14 @@
 require 'spec_helper'
 
 RSpec.describe LicenseMonitoringHelper do
-  let_it_be(:admin) { create(:admin) }
-  let_it_be(:user) { create(:user) }
-  let_it_be(:license_seats_limit) { 10 }
-
-  let_it_be(:license) do
-    create(:license, data: build(:gitlab_license, restrictions: { active_user_count: license_seats_limit }).export)
-  end
-
   describe '#show_active_user_count_threshold_banner?' do
+    let_it_be(:admin) { create(:admin) }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:license_seats_limit) { 10 }
+    let_it_be(:license) do
+      create(:license, data: build(:gitlab_license, restrictions: { active_user_count: license_seats_limit }).export)
+    end
+
     subject { helper.show_active_user_count_threshold_banner? }
 
     shared_examples 'banner hidden when below the threshold' do
@@ -65,7 +64,7 @@ RSpec.describe LicenseMonitoringHelper do
       context 'when current active user count greater than total user count' do
         before do
           allow(license).to receive(:restricted_user_count).and_return(license_seats_limit)
-          allow(license).to receive(:current_active_users_count).and_return(license_seats_limit + 1)
+          allow(license).to receive(:daily_billable_users_count).and_return(license_seats_limit + 1)
           allow(License).to receive(:current).and_return(license)
         end
 
@@ -107,4 +106,30 @@ RSpec.describe LicenseMonitoringHelper do
       end
     end
   end
+
+  describe '#users_over_license' do
+    context 'with an user overage' do
+      let(:license) { build(:license) }
+
+      before do
+        allow(helper).to receive(:license_is_over_capacity?).and_return true
+        allow(License).to receive(:current).and_return(license)
+        allow(license).to receive(:overage_with_historical_max) { 5 }
+      end
+
+      it 'shows overage as a number' do
+        expect(helper.users_over_license).to eq 5
+      end
+    end
+
+    context 'without an user overage' do
+      before do
+        allow(helper).to receive(:license_is_over_capacity?).and_return false
+      end
+
+      it 'shows overage as a number' do
+        expect(helper.users_over_license).to eq 0
+      end
+    end
+  end
 end

ee/spec/mailers/license_mailer_spec.rb

@@ -17,7 +17,7 @@ RSpec.describe LicenseMailer do
     subject { described_class.approaching_active_user_count_limit(recipients) }
 
     before do
-      allow(license).to receive(:current_active_users_count).and_return(active_user_count)
+      allow(license).to receive(:daily_billable_users_count).and_return(active_user_count)
       allow(License).to receive(:current).and_return(license)
     end
 

ee/spec/models/ee/analytics/instance_statistics/measurement_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Analytics::InstanceStatistics::Measurement do
+  describe '.identifier_query_mapping' do
+    subject { described_class.identifier_query_mapping.keys }
+
+    it { is_expected.to include described_class.identifiers[:billable_users] }
+  end
+
+  describe '.identifier_min_max_queries' do
+    subject { described_class.identifier_min_max_queries.keys }
+
+    it { is_expected.to include described_class.identifiers[:billable_users] }
+  end
+end

ee/spec/models/historical_data_spec.rb

@@ -29,7 +29,7 @@ RSpec.describe HistoricalData do
 
   describe ".track!" do
     before do
-      allow(License).to receive(:current_active_users).and_return([1, 2, 3, 4, 5])
+      allow(User).to receive(:billable).and_return([1, 2, 3, 4, 5])
     end
 
     it "creates a new historical data record" do

ee/spec/models/license_spec.rb

@@ -120,7 +120,7 @@ RSpec.describe License do
     end
 
     describe "Historical active user count" do
-      let(:active_user_count) { described_class.current_active_users.count + 10 }
+      let(:active_user_count) { described_class.current.daily_billable_users_count + 10 }
       let(:date)              { described_class.current.starts_at }
       let!(:historical_data)  { HistoricalData.create!(recorded_at: date, active_user_count: active_user_count) }
 
@@ -136,7 +136,7 @@ RSpec.describe License do
 
           gl_license.restrictions = {
             previous_user_count: 1,
-            active_user_count: described_class.current_active_users.count - 1
+            active_user_count: described_class.current.daily_billable_users_count - 1
           }
 
           HistoricalData.delete_all
@@ -637,21 +637,6 @@ RSpec.describe License do
         end
       end
     end
-
-    describe '.current_active_users' do
-      before do
-        create(:group_member)
-        create(:group_member, user: create(:admin))
-        create(:group_member, user: create(:user, :bot))
-        create(:group_member, user: create(:user, :project_bot))
-        create(:group_member, user: create(:user, :ghost))
-        create(:group_member).user.deactivate!
-      end
-
-      it 'includes guests in the count' do
-        expect(license.current_active_users_count).to eq(2)
-      end
-    end
   end
 
   describe "#md5" do
@@ -816,7 +801,7 @@ RSpec.describe License do
     end
   end
 
-  describe '#current_active_users_count' do
+  describe '#daily_billable_users_count' do
     before_all do
       create(:group_member)
       create(:group_member, user: create(:admin))
@@ -829,17 +814,15 @@ RSpec.describe License do
 
     context 'when license is not for Ultimate plan' do
       it 'includes guests in the count' do
-        expect(license.current_active_users_count).to eq(3)
+        expect(license.daily_billable_users_count).to eq(3)
       end
     end
 
     context 'when license is for Ultimate plan' do
-      before do
-        allow(license).to receive(:plan).and_return(License::ULTIMATE_PLAN)
-      end
-
       it 'excludes guests in the count' do
-        expect(license.current_active_users_count).to eq(2)
+        new_license = create(:license, plan: License::ULTIMATE_PLAN)
+
+        expect(new_license.daily_billable_users_count).to eq(2)
       end
     end
   end
@@ -857,8 +840,8 @@ RSpec.describe License do
       expect(license.overage(14)).to eq(4)
     end
 
-    it 'returns the difference using current_active_users_count as user_count if no user_count argument provided' do
-      allow(license).to receive(:current_active_users_count) { 110 }
+    it 'returns the difference using daily_billable_users_count as user_count if no user_count argument provided' do
+      allow(license).to receive(:daily_billable_users_count) { 110 }
       allow(license).to receive(:restricted_user_count) { 100 }
 
       expect(license.overage).to eq(10)
@@ -874,7 +857,7 @@ RSpec.describe License do
   describe '#maximum_user_count' do
     subject { license.maximum_user_count }
 
-    where(:current_active_users_count, :historical_max, :expected) do
+    where(:daily_billable_users_count, :historical_max, :expected) do
       100 | 50  | 100
       50  | 100 | 100
       50  | 50  | 50
@@ -882,7 +865,7 @@ RSpec.describe License do
 
     with_them do
       before do
-        allow(license).to receive(:current_active_users_count) { current_active_users_count }
+        allow(license).to receive(:daily_billable_users_count) { daily_billable_users_count }
         allow(license).to receive(:historical_max) { historical_max }
       end
 
@@ -1116,7 +1099,7 @@ RSpec.describe License do
       with_them do
         before do
           allow(license).to receive(:restricted_user_count).and_return(restricted_user_count)
-          allow(license).to receive(:current_active_users_count).and_return(active_user_count)
+          allow(license).to receive(:daily_billable_users_count).and_return(active_user_count)
         end
 
         it { is_expected.not_to be_nil }
@@ -1128,7 +1111,7 @@ RSpec.describe License do
   describe '#active_user_count_threshold_reached?' do
     subject { license.active_user_count_threshold_reached? }
 
-    where(:restricted_user_count, :current_active_users_count, :result) do
+    where(:restricted_user_count, :daily_billable_users_count, :result) do
       10   | 9   | true
       nil  | 9   | false
       10   | 15  | false
@@ -1137,7 +1120,7 @@ RSpec.describe License do
 
     with_them do
       before do
-        allow(license).to receive(:current_active_users_count).and_return(current_active_users_count)
+        allow(license).to receive(:daily_billable_users_count).and_return(daily_billable_users_count)
         allow(license).to receive(:restricted_user_count).and_return(restricted_user_count)
       end
 

ee/spec/models/user_spec.rb

@@ -580,6 +580,71 @@ RSpec.describe User do
     end
   end
 
+  describe '.billable' do
+    let_it_be(:bot_user) { create(:user, :bot) }
+    let_it_be(:regular_user) { create(:user) }
+    let_it_be(:project_reporter_user) { create(:project_member, :reporter).user }
+    let_it_be(:project_guest_user) { create(:project_member, :guest).user }
+
+    subject(:users) { described_class.billable }
+
+    context 'with guests' do
+      it 'validates the sql matches the specific index we have' do
+        expected_sql = <<~SQL
+          SELECT "users".* FROM "users"
+          WHERE ("users"."state" IN ('active'))
+          AND
+          ("users"."user_type" IS NULL OR "users"."user_type" IN (NULL, 6, 4))
+          AND
+          ("users"."user_type" IS NULL OR "users"."user_type" NOT IN (2, 6, 1, 3, 7, 8))
+        SQL
+
+        expect(users.to_sql.squish).to eq expected_sql.squish
+      end
+
+      it 'returns users' do
+        expect(users).to include(project_reporter_user)
+        expect(users).to include(project_guest_user)
+        expect(users).to include(regular_user)
+
+        expect(users).not_to include(bot_user)
+      end
+    end
+
+    context 'without guests' do
+      before do
+        license = double('License', exclude_guests_from_active_count?: true, trial?: false)
+        allow(License).to receive(:current) { license }
+      end
+
+      it 'validates the sql matches the specific index we have' do
+        expected_sql = <<~SQL
+          SELECT "users".* FROM "users"
+          WHERE ("users"."state" IN ('active'))
+          AND
+          ("users"."user_type" IS NULL OR "users"."user_type" IN (NULL, 6, 4))
+          AND
+          ("users"."user_type" IS NULL OR "users"."user_type" NOT IN (2, 6, 1, 3, 7, 8))
+          AND
+          (EXISTS (SELECT 1 FROM "members"
+            WHERE "members"."user_id" = "users"."id"
+            AND
+            (members.access_level > 10)))
+        SQL
+
+        expect(users.to_sql.squish).to eq expected_sql.squish
+      end
+
+      it 'returns users' do
+        expect(users).to include(project_reporter_user)
+
+        expect(users).not_to include(regular_user)
+        expect(users).not_to include(project_guest_user)
+        expect(users).not_to include(bot_user)
+      end
+    end
+  end
+
   describe '#group_managed_account?' do
     subject { user.group_managed_account? }
 

lib/gitlab/analytics/instance_statistics/workers_argument_builder.rb

@@ -11,21 +11,36 @@ module Gitlab
 
         def execute
           measurement_identifiers.map do |measurement_identifier|
-            query_scope = ::Analytics::InstanceStatistics::Measurement::IDENTIFIER_QUERY_MAPPING[measurement_identifier]&.call
+            query_scope = query_mappings[measurement_identifier]&.call
 
             next if query_scope.nil?
 
-            # Determining the query range (id range) as early as possible in order to get more accurate counts.
-            start = query_scope.minimum(:id)
-            finish = query_scope.maximum(:id)
-
-            [measurement_identifier, start, finish, recorded_at]
+            [measurement_identifier, *determine_start_and_finish(measurement_identifier, query_scope), recorded_at]
           end.compact
         end
 
         private
 
         attr_reader :measurement_identifiers, :recorded_at
+
+        # Determining the query range (id range) as early as possible in order to get more accurate counts.
+        def determine_start_and_finish(measurement_identifier, query_scope)
+          queries = custom_min_max_queries[measurement_identifier]
+
+          if queries
+            [queries[:minimum_query].call, queries[:maximum_query].call]
+          else
+            [query_scope.minimum(:id), query_scope.maximum(:id)]
+          end
+        end
+
+        def custom_min_max_queries
+          ::Analytics::InstanceStatistics::Measurement.identifier_min_max_queries
+        end
+
+        def query_mappings
+          ::Analytics::InstanceStatistics::Measurement.identifier_query_mapping
+        end
       end
     end
   end

locale/gitlab.pot

@@ -31089,7 +31089,7 @@ msgstr ""
 msgid "You won't be able to pull or push project code via SSH until you add an SSH key to your profile"
 msgstr ""
 
-msgid "You'll be charged for %{true_up_link_start}users over license%{link_end} on a quartely or annual basis, depending on the terms of your agreement."
+msgid "You'll be charged for %{true_up_link_start}users over license%{link_end} on a quarterly or annual basis, depending on the terms of your agreement."
 msgstr ""
 
 msgid "You'll be signed out from your current account automatically."

spec/graphql/types/admin/analytics/instance_statistics/measurement_identifier_enum_spec.rb

@@ -6,7 +6,10 @@ RSpec.describe GitlabSchema.types['MeasurementIdentifier'] do
   specify { expect(described_class.graphql_name).to eq('MeasurementIdentifier') }
 
   it 'exposes all the existing identifier values' do
-    identifiers = Analytics::InstanceStatistics::Measurement.identifiers.keys.map(&:upcase)
+    ee_only_identifiers = %w[billable_users]
+    identifiers = Analytics::InstanceStatistics::Measurement.identifiers.keys.reject do |x|
+      ee_only_identifiers.include?(x)
+    end.map(&:upcase)
 
     expect(described_class.values.keys).to match_array(identifiers)
   end

spec/lib/gitlab/analytics/instance_statistics/workers_argument_builder_spec.rb

@@ -42,5 +42,40 @@ RSpec.describe Gitlab::Analytics::InstanceStatistics::WorkersArgumentBuilder do
         ])
       end
     end
+
+    context 'when custom min and max queries are present' do
+      let(:min_id) { User.second.id }
+      let(:max_id) { User.maximum(:id) }
+      let(:users_measurement_identifier) { ::Analytics::InstanceStatistics::Measurement.identifiers.fetch(:users) }
+
+      before do
+        create_list(:user, 2)
+
+        min_max_queries = {
+          ::Analytics::InstanceStatistics::Measurement.identifiers[:users] => {
+            minimum_query: -> { min_id },
+            maximum_query: -> { max_id }
+          }
+        }
+
+        allow(::Analytics::InstanceStatistics::Measurement).to receive(:identifier_min_max_queries) { min_max_queries }
+      end
+
+      subject do
+        described_class.new(measurement_identifiers: [users_measurement_identifier], recorded_at: recorded_at)
+          .execute
+      end
+
+      it 'uses custom min/max for ids' do
+        expect(subject).to eq([
+                                [
+                                  users_measurement_identifier,
+                                  min_id,
+                                  max_id,
+                                  recorded_at
+                                ]
+                              ])
+      end
+    end
   end
 end

spec/models/analytics/instance_statistics/measurement_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do
 
   describe 'identifiers enum' do
     it 'maps to the correct values' do
-      expect(described_class.identifiers).to eq({
+      identifiers = {
         projects: 1,
         users: 2,
         issues: 3,
@@ -24,8 +24,11 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do
         pipelines_succeeded: 7,
         pipelines_failed: 8,
         pipelines_canceled: 9,
-        pipelines_skipped: 10
-      }.with_indifferent_access)
+        pipelines_skipped: 10,
+        billable_users: 11
+      }
+
+      expect(described_class.identifiers).to eq(identifiers.with_indifferent_access)
     end
   end
 
@@ -75,11 +78,41 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do
     end
   end
 
-  describe '#measurement_identifier_values' do
-    let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifiers.size }
+  describe '.identifier_query_mapping' do
+    subject { described_class.identifier_query_mapping }
+
+    it { is_expected.to be_a Hash }
+  end
+
+  describe '.identifier_min_max_queries' do
+    subject { described_class.identifier_min_max_queries }
+
+    it { is_expected.to be_a Hash }
+  end
+
+  describe '.measurement_identifier_values' do
+    let(:expected_count) { described_class.identifiers.size }
 
     subject { described_class.measurement_identifier_values.count }
 
     it { is_expected.to eq(expected_count) }
   end
+
+  describe '.find_latest_or_fallback' do
+    subject(:count) { described_class.find_latest_or_fallback(:pipelines_skipped).count }
+
+    context 'with instance statistics' do
+      let!(:measurement) { create(:instance_statistics_measurement, :pipelines_skipped_count) }
+
+      it 'returns the latest stored measurement' do
+        expect(count).to eq measurement.count
+      end
+    end
+
+    context 'without instance statistics' do
+      it 'returns the realtime query of the measurement' do
+        expect(count).to eq 0
+      end
+    end
+  end
 end

spec/workers/analytics/instance_statistics/count_job_trigger_worker_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe Analytics::InstanceStatistics::CountJobTriggerWorker do
   it_behaves_like 'an idempotent worker'
 
   context 'triggers a job for each measurement identifiers' do
-    let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifiers.size }
+    let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifier_query_mapping.keys.size }
 
     it 'triggers CounterJobWorker jobs' do
       subject.perform