Commit 993d49f3 authored by Nikola Milojevic's avatar Nikola Milojevic

Merge branch '334704-accept-lowercase-for-data_category-in-metric-definition' into 'master'

Accept both uppercase and lowercase for data_category in metric definition

See merge request gitlab-org/gitlab!67469
parents 451be2ed 380ca805
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
module ServicePing module ServicePing
class PermitDataCategoriesService class PermitDataCategoriesService
STANDARD_CATEGORY = 'Standard' STANDARD_CATEGORY = 'standard'
SUBSCRIPTION_CATEGORY = 'Subscription' SUBSCRIPTION_CATEGORY = 'subscription'
OPERATIONAL_CATEGORY = 'Operational' OPERATIONAL_CATEGORY = 'operational'
OPTIONAL_CATEGORY = 'Optional' OPTIONAL_CATEGORY = 'optional'
CATEGORIES = [ CATEGORIES = [
STANDARD_CATEGORY, STANDARD_CATEGORY,
SUBSCRIPTION_CATEGORY, SUBSCRIPTION_CATEGORY,
......
...@@ -2,6 +2,6 @@ ...@@ -2,6 +2,6 @@
"type": "array", "type": "array",
"items": { "items": {
"type": ["string", "null"], "type": ["string", "null"],
"enum": ["Standard", "Subscription", "Operational", "Optional"] "enum": ["standard", "subscription", "operational", "optional"]
} }
} }
...@@ -59,7 +59,7 @@ ...@@ -59,7 +59,7 @@
}, },
"data_category": { "data_category": {
"type": "string", "type": "string",
"enum": ["Operational", "Optional", "Subscription", "Standard"] "enum": ["Operational", "Optional", "Subscription", "Standard", "operational", "optional", "subscription", "standard"]
}, },
"instrumentation_class": { "instrumentation_class": {
"type": "string", "type": "string",
......
...@@ -37,7 +37,7 @@ Each metric is defined in a separate YAML file consisting of a number of fields: ...@@ -37,7 +37,7 @@ Each metric is defined in a separate YAML file consisting of a number of fields:
| `status` | yes | `string`; [status](#metric-statuses) of the metric, may be set to `data_available`, `implemented`, `not_used`, `deprecated`, `removed`, `broken`. | | `status` | yes | `string`; [status](#metric-statuses) of the metric, may be set to `data_available`, `implemented`, `not_used`, `deprecated`, `removed`, `broken`. |
| `time_frame` | yes | `string`; may be set to a value like `7d`, `28d`, `all`, `none`. | | `time_frame` | yes | `string`; may be set to a value like `7d`, `28d`, `all`, `none`. |
| `data_source` | yes | `string`; may be set to a value like `database`, `redis`, `redis_hll`, `prometheus`, `system`. | | `data_source` | yes | `string`; may be set to a value like `database`, `redis`, `redis_hll`, `prometheus`, `system`. |
| `data_category` | yes | `string`; [categories](#data-category) of the metric, may be set to `Operational`, `Optional`, `Subscription`, `Standard`. The default value is `Optional`.| | `data_category` | yes | `string`; [categories](#data-category) of the metric, may be set to `operational`, `optional`, `subscription`, `standard`. The default value is `optional`.|
| `instrumentation_class` | no | `string`; [the class that implements the metric](metrics_instrumentation.md). | | `instrumentation_class` | no | `string`; [the class that implements the metric](metrics_instrumentation.md). |
| `distribution` | yes | `array`; may be set to one of `ce, ee` or `ee`. The [distribution](https://about.gitlab.com/handbook/marketing/strategic-marketing/tiers/#definitions) where the tracked feature is available. | | `distribution` | yes | `array`; may be set to one of `ce, ee` or `ee`. The [distribution](https://about.gitlab.com/handbook/marketing/strategic-marketing/tiers/#definitions) where the tracked feature is available. |
| `performance_indicator_type` | no | `array`; may be set to one of [`gmau`, `smau`, `paid_gmau`, or `umau`](https://about.gitlab.com/handbook/business-technology/data-team/data-catalog/xmau-analysis/). | | `performance_indicator_type` | no | `array`; may be set to one of [`gmau`, `smau`, `paid_gmau`, or `umau`](https://about.gitlab.com/handbook/business-technology/data-team/data-catalog/xmau-analysis/). |
...@@ -99,10 +99,10 @@ should be changed. ...@@ -99,10 +99,10 @@ should be changed.
We use the following categories to classify a metric: We use the following categories to classify a metric:
- `Operational`: Required data for operational purposes. - `operational`: Required data for operational purposes.
- `Optional`: Default value for a metric. Data that is optional to collect. This can be [enabled or disabled](../service_ping/index.md#disable-service-ping) in the Admin Area. - `optional`: Default value for a metric. Data that is optional to collect. This can be [enabled or disabled](../service_ping/index.md#disable-service-ping) in the Admin Area.
- `Subscription`: Data related to licensing. - `subscription`: Data related to licensing.
- `Standard`: Standard set of identifiers that are included when collecting data. - `standard`: Standard set of identifiers that are included when collecting data.
### Metric name suggestion examples ### Metric name suggestion examples
......
...@@ -9,7 +9,7 @@ RSpec.describe ServicePing::BuildPayloadService do ...@@ -9,7 +9,7 @@ RSpec.describe ServicePing::BuildPayloadService do
include_context 'stubbed service ping metrics definitions' do include_context 'stubbed service ping metrics definitions' do
let(:subscription_metrics) do let(:subscription_metrics) do
[ [
metric_attributes('license_md5', "Subscription") metric_attributes('license_md5', "subscription")
] ]
end end
end end
......
...@@ -13,7 +13,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do ...@@ -13,7 +13,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do
end end
it 'returns all categories' do it 'returns all categories' do
expect(permitted_categories).to match_array(%w[Standard Subscription Operational Optional]) expect(permitted_categories).to match_array(%w[standard subscription operational optional])
end end
end end
...@@ -41,7 +41,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do ...@@ -41,7 +41,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do
end end
it 'returns all categories' do it 'returns all categories' do
expect(permitted_categories).to match_array(%w[Standard Subscription Operational Optional]) expect(permitted_categories).to match_array(%w[standard subscription operational optional])
end end
context 'when User.single_user&.requires_usage_stats_consent? is required' do context 'when User.single_user&.requires_usage_stats_consent? is required' do
...@@ -62,7 +62,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do ...@@ -62,7 +62,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do
end end
it 'returns all categories' do it 'returns all categories' do
expect(permitted_categories).to match_array(%w[Standard Subscription Operational Optional]) expect(permitted_categories).to match_array(%w[standard subscription operational optional])
end end
end end
end end
...@@ -80,7 +80,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do ...@@ -80,7 +80,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do
end end
it 'returns all categories' do it 'returns all categories' do
expect(permitted_categories).to match_array(%w[Standard Subscription Operational]) expect(permitted_categories).to match_array(%w[standard subscription operational])
end end
end end
......
...@@ -11,7 +11,7 @@ milestone: "<%= milestone %>" ...@@ -11,7 +11,7 @@ milestone: "<%= milestone %>"
introduced_by_url: introduced_by_url:
time_frame: <%= time_frame %> time_frame: <%= time_frame %>
data_source: data_source:
data_category: Optional data_category: optional
performance_indicator_type: performance_indicator_type:
distribution: distribution:
<%= distribution %> <%= distribution %>
......
...@@ -55,6 +55,10 @@ module Gitlab ...@@ -55,6 +55,10 @@ module Gitlab
end end
end end
def category_to_lowercase
attributes[:data_category]&.downcase!
end
alias_method :to_dictionary, :to_h alias_method :to_dictionary, :to_h
class << self class << self
...@@ -96,7 +100,7 @@ module Gitlab ...@@ -96,7 +100,7 @@ module Gitlab
definition = YAML.safe_load(definition) definition = YAML.safe_load(definition)
definition.deep_symbolize_keys! definition.deep_symbolize_keys!
self.new(path, definition).tap(&:validate!) self.new(path, definition).tap(&:validate!).tap(&:category_to_lowercase)
rescue StandardError => e rescue StandardError => e
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(InvalidError.new(e.message)) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(InvalidError.new(e.message))
end end
......
...@@ -12,7 +12,7 @@ milestone: "13.9" ...@@ -12,7 +12,7 @@ milestone: "13.9"
introduced_by_url: introduced_by_url:
time_frame: 7d time_frame: 7d
data_source: data_source:
data_category: Operational data_category: operational
performance_indicator_type: performance_indicator_type:
distribution: distribution:
- ce - ce
......
...@@ -12,7 +12,7 @@ milestone: "13.9" ...@@ -12,7 +12,7 @@ milestone: "13.9"
introduced_by_url: introduced_by_url:
time_frame: 7d time_frame: 7d
data_source: data_source:
data_category: Optional data_category: optional
performance_indicator_type: performance_indicator_type:
distribution: distribution:
- ee - ee
......
...@@ -13,7 +13,7 @@ milestone: "13.9" ...@@ -13,7 +13,7 @@ milestone: "13.9"
introduced_by_url: introduced_by_url:
time_frame: 7d time_frame: 7d
data_source: data_source:
data_category: Optional data_category: optional
performance_indicator_type: performance_indicator_type:
distribution: distribution:
- ce - ce
......
...@@ -13,7 +13,7 @@ RSpec.describe Gitlab::Usage::Docs::Helper do ...@@ -13,7 +13,7 @@ RSpec.describe Gitlab::Usage::Docs::Helper do
let(:metric_definition) do let(:metric_definition) do
{ {
data_category: 'Standard', data_category: 'standard',
name: 'test_metric', name: 'test_metric',
description: description, description: description,
product_group: 'group::product intelligence', product_group: 'group::product intelligence',
...@@ -66,7 +66,7 @@ RSpec.describe Gitlab::Usage::Docs::Helper do ...@@ -66,7 +66,7 @@ RSpec.describe Gitlab::Usage::Docs::Helper do
end end
describe '#render_data_category' do describe '#render_data_category' do
let(:expected) { 'Data Category: `Standard`' } let(:expected) { 'Data Category: `standard`' }
it { expect(helper.render_data_category(metric_definition)).to eq(expected) } it { expect(helper.render_data_category(metric_definition)).to eq(expected) }
end end
......
...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do ...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
distribution: %w(ee ce), distribution: %w(ee ce),
tier: %w(free starter premium ultimate bronze silver gold), tier: %w(free starter premium ultimate bronze silver gold),
name: 'uuid', name: 'uuid',
data_category: 'Standard' data_category: 'standard'
} }
end end
...@@ -199,7 +199,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do ...@@ -199,7 +199,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
data_source: 'database', data_source: 'database',
distribution: %w(ee ce), distribution: %w(ee ce),
tier: %w(free starter premium ultimate bronze silver gold), tier: %w(free starter premium ultimate bronze silver gold),
data_category: 'Optional' data_category: 'optional'
} }
end end
......
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CollectedDataCategoriesMetric do RSpec.describe Gitlab::Usage::Metrics::Instrumentations::CollectedDataCategoriesMetric do
it_behaves_like 'a correct instrumented metric value', {} do it_behaves_like 'a correct instrumented metric value', {} do
let(:expected_value) { %w[Standard Subscription Operational Optional] } let(:expected_value) { %w[standard subscription operational optional] }
before do before do
allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance| allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance|
......
...@@ -1080,7 +1080,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do ...@@ -1080,7 +1080,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
end end
it 'reports collected data categories' do it 'reports collected data categories' do
expected_value = %w[Standard Subscription Operational Optional] expected_value = %w[standard subscription operational optional]
allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance| allow_next_instance_of(ServicePing::PermitDataCategoriesService) do |instance|
expect(instance).to receive(:execute).and_return(expected_value) expect(instance).to receive(:execute).and_return(expected_value)
......
...@@ -13,7 +13,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do ...@@ -13,7 +13,7 @@ RSpec.describe ServicePing::PermitDataCategoriesService do
end end
it 'returns all categories' do it 'returns all categories' do
expect(permitted_categories).to match_array(%w[Standard Subscription Operational Optional]) expect(permitted_categories).to match_array(%w[standard subscription operational optional])
end end
end end
......
...@@ -6,20 +6,20 @@ RSpec.shared_context 'stubbed service ping metrics definitions' do ...@@ -6,20 +6,20 @@ RSpec.shared_context 'stubbed service ping metrics definitions' do
let(:metrics_definitions) { standard_metrics + subscription_metrics + operational_metrics + optional_metrics } let(:metrics_definitions) { standard_metrics + subscription_metrics + operational_metrics + optional_metrics }
let(:standard_metrics) do let(:standard_metrics) do
[ [
metric_attributes('uuid', "Standard") metric_attributes('uuid', "standard")
] ]
end end
let(:operational_metrics) do let(:operational_metrics) do
[ [
metric_attributes('counts.merge_requests', "Operational"), metric_attributes('counts.merge_requests', "operational"),
metric_attributes('counts.todos', "Operational") metric_attributes('counts.todos', "operational")
] ]
end end
let(:optional_metrics) do let(:optional_metrics) do
[ [
metric_attributes('counts.boards', "Optional"), metric_attributes('counts.boards', "optional"),
metric_attributes('gitaly.filesystems', '').except('data_category') metric_attributes('gitaly.filesystems', '').except('data_category')
] ]
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment