Merge branch...

Merge branch '335532-fix-parse_usage_ping_keys-method-to-be-able-to-check-hash-values' into 'master'

Fix parse_usage_ping_keys method to test metrics with schemas

See merge request gitlab-org/gitlab!66971

config/metrics/objects_schemas/projects_with_enabled_alert_integrations_histogram.json

 {
   "type": "object",
-  "description": "Histogram (buckets 1 to 100) of projects with at least 1 enabled integration"
+  "description": "Histogram (buckets 1 to 100) of projects with at least 1 enabled integration",
   "propertyNames": {
     "pattern": "^[1-9][0-9]?0?$"
   }

config/metrics/objects_schemas/topology_schema.json

 {
   "type": "object",
-  "required": ["duration", "failures"],
+  "required": ["duration_s", "failures"],
   "properties": {
-    "duration": { "type": "number", "description": "The time it took to collect topology data" },
+    "duration_s": { "type": "number", "description": "The time it took to collect topology data" },
     "failures": { "type": "array", "description": "The information about failed queries" },
     "application_requests_per_hour": { "type": "number", "description": "The number of requests to the web application per hour" },
     "nodes": {

ee/spec/config/metrics/every_metric_definition_spec.rb

@@ -11,7 +11,6 @@ RSpec.describe 'Every metric definition' do
       geo_node_usage
       license_add_ons
       testing_total_unique_counts
-      topology
       user_auth_by_provider
     ).freeze
   end
@@ -19,7 +18,7 @@ RSpec.describe 'Every metric definition' do
   let(:usage_ping_key_paths) do
     parse_usage_ping_keys(usage_ping)
       .flatten
-      .reject { |v| v =~ Regexp.union(ignored_usage_ping_key_patterns)}
+      .reject { |v| v =~ Regexp.union(ignored_usage_ping_key_patterns) }
       .sort
   end
 
@@ -29,7 +28,6 @@ RSpec.describe 'Every metric definition' do
       geo_node_usage
       mock_ci
       mock_monitoring
-      projects_with_enabled_alert_integrations_histogram
       user_auth_by_provider
       user_dast_scans
       user_sast_scans
@@ -38,22 +36,27 @@ RSpec.describe 'Every metric definition' do
       user_secret_detection_scans
       user_coverage_fuzzing_scans
       user_api_fuzzing_scans
-      topology
     ).freeze
   end
 
   let(:metric_files_key_paths) do
     Gitlab::Usage::MetricDefinition
       .definitions
-      .reject { |k, v| v.status == 'removed' || v.key_path =~ Regexp.union(ignored_metric_files_key_patterns)}
+      .reject { |k, v| v.status == 'removed' || v.key_path =~ Regexp.union(ignored_metric_files_key_patterns) }
       .keys
       .sort
   end
 
+  let(:metric_files_with_schema) do
+    Gitlab::Usage::MetricDefinition
+      .definitions
+      .select { |k, v| v.respond_to?(:value_json_schema) }
+  end
+
   # Recursively traverse nested Hash of a generated Usage Ping to return an Array of key paths
   # in the dotted format used in metric definition YAML files, e.g.: 'count.category.metric_name'
   def parse_usage_ping_keys(object, key_path = [])
-    if object.is_a? Hash
+    if object.is_a?(Hash) && !object_with_schema?(key_path.join('.'))
       object.each_with_object([]) do |(key, value), result|
         result.append parse_usage_ping_keys(value, key_path + [key])
       end
@@ -62,6 +65,10 @@ RSpec.describe 'Every metric definition' do
     end
   end
 
+  def object_with_schema?(key_path)
+    metric_files_with_schema.key?(key_path)
+  end
+
   before do
     allow(Gitlab::UsageData).to receive_messages(count: -1, distinct_count: -1, estimate_batch_distinct_count: -1, sum: -1, alt_usage_data: -1)
     allow(Gitlab::Geo).to receive(:enabled?).and_return(true)
@@ -73,4 +80,14 @@ RSpec.describe 'Every metric definition' do
   it 'is included in the Usage Ping hash structure' do
     expect(metric_files_key_paths).to match_array(usage_ping_key_paths)
   end
+
+  context 'with value json schema' do
+    it 'has a valid structure', :aggregate_failures do
+      metric_files_with_schema.each do |key_path, metric|
+        structure = usage_ping.dig(*key_path.split('.').map(&:to_sym))
+
+        expect(structure).to match_metric_definition_schema(metric.value_json_schema)
+      end
+    end
+  end
 end

lib/gitlab/usage/metric_definition.rb

@@ -114,6 +114,10 @@ module Gitlab
         attributes[method] || super
       end
 
+      def respond_to_missing?(method, *args)
+        attributes[method].present? || super
+      end
+
       def skip_validation?
         !!attributes[:skip_validation] || @skip_validation || SKIP_VALIDATION_STATUSES.include?(attributes[:status])
       end

lib/gitlab/usage/metrics/instrumentations/collected_data_categories_metric.rb

@@ -6,7 +6,7 @@ module Gitlab
       module Instrumentations
         class CollectedDataCategoriesMetric < GenericMetric
           value do
-            ::ServicePing::PermitDataCategoriesService.new.execute
+            ::ServicePing::PermitDataCategoriesService.new.execute.to_a
           end
         end
       end

spec/support/matchers/schema_matcher.rb

@@ -45,6 +45,17 @@ RSpec::Matchers.define :match_response_schema do |schema, dir: nil, **options|
   end
 end
 
+RSpec::Matchers.define :match_metric_definition_schema do |path, dir: nil, **options|
+  match do |data|
+    schema_path = Pathname.new(Rails.root.join(dir.to_s, path).to_s)
+    validator = SchemaPath.validator(schema_path)
+
+    data = data.stringify_keys if data.is_a? Hash
+
+    validator.valid?(data)
+  end
+end
+
 RSpec::Matchers.define :match_snowplow_schema do |schema, dir: nil, **options|
   match do |data|
     schema_path = Pathname.new(Rails.root.join(dir.to_s, 'spec', "fixtures/product_intelligence/#{schema}.json").to_s)