Merge branch 'mwaw/333681-filter-usage_data-payload-considering-metric-category' into 'master'

[RUN ALL RSPEC] [RUN AS-IF-FOSS] Resolve "Filter usage_data payload considering metric category"

See merge request gitlab-org/gitlab!64500

app/services/service_ping/build_payload_service.rb

+# frozen_string_literal: true
+
+module ServicePing
+  class BuildPayloadService
+    def execute
+      return {} unless allowed_to_report?
+
+      raw_payload
+    end
+
+    private
+
+    def allowed_to_report?
+      product_intelligence_enabled? && !User.single_user&.requires_usage_stats_consent?
+    end
+
+    def product_intelligence_enabled?
+      ::Gitlab::CurrentSettings.usage_ping_enabled?
+    end
+
+    def raw_payload
+      @raw_payload ||= ::Gitlab::UsageData.data(force_refresh: true)
+    end
+  end
+end
+
+ServicePing::BuildPayloadService.prepend_mod_with('ServicePing::BuildPayloadService')

ee/app/services/ee/service_ping/build_payload_service.rb

+# frozen_string_literal: true
+
+module EE
+  module ServicePing
+    module BuildPayloadService
+      extend ::Gitlab::Utils::Override
+
+      STANDARD_CATEGORY = 'Standard'
+      SUBSCRIPTION_CATEGORY = 'Subscription'
+      OPTIONAL_CATEGORY = 'Optional'
+      OPERATIONAL_CATEGORY = 'Operational'
+
+      override :execute
+      def execute
+        return super unless ::License.current.present?
+
+        filtered_usage_data(super)
+      end
+
+      private
+
+      override :product_intelligence_enabled?
+      def product_intelligence_enabled?
+        ::License.current&.usage_ping? || super
+      end
+
+      def filtered_usage_data(payload = raw_payload, parents = [])
+        payload.keep_if do |label, node|
+          if leaf?(node)
+            permitted_categories.include?(metric_category(label, parents))
+          else
+            filtered_usage_data(node, parents.dup << label)
+          end
+        end
+      end
+
+      def permitted_categories
+        @permitted_categories ||= collect_permitted_categories
+      end
+
+      def collect_permitted_categories
+        categories = [STANDARD_CATEGORY, SUBSCRIPTION_CATEGORY]
+        categories << OPTIONAL_CATEGORY if ::Gitlab::CurrentSettings.usage_ping_enabled?
+        categories << OPERATIONAL_CATEGORY if ::License.current.usage_ping?
+        categories
+      end
+
+      def metric_category(key, parent_keys)
+        key_path = parent_keys.dup.append(key).join('.')
+        metric_definitions[key_path]&.attributes&.fetch(:data_category, OPTIONAL_CATEGORY)
+      end
+
+      def metric_definitions
+        @metric_definitions ||= ::Gitlab::Usage::MetricDefinition.definitions
+      end
+
+      def leaf?(node)
+        !node.is_a?(Hash)
+      end
+    end
+  end
+end

ee/spec/services/ee/service_ping/build_payload_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ServicePing::BuildPayloadService do
+  describe '#execute' do
+    subject(:service_ping_payload) { described_class.new.execute }
+
+    include_context 'stubbed service ping metrics definitions' do
+      let(:subscription_metrics) do
+        [
+          metric_attributes('license_md5', "Subscription")
+        ]
+      end
+    end
+
+    before do
+      allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: false))
+    end
+
+    context 'GitLab instance have a license' do
+      # License.current.present? == true
+      context 'Instance consented to submit optional product intelligence data' do
+        before do
+          # Gitlab::CurrentSettings.usage_ping_enabled? == true
+          stub_config_setting(usage_ping_enabled: true)
+        end
+
+        context 'Instance subscribes to free TAM service' do
+          before do
+            # License.current.usage_ping? == true
+            create_current_license(usage_ping_required_metrics_enabled: true)
+          end
+
+          it_behaves_like 'complete service ping payload'
+        end
+
+        context 'Instance does NOT subscribe to free TAM service' do
+          before do
+            # License.current.usage_ping? == false
+            create_current_license(usage_ping_required_metrics_enabled: false)
+          end
+
+          it_behaves_like 'service ping payload with all expected metrics' do
+            let(:expected_metrics) { standard_metrics + subscription_metrics + optional_metrics }
+          end
+
+          it_behaves_like 'service ping payload without restricted metrics' do
+            let(:restricted_metrics) { operational_metrics }
+          end
+        end
+      end
+
+      context 'Instance does NOT consented to submit optional product intelligence data' do
+        before do
+          # Gitlab::CurrentSettings.usage_ping_enabled? == false
+          stub_config_setting(usage_ping_enabled: false)
+        end
+
+        context 'Instance subscribes to free TAM service' do
+          before do
+            # License.current.usage_ping? == true
+            create_current_license(usage_ping_required_metrics_enabled: true)
+          end
+
+          it_behaves_like 'service ping payload with all expected metrics' do
+            let(:expected_metrics) { standard_metrics + subscription_metrics + operational_metrics }
+          end
+
+          it_behaves_like 'service ping payload without restricted metrics' do
+            let(:restricted_metrics) { optional_metrics }
+          end
+        end
+
+        context 'Instance does NOT subscribe to free TAM service' do
+          before do
+            # License.current.usage_ping? == false
+            create_current_license(usage_ping_required_metrics_enabled: false)
+          end
+
+          it 'returns empty service ping payload' do
+            expect(service_ping_payload).to eq({})
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/spec_helper.rb

