From 3867ab22dd2d74e497802b67c7acaf0370334573 Mon Sep 17 00:00:00 2001 From: Nikolay Belokolodov Date: Mon, 8 Nov 2021 08:43:07 +0000 Subject: [PATCH] Add support for creating multiple metric definitions --- .../service_ping/metrics_dictionary.md | 16 ++++++++--- .../metric_definition.yml | 2 +- .../usage_metric_definition_generator.rb | 27 +++++++++++-------- .../usage_metric_definition_generator_spec.rb | 11 ++++++++ 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/doc/development/service_ping/metrics_dictionary.md b/doc/development/service_ping/metrics_dictionary.md index c1478e6290e0..b3c404de1507 100644 --- a/doc/development/service_ping/metrics_dictionary.md +++ b/doc/development/service_ping/metrics_dictionary.md @@ -197,18 +197,28 @@ tier: The GitLab codebase provides a dedicated [generator](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/generators/gitlab/usage_metric_definition_generator.rb) to create new metric definitions. -For uniqueness, the generated file includes a timestamp prefix, in ISO 8601 format. +For uniqueness, the generated files include a timestamp prefix in ISO 8601 format. -The generator takes the key path argument and 2 options and creates the metric YAML definition in corresponding location: +The generator takes a list of key paths and 2 options as arguments. It creates metric YAML definitions in the corresponding location: - `--ee`, `--no-ee` Indicates if metric is for EE. -- `--dir=DIR` indicates the metric directory. It must be one of: `counts_7d`, `7d`, `counts_28d`, `28d`, `counts_all`, `all`, `settings`, `license`. +- `--dir=DIR` Indicates the metric directory. It must be one of: `counts_7d`, `7d`, `counts_28d`, `28d`, `counts_all`, `all`, `settings`, `license`. + +**Single metric example** ```shell bundle exec rails generate gitlab:usage_metric_definition counts.issues --dir=7d create config/metrics/counts_7d/issues.yml ``` +**Multiple metrics example** + +```shell +bundle exec rails generate gitlab:usage_metric_definition counts.issues counts.users --dir=7d +create config/metrics/counts_7d/issues.yml +create config/metrics/counts_7d/users.yml +``` + NOTE: To create a metric definition used in EE, add the `--ee` flag. diff --git a/generator_templates/usage_metric_definition/metric_definition.yml b/generator_templates/usage_metric_definition/metric_definition.yml index 9ce03bdb7b4c..c094d6d3d88e 100644 --- a/generator_templates/usage_metric_definition/metric_definition.yml +++ b/generator_templates/usage_metric_definition/metric_definition.yml @@ -1,5 +1,5 @@ --- -key_path: <%= key_path %><%= metric_name_suggestion %> +key_path: <%= args.second %><%= metric_name_suggestion(args.second) %> description: product_section: product_stage: diff --git a/lib/generators/gitlab/usage_metric_definition_generator.rb b/lib/generators/gitlab/usage_metric_definition_generator.rb index 2d65363bf7b3..bd34ab0a16f7 100644 --- a/lib/generators/gitlab/usage_metric_definition_generator.rb +++ b/lib/generators/gitlab/usage_metric_definition_generator.rb @@ -30,18 +30,20 @@ def match?(str) source_root File.expand_path('../../../generator_templates/usage_metric_definition', __dir__) - desc 'Generates a metric definition yml file' + desc 'Generates metric definitions yml files' class_option :ee, type: :boolean, optional: true, default: false, desc: 'Indicates if metric is for ee' class_option :dir, type: :string, desc: "Indicates the metric location. It must be one of: #{VALID_INPUT_DIRS.join(', ')}" - argument :key_path, type: :string, desc: 'Unique JSON key path for the metric' + argument :key_paths, type: :array, desc: 'Unique JSON key paths for the metrics' def create_metric_file validate! - template "metric_definition.yml", file_path + key_paths.each do |key_path| + template "metric_definition.yml", file_path(key_path), key_path + end end def time_frame @@ -66,12 +68,12 @@ def milestone private - def metric_name_suggestion + def metric_name_suggestion(key_path) "\nname: \"#{Usage::Metrics::NamesSuggestions::Generator.generate(key_path)}\"" end - def file_path - path = File.join(TOP_LEVEL_DIR, 'metrics', directory&.name, "#{file_name}.yml") + def file_path(key_path) + path = File.join(TOP_LEVEL_DIR, 'metrics', directory&.name, "#{file_name(key_path)}.yml") path = File.join(TOP_LEVEL_DIR_EE, path) if ee? path end @@ -79,7 +81,10 @@ def file_path def validate! raise "--dir option is required" unless input_dir.present? raise "Invalid dir #{input_dir}, allowed options are #{VALID_INPUT_DIRS.join(', ')}" unless directory.present? - raise "Metric definition with key path '#{key_path}' already exists" if metric_definition_exists? + + key_paths.each do |key_path| + raise "Metric definition with key path '#{key_path}' already exists" if metric_definition_exists?(key_path) + end end def ee? @@ -93,15 +98,15 @@ def input_dir # Example of file name # # 20210201124931_g_project_management_issue_title_changed_weekly.yml - def file_name - "#{Time.now.utc.strftime("%Y%m%d%H%M%S")}_#{metric_name}" + def file_name(key_path) + "#{Time.now.utc.strftime("%Y%m%d%H%M%S")}_#{metric_name(key_path)}" end def directory @directory ||= TIME_FRAME_DIRS.find { |d| d.match?(input_dir) } end - def metric_name + def metric_name(key_path) key_path.split('.').last end @@ -109,7 +114,7 @@ def metric_definitions @definitions ||= Gitlab::Usage::MetricDefinition.definitions(skip_validation: true) end - def metric_definition_exists? + def metric_definition_exists?(key_path) metric_definitions[key_path].present? end end diff --git a/spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb b/spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb index 05833cf4ec47..b67425ae0127 100644 --- a/spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb +++ b/spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb @@ -99,4 +99,15 @@ expect(YAML.safe_load(File.read(metric_definition_path))).to include("name" => "some name") end end + + context 'with multiple file names' do + let(:key_paths) { ['counts_weekly.test_metric', 'counts_weekly.test1_metric'] } + + it 'creates multiple files' do + described_class.new(key_paths, { 'dir' => dir }).invoke_all + files = Dir.glob(File.join(temp_dir, 'metrics/counts_7d/*_metric.yml')) + + expect(files.count).to eq(2) + end + end end