Merge branch '338461-migrate-subscription-metrics-to-use-instrumentation-classes-2' into 'master'

Migrate subscription metrics to use instrumentation classes

See merge request gitlab-org/gitlab!72495

ee/config/metrics/license/20210204124831_license_md5.yml

@@ -9,13 +9,15 @@ value_type: string
 status: active
 time_frame: none
 data_source: license
-instrumentation_class: LicenseMdFiveMetric
+instrumentation_class: LicenseMetric
+options:
+  attribute: md5
 data_category: standard
 distribution:
-- ee
+  - ee
 tier:
-- premium
-- ultimate
+  - premium
+  - ultimate
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/557
 performance_indicator_type: []
 milestone: "<13.9"

ee/config/metrics/license/20210204124833_license_id.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: license
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: id
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124843_license_user_count.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: license
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: user_count
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124845_license_starts_at.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: license
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: starts_at
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124847_license_expires_at.yml

@@ -10,6 +10,9 @@ status: active
 data_category: subscription
 time_frame: none
 data_source: license
+instrumentation_class: LicenseMetric
+options:
+  attribute: expires_at
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124849_license_plan.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: license
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: plan
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124851_license_trial.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: license
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: trial
 distribution:
 - ee
 tier:

ee/config/metrics/license/20210204124852_license_subscription_id.yml

@@ -9,7 +9,9 @@ value_type: string
 status: active
 time_frame: none
 data_source: license
-instrumentation_class: ZuoraSubscriptionIdMetric
+instrumentation_class: LicenseMetric
+options:
+  attribute: subscription_id
 data_category: standard
 distribution:
 - ee

ee/config/metrics/license/20210204124926_license_trial_ends_on.yml

@@ -10,6 +10,9 @@ status: active
 time_frame: none
 data_source: database
 data_category: subscription
+instrumentation_class: LicenseMetric
+options:
+  attribute: trial_ends_on
 distribution:
 - ee
 tier:

ee/lib/ee/gitlab/usage_data.rb

@@ -72,7 +72,7 @@ module EE
         def features_usage_data_ee
           {
             elasticsearch_enabled: alt_usage_data(fallback: nil) { ::Gitlab::CurrentSettings.elasticsearch_search? },
-            license_trial_ends_on: alt_usage_data(fallback: nil) { License.trial_ends_on },
+            license_trial_ends_on: add_metric("LicenseMetric", options: { attribute: "trial_ends_on" }),
             geo_enabled: alt_usage_data(fallback: nil) { ::Gitlab::Geo.enabled? },
             user_cap_feature_enabled: add_metric('UserCapSettingEnabledMetric', time_frame: 'none')
           }
@@ -90,7 +90,7 @@ module EE
             end
 
           if license
-            usage_data[:license_md5] = add_metric("LicenseMdFiveMetric")
+            usage_data[:license_md5] = add_metric("LicenseMetric", options: { attribute: 'md5' })
             usage_data[:license_id] = license.license_id
             # rubocop: disable UsageData/LargeTable
             usage_data[:historical_max_users] = add_metric("HistoricalMaxUsersMetric")
@@ -103,7 +103,7 @@ module EE
             usage_data[:license_plan] = license.plan
             usage_data[:license_add_ons] = license.add_ons
             usage_data[:license_trial] = license.trial?
-            usage_data[:license_subscription_id] = alt_usage_data(fallback: nil) { license.subscription_id }
+            usage_data[:license_subscription_id] = license.subscription_id
           end
 
           usage_data

ee/lib/gitlab/usage/metrics/instrumentations/license_md_five_metric.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Usage
-    module Metrics
-      module Instrumentations
-        class LicenseMdFiveMetric < ::Gitlab::Usage::Metrics::Instrumentations::GenericMetric
-          value do
-            ::License.current.md5
-          end
-        end
-      end
-    end
-  end
-end