@@ -21,6 +21,14 @@ RSpec.configure do |config|
     TestLicense.init
   end
 
+  config.before(:context, :without_license) do
+    License.destroy_all # rubocop: disable Cop/DestroyAll
+  end
+
+  config.after(:context, :without_license) do
+    TestLicense.init
+  end
+
   config.around(:each, :geo_tracking_db) do |example|
     example.run if Gitlab::Geo.geo_database_configured?
   end

spec/services/service_ping/build_payload_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ServicePing::BuildPayloadService do
+  describe '#execute', :without_license do
+    subject(:service_ping_payload) { described_class.new.execute }
+
+    include_context 'stubbed service ping metrics definitions' do
+      let(:subscription_metrics) do
+        [
+          metric_attributes('active_user_count', "Subscription")
+        ]
+      end
+    end
+
+    context 'when usage_ping_enabled setting is false' do
+      before do
+        # Gitlab::CurrentSettings.usage_ping_enabled? == false
+        stub_config_setting(usage_ping_enabled: false)
+      end
+
+      it 'returns empty service ping payload' do
+        expect(service_ping_payload).to eq({})
+      end
+    end
+
+    context 'when usage_ping_enabled setting is true' do
+      before do
+        # Gitlab::CurrentSettings.usage_ping_enabled? == true
+        stub_config_setting(usage_ping_enabled: true)
+      end
+
+      it_behaves_like 'complete service ping payload'
+
+      context 'with require stats consent enabled' do
+        before do
+          allow(User).to receive(:single_user).and_return(double(:user, requires_usage_stats_consent?: true))
+        end
+
+        it 'returns empty service ping payload' do
+          expect(service_ping_payload).to eq({})
+        end
+      end
+    end
+  end
+end

spec/support/matchers/usage_metric_matchers.rb

+# frozen_string_literal: true
+
+RSpec::Matchers.define :have_usage_metric do |key_path|
+  match do |payload|
+    payload = payload.deep_stringify_keys
+
+    key_path.split('.').each do |part|
+      break false unless payload&.has_key?(part)
+
+      payload = payload[part]
+    end
+  end
+
+  failure_message do
+    "Payload does not contain metric with key path: '#{key_path}'"
+  end
+
+  failure_message_when_negated do
+    "Payload contains restricted metric with key path: '#{key_path}'"
+  end
+end

spec/support/shared_contexts/services/service_ping/stubbed_service_ping_metrics_definitions_shared_context.rb

+# frozen_string_literal: true
+
+RSpec.shared_context 'stubbed service ping metrics definitions' do
+  include UsageDataHelpers
+
+  let(:metrics_definitions) { standard_metrics + subscription_metrics + operational_metrics + optional_metrics }
+  let(:standard_metrics) do
+    [
+      metric_attributes('uuid', "Standard")
+    ]
+  end
+
+  let(:operational_metrics) do
+    [
+      metric_attributes('counts.merge_requests', "Operational"),
+      metric_attributes('counts.todos', "Operational")
+    ]
+  end
+
+  let(:optional_metrics) do
+    [
+      metric_attributes('counts.boards', "Optional"),
+      metric_attributes('gitaly.filesystems', '').except('data_category')
+    ]
+  end
+
+  before do
+    stub_usage_data_connections
+    stub_object_store_settings
+
+    allow(Gitlab::Usage::MetricDefinition).to(
+      receive(:definitions)
+        .and_return(metrics_definitions.to_h { |definition| [definition['key_path'], Gitlab::Usage::MetricDefinition.new('', definition.symbolize_keys)] })
+    )
+  end
+
+  def metric_attributes(key_path, category)
+    {
+      'key_path' => key_path,
+      'data_category' => category
+    }
+  end
+end

spec/support/shared_examples/services/service_ping/complete_service_ping_payload_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'complete service ping payload' do
+  it_behaves_like 'service ping payload with all expected metrics' do
+    let(:expected_metrics) do
+      standard_metrics + subscription_metrics + operational_metrics + optional_metrics
+    end
+  end
+end

spec/support/shared_examples/services/service_ping/service_ping_payload_with_all_expected_metrics_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'service ping payload with all expected metrics' do
+  specify do
+    aggregate_failures do
+      expected_metrics.each do |metric|
+        is_expected.to have_usage_metric metric['key_path']
+      end
+    end
+  end
+end

spec/support/shared_examples/services/service_ping/service_ping_payload_without_restricted_metrics_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'service ping payload without restricted metrics' do
+  specify do
+    aggregate_failures do
+      restricted_metrics.each do |metric|
+        is_expected.not_to have_usage_metric metric['key_path']
+      end
+    end
+  end
+end