ee/lib/gitlab/usage/metrics/instrumentations/license_metric.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        class LicenseMetric < GenericMetric
+          # Usage example
+          #
+          # In metric YAML defintion
+          # instrumentation_class: LicenseMetric
+          # options:
+          #   attribute: md5
+          # end
+
+          ALLOWED_ATTRIBUTES = %w(md5
+                                  id
+                                  plan
+                                  trial
+                                  starts_at
+                                  expires_at
+                                  user_count
+                                  trial_ends_on
+                                  subscription_id).freeze
+
+          def initialize(time_frame:, options: {})
+            super
+
+            raise ArgumentError, "License options attribute are required" unless license_attribute.present?
+            raise ArgumentError, "Attribute: #{license_attribute} it not allowed" unless license_attribute.in?(ALLOWED_ATTRIBUTES)
+          end
+
+          def value
+            return ::License.trial_ends_on if license_attribute == "trial_ends_on"
+
+            alt_usage_data(fallback: nil) do
+              # license_attribute is checked in the constructor, so it's safe
+              ::License.current.send(license_attribute)  # rubocop: disable GitlabSecurity/PublicSend
+            end
+          end
+
+          private
+
+          def license_attribute
+            options[:attribute]
+          end
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/usage/metrics/instrumentations/zuora_subscription_id_metric.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Usage
-    module Metrics
-      module Instrumentations
-        class ZuoraSubscriptionIdMetric < ::Gitlab::Usage::Metrics::Instrumentations::GenericMetric
-          value do
-            ::License.current.subscription_id
-          end
-        end
-      end
-    end
-  end
-end

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -187,6 +187,10 @@ RSpec.describe Gitlab::UsageData do
   describe '.features_usage_data_ee' do
     subject { described_class.features_usage_data_ee }
 
+    before do
+      stub_feature_flags(usage_data_instrumentation: false)
+    end
+
     it 'gathers feature usage data of EE' do
       expect(subject[:elasticsearch_enabled]).to eq(Gitlab::CurrentSettings.elasticsearch_search?)
       expect(subject[:geo_enabled]).to eq(Gitlab::Geo.enabled?)

ee/spec/lib/gitlab/usage/metrics/instrumentations/license_md_five_metric_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Usage::Metrics::Instrumentations::LicenseMdFiveMetric do
-  it_behaves_like 'a correct instrumented metric value', { time_frame: 'none', data_source: 'ruby' } do
-    let(:expected_value) { Digest::MD5.hexdigest(::License.current.data) }
-  end
-end

ee/spec/lib/gitlab/usage/metrics/instrumentations/zuora_subscription_id_metric_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Usage::Metrics::Instrumentations::ZuoraSubscriptionIdMetric do
-  it_behaves_like 'a correct instrumented metric value', { time_frame: 'none', data_source: 'ruby' } do
-    let(:expected_value) { ::License.current.subscription_id }
-  end
-end

lib/gitlab/usage/metrics/names_suggestions/generator.rb

@@ -10,10 +10,10 @@ module Gitlab
               uncached_data.deep_stringify_keys.dig(*key_path.split('.'))
             end
 
-            def add_metric(metric, time_frame: 'none')
+            def add_metric(metric, time_frame: 'none', options: {})
               metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
-              metric_class.new(time_frame: time_frame).suggested_name
+              metric_class.new(time_frame: time_frame, options: options).suggested_name
             end
 
             private

lib/gitlab/usage_data_non_sql_metrics.rb

@@ -12,10 +12,10 @@ module Gitlab
         super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
       end
 
-      def add_metric(metric, time_frame: 'none')
+      def add_metric(metric, time_frame: 'none', options: {})
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
-        metric_class.new(time_frame: time_frame).instrumentation
+        metric_class.new(time_frame: time_frame, options: options).instrumentation
       end
 
       def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)

lib/gitlab/usage_data_queries.rb

@@ -12,10 +12,10 @@ module Gitlab
         super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
       end
 
-      def add_metric(metric, time_frame: 'none')
+      def add_metric(metric, time_frame: 'none', options: {})
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
-        metric_class.new(time_frame: time_frame).instrumentation
+        metric_class.new(time_frame: time_frame, options: options).instrumentation
       end
 
       def count(relation, column = nil, *args, **kwargs)

lib/gitlab/utils/usage_data.rb

@@ -45,14 +45,14 @@ module Gitlab
       MAX_BUCKET_SIZE = 100
       INSTRUMENTATION_CLASS_FALLBACK = -100
 
-      def add_metric(metric, time_frame: 'none')
+      def add_metric(metric, time_frame: 'none', options: {})
         # Results of this method should be overwritten by instrumentation class values
         # -100 indicates the metric was not properly merged.
         return INSTRUMENTATION_CLASS_FALLBACK if Feature.enabled?(:usage_data_instrumentation)
 
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
-        metric_class.new(time_frame: time_frame).value
+        metric_class.new(time_frame: time_frame, options: options).value
       end
 
       def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